diff options
11 files changed, 88 insertions, 41 deletions
diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h index 0bbcfac3b81..2b18a7df53d 100644 --- a/clang/include/clang/Lex/ModuleMap.h +++ b/clang/include/clang/Lex/ModuleMap.h @@ -231,8 +231,7 @@ private: return static_cast<bool>(findHeaderInUmbrellaDirs(File, IntermediateDirs)); } - Module *inferFrameworkModule(StringRef ModuleName, - const DirectoryEntry *FrameworkDir, + Module *inferFrameworkModule(const DirectoryEntry *FrameworkDir, Attributes Attrs, Module *Parent); public: @@ -344,10 +343,9 @@ public: /// \brief Infer the contents of a framework module map from the given /// framework directory. - Module *inferFrameworkModule(StringRef ModuleName, - const DirectoryEntry *FrameworkDir, + Module *inferFrameworkModule(const DirectoryEntry *FrameworkDir, bool IsSystem, Module *Parent); - + /// \brief Retrieve the module map file containing the definition of the given /// module. /// diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index 89120ff84d9..6c5c64bd266 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -532,9 +532,13 @@ const FileEntry *DirectoryLookup::DoFrameworkLookup( // Load this framework module. If that succeeds, find the suggested module // for this header, if any. bool IsSystem = getDirCharacteristic() != SrcMgr::C_User; - if (HS.loadFrameworkModule(ModuleName, TopFrameworkDir, IsSystem)) { - *SuggestedModule = HS.findModuleForHeader(FE); - } + HS.loadFrameworkModule(ModuleName, TopFrameworkDir, IsSystem); + + // FIXME: This can find a module not part of ModuleName, which is + // important so that we're consistent about whether this header + // corresponds to a module. Possibly we should lock down framework modules + // so that this is not possible. + *SuggestedModule = HS.findModuleForHeader(FE); } else { *SuggestedModule = HS.findModuleForHeader(FE); } @@ -1238,6 +1242,9 @@ Module *HeaderSearch::loadFrameworkModule(StringRef Name, // Try to load a module map file. switch (loadModuleMapFile(Dir, IsSystem, /*IsFramework*/true)) { case LMM_InvalidModuleMap: + // Try to infer a module map from the framework directory. + if (HSOpts->ImplicitModuleMaps) + ModMap.inferFrameworkModule(Dir, IsSystem, /*Parent=*/nullptr); break; case LMM_AlreadyLoaded: @@ -1245,15 +1252,10 @@ Module *HeaderSearch::loadFrameworkModule(StringRef Name, return nullptr; case LMM_NewlyLoaded: - return ModMap.findModule(Name); + break; } - - // Try to infer a module map from the framework directory. - if (HSOpts->ImplicitModuleMaps) - return ModMap.inferFrameworkModule(Name, Dir, IsSystem, /*Parent=*/nullptr); - - return nullptr; + return ModMap.findModule(Name); } diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index 306401c5e6f..e6fe38927e2 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -583,19 +583,28 @@ static void inferFrameworkLink(Module *Mod, const DirectoryEntry *FrameworkDir, } } -Module * -ModuleMap::inferFrameworkModule(StringRef ModuleName, - const DirectoryEntry *FrameworkDir, - bool IsSystem, - Module *Parent) { +Module *ModuleMap::inferFrameworkModule(const DirectoryEntry *FrameworkDir, + bool IsSystem, Module *Parent) { Attributes Attrs; Attrs.IsSystem = IsSystem; - return inferFrameworkModule(ModuleName, FrameworkDir, Attrs, Parent); + return inferFrameworkModule(FrameworkDir, Attrs, Parent); } -Module *ModuleMap::inferFrameworkModule(StringRef ModuleName, - const DirectoryEntry *FrameworkDir, +Module *ModuleMap::inferFrameworkModule(const DirectoryEntry *FrameworkDir, Attributes Attrs, Module *Parent) { + // Note: as an egregious but useful hack we use the real path here, because + // we might be looking at an embedded framework that symlinks out to a + // top-level framework, and we need to infer as if we were naming the + // top-level framework. + StringRef FrameworkDirName = + SourceMgr.getFileManager().getCanonicalName(FrameworkDir); + + // In case this is a case-insensitive filesystem, use the canonical + // directory name as the ModuleName, since modules are case-sensitive. + // FIXME: we should be able to give a fix-it hint for the correct spelling. + SmallString<32> ModuleNameStorage; + StringRef ModuleName = sanitizeFilenameAsIdentifier( + llvm::sys::path::stem(FrameworkDirName), ModuleNameStorage); // Check whether we've already found this module. if (Module *Mod = lookupModuleQualified(ModuleName, Parent)) @@ -608,20 +617,6 @@ Module *ModuleMap::inferFrameworkModule(StringRef ModuleName, const FileEntry *ModuleMapFile = nullptr; if (!Parent) { // Determine whether we're allowed to infer a module map. - - // Note: as an egregious but useful hack we use the real path here, because - // we might be looking at an embedded framework that symlinks out to a - // top-level framework, and we need to infer as if we were naming the - // top-level framework. - StringRef FrameworkDirName - = SourceMgr.getFileManager().getCanonicalName(FrameworkDir); - - // In case this is a case-insensitive filesystem, make sure the canonical - // directory name matches ModuleName exactly. Modules are case-sensitive. - // FIXME: we should be able to give a fix-it hint for the correct spelling. - if (llvm::sys::path::stem(FrameworkDirName) != ModuleName) - return nullptr; - bool canInfer = false; if (llvm::sys::path::has_parent_path(FrameworkDirName)) { // Figure out the parent path. @@ -747,10 +742,7 @@ Module *ModuleMap::inferFrameworkModule(StringRef ModuleName, continue; // FIXME: Do we want to warn about subframeworks without umbrella headers? - SmallString<32> NameBuf; - inferFrameworkModule(sanitizeFilenameAsIdentifier( - llvm::sys::path::stem(Dir->path()), NameBuf), - SubframeworkDir, Attrs, Result); + inferFrameworkModule(SubframeworkDir, Attrs, Result); } } diff --git a/clang/test/Modules/Inputs/ImportNameInDir.h b/clang/test/Modules/Inputs/ImportNameInDir.h new file mode 100644 index 00000000000..ae7b1a0fb27 --- /dev/null +++ b/clang/test/Modules/Inputs/ImportNameInDir.h @@ -0,0 +1,4 @@ +#import <NameInDir/NameInDir.h> + +// Don't crash. +#undef NAME_IN_DIR diff --git a/clang/test/Modules/Inputs/NameInDir.framework/Headers/NameInDir.h b/clang/test/Modules/Inputs/NameInDir.framework/Headers/NameInDir.h new file mode 100644 index 00000000000..bea23914858 --- /dev/null +++ b/clang/test/Modules/Inputs/NameInDir.framework/Headers/NameInDir.h @@ -0,0 +1,2 @@ +// NameInDir.h +#define NAME_IN_DIR 1 diff --git a/clang/test/Modules/Inputs/NameInDir.framework/Modules/module.modulemap b/clang/test/Modules/Inputs/NameInDir.framework/Modules/module.modulemap new file mode 100644 index 00000000000..48e1c56749f --- /dev/null +++ b/clang/test/Modules/Inputs/NameInDir.framework/Modules/module.modulemap @@ -0,0 +1,5 @@ +framework module NameInModMap { + umbrella header "NameInDir.h" + export * + module * { export * } +} diff --git a/clang/test/Modules/Inputs/NameInDir2.framework/Headers/NameInDir2.h b/clang/test/Modules/Inputs/NameInDir2.framework/Headers/NameInDir2.h new file mode 100644 index 00000000000..6dc3eea149e --- /dev/null +++ b/clang/test/Modules/Inputs/NameInDir2.framework/Headers/NameInDir2.h @@ -0,0 +1 @@ +// NameInDir2.h diff --git a/clang/test/Modules/Inputs/NameInDir2.framework/Modules/module.modulemap b/clang/test/Modules/Inputs/NameInDir2.framework/Modules/module.modulemap new file mode 100644 index 00000000000..24f15f8e846 --- /dev/null +++ b/clang/test/Modules/Inputs/NameInDir2.framework/Modules/module.modulemap @@ -0,0 +1,5 @@ +framework module NameInDir2 { + umbrella header "NameInDir2.h" + export * + module * { export * } +} diff --git a/clang/test/Modules/Inputs/NameInDirInferred.framework/Headers/NameInDirInferred.h b/clang/test/Modules/Inputs/NameInDirInferred.framework/Headers/NameInDirInferred.h new file mode 100644 index 00000000000..c0b12e6f945 --- /dev/null +++ b/clang/test/Modules/Inputs/NameInDirInferred.framework/Headers/NameInDirInferred.h @@ -0,0 +1 @@ +// NameInDirInferred.h diff --git a/clang/test/Modules/Inputs/module.map b/clang/test/Modules/Inputs/module.map index b9c8e72cfa9..ffaa53e18e2 100644 --- a/clang/test/Modules/Inputs/module.map +++ b/clang/test/Modules/Inputs/module.map @@ -332,3 +332,7 @@ module DebugModule { header "DebugModule.h" } +module ImportNameInDir { + header "ImportNameInDir.h" + export * +} diff --git a/clang/test/Modules/framework-name.m b/clang/test/Modules/framework-name.m new file mode 100644 index 00000000000..a63e206073d --- /dev/null +++ b/clang/test/Modules/framework-name.m @@ -0,0 +1,33 @@ +// REQUIRES: shell +// RUN: rm -rf %t.mcp %t +// RUN: mkdir -p %t +// RUN: ln -s %S/Inputs/NameInDir2.framework %t/NameInImport.framework +// RUN: ln -s %S/Inputs/NameInDirInferred.framework %t/NameInImportInferred.framework +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t.mcp -fimplicit-module-maps -I %S/Inputs -F %S/Inputs -F %t -Wauto-import -verify %s + +// Sanity check that we won't somehow find non-canonical module names or +// modules where we shouldn't search the framework. +// RUN: echo '@import NameInModMap' | not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -F %S/Inputs -F %t -Wauto-import -x objective-c - 2>&1 | FileCheck %s +// RUN: echo '@import NameInDir' | not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -F %S/Inputs -F %t -Wauto-import -x objective-c - 2>&1 | FileCheck %s +// RUN: echo '@import NameInImport' | not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -F %S/Inputs -F %t -Wauto-import -x objective-c - 2>&1 | FileCheck %s +// RUN: echo '@import NameInImportInferred' | not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -F %S/Inputs -F %t -Wauto-import -x objective-c - 2>&1 | FileCheck %s +// CHECK: module '{{.*}}' not found + +// FIXME: We might want to someday lock down framework modules so that these +// name mismatches are disallowed. However, as long as we *don't* prevent them +// it's important that they map correctly to module imports. + +// The module map name doesn't match the directory name. +#import <NameInDir/NameInDir.h> // expected-warning {{import of module 'NameInModMap'}} + +// The name in the import doesn't match the module name. +#import <NameInImport/NameInDir2.h> // expected-warning {{import of module 'NameInDir2'}} +@import NameInDir2; // OK + +// The name in the import doesn't match the module name (inferred framework module). +#import <NameInImportInferred/NameInDirInferred.h> // expected-warning {{import of module 'NameInDirInferred'}} + +@import ImportNameInDir; +#ifdef NAME_IN_DIR +#error NAME_IN_DIR should be undef'd +#endif |