r241258 - [Modules] Be consistent about finding a module for framework headers
Ben Langmuir
blangmuir at apple.com
Thu Jul 2 06:19:49 PDT 2015
Author: benlangmuir
Date: Thu Jul 2 08:19:48 2015
New Revision: 241258
URL: http://llvm.org/viewvc/llvm-project?rev=241258&view=rev
Log:
[Modules] Be consistent about finding a module for framework headers
We use findModuleForHeader() in several places, but in header search we
were not calling it when a framework module didn't show up with the
expected name, which would then lead to unexpected non-modular includes.
Now we will find the module unconditionally for frameworks. For regular
frameworks, we use the spelling of the module name from the module map
file, and for inferred ones we use the canonical directory name.
In the future we might want to lock down framework modules sufficiently
that these name mismatches cannot happen.
rdar://problem/20465870
Added:
cfe/trunk/test/Modules/Inputs/ImportNameInDir.h
cfe/trunk/test/Modules/Inputs/NameInDir.framework/
cfe/trunk/test/Modules/Inputs/NameInDir.framework/Headers/
cfe/trunk/test/Modules/Inputs/NameInDir.framework/Headers/NameInDir.h
cfe/trunk/test/Modules/Inputs/NameInDir.framework/Modules/
cfe/trunk/test/Modules/Inputs/NameInDir.framework/Modules/module.modulemap
cfe/trunk/test/Modules/Inputs/NameInDir2.framework/
cfe/trunk/test/Modules/Inputs/NameInDir2.framework/Headers/
cfe/trunk/test/Modules/Inputs/NameInDir2.framework/Headers/NameInDir2.h
cfe/trunk/test/Modules/Inputs/NameInDir2.framework/Modules/
cfe/trunk/test/Modules/Inputs/NameInDir2.framework/Modules/module.modulemap
cfe/trunk/test/Modules/Inputs/NameInDirInferred.framework/
cfe/trunk/test/Modules/Inputs/NameInDirInferred.framework/Headers/
cfe/trunk/test/Modules/Inputs/NameInDirInferred.framework/Headers/NameInDirInferred.h
cfe/trunk/test/Modules/framework-name.m
Modified:
cfe/trunk/include/clang/Lex/ModuleMap.h
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/lib/Lex/ModuleMap.cpp
cfe/trunk/test/Modules/Inputs/module.map
Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=241258&r1=241257&r2=241258&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
+++ cfe/trunk/include/clang/Lex/ModuleMap.h Thu Jul 2 08:19:48 2015
@@ -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.
///
Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=241258&r1=241257&r2=241258&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Thu Jul 2 08:19:48 2015
@@ -532,9 +532,13 @@ const FileEntry *DirectoryLookup::DoFram
// 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::loadFrameworkModul
// 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::loadFrameworkModul
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);
}
Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=241258&r1=241257&r2=241258&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Thu Jul 2 08:19:48 2015
@@ -583,19 +583,28 @@ static void inferFrameworkLink(Module *M
}
}
-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(
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(
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);
}
}
Added: cfe/trunk/test/Modules/Inputs/ImportNameInDir.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/ImportNameInDir.h?rev=241258&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/ImportNameInDir.h (added)
+++ cfe/trunk/test/Modules/Inputs/ImportNameInDir.h Thu Jul 2 08:19:48 2015
@@ -0,0 +1,4 @@
+#import <NameInDir/NameInDir.h>
+
+// Don't crash.
+#undef NAME_IN_DIR
Added: cfe/trunk/test/Modules/Inputs/NameInDir.framework/Headers/NameInDir.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/NameInDir.framework/Headers/NameInDir.h?rev=241258&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/NameInDir.framework/Headers/NameInDir.h (added)
+++ cfe/trunk/test/Modules/Inputs/NameInDir.framework/Headers/NameInDir.h Thu Jul 2 08:19:48 2015
@@ -0,0 +1,2 @@
+// NameInDir.h
+#define NAME_IN_DIR 1
Added: cfe/trunk/test/Modules/Inputs/NameInDir.framework/Modules/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/NameInDir.framework/Modules/module.modulemap?rev=241258&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/NameInDir.framework/Modules/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/NameInDir.framework/Modules/module.modulemap Thu Jul 2 08:19:48 2015
@@ -0,0 +1,5 @@
+framework module NameInModMap {
+ umbrella header "NameInDir.h"
+ export *
+ module * { export * }
+}
Added: cfe/trunk/test/Modules/Inputs/NameInDir2.framework/Headers/NameInDir2.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/NameInDir2.framework/Headers/NameInDir2.h?rev=241258&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/NameInDir2.framework/Headers/NameInDir2.h (added)
+++ cfe/trunk/test/Modules/Inputs/NameInDir2.framework/Headers/NameInDir2.h Thu Jul 2 08:19:48 2015
@@ -0,0 +1 @@
+// NameInDir2.h
Added: cfe/trunk/test/Modules/Inputs/NameInDir2.framework/Modules/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/NameInDir2.framework/Modules/module.modulemap?rev=241258&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/NameInDir2.framework/Modules/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/NameInDir2.framework/Modules/module.modulemap Thu Jul 2 08:19:48 2015
@@ -0,0 +1,5 @@
+framework module NameInDir2 {
+ umbrella header "NameInDir2.h"
+ export *
+ module * { export * }
+}
Added: cfe/trunk/test/Modules/Inputs/NameInDirInferred.framework/Headers/NameInDirInferred.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/NameInDirInferred.framework/Headers/NameInDirInferred.h?rev=241258&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/NameInDirInferred.framework/Headers/NameInDirInferred.h (added)
+++ cfe/trunk/test/Modules/Inputs/NameInDirInferred.framework/Headers/NameInDirInferred.h Thu Jul 2 08:19:48 2015
@@ -0,0 +1 @@
+// NameInDirInferred.h
Modified: cfe/trunk/test/Modules/Inputs/module.map
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/module.map?rev=241258&r1=241257&r2=241258&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/module.map (original)
+++ cfe/trunk/test/Modules/Inputs/module.map Thu Jul 2 08:19:48 2015
@@ -332,3 +332,7 @@ module DebugModule {
header "DebugModule.h"
}
+module ImportNameInDir {
+ header "ImportNameInDir.h"
+ export *
+}
Added: cfe/trunk/test/Modules/framework-name.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/framework-name.m?rev=241258&view=auto
==============================================================================
--- cfe/trunk/test/Modules/framework-name.m (added)
+++ cfe/trunk/test/Modules/framework-name.m Thu Jul 2 08:19:48 2015
@@ -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'}}
+ at 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'}}
+
+ at import ImportNameInDir;
+#ifdef NAME_IN_DIR
+#error NAME_IN_DIR should be undef'd
+#endif
More information about the cfe-commits
mailing list