EricWF updated this revision to Diff 101307.
EricWF edited the summary of this revision.
EricWF added reviewers: rtrieu, dexonsmith, dblaikie, aprantl, v.g.vassilev.
EricWF added a comment.
- Only allow the top level header search directories to be search for
`module.modulemap` files when loading modules in `ReadAST.cpp`.
Perhaps it would be better to figure out what top-level search directory the
module was originally built under, and then remove the prefix of the top-level
directory. Then we could allow the search to look under matching subdirectories
of other top-level search paths. However this seems like it could be incorrect
as well.
@zygoloid: Any thoughts?
https://reviews.llvm.org/D32111
Files:
include/clang/Lex/HeaderSearch.h
lib/Lex/HeaderSearch.cpp
lib/Serialization/ASTReader.cpp
test/Modules/Inputs/PR31905/my-project/module.modulemap
test/Modules/Inputs/PR31905/my-project/my-header.h
test/Modules/pr31905.cpp
Index: test/Modules/pr31905.cpp
===================================================================
--- /dev/null
+++ test/Modules/pr31905.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cp -R %S/Inputs/PR31905 %t/other-include
+// RUN: %clang_cc1 -std=c++11 -I%S/Inputs/PR31905/ -I%t/other-include -fmodules -fmodules-local-submodule-visibility \
+// RUN: -fimplicit-module-maps -fmodules-cache-path=%t -verify %s
+
+#include "my-project/my-header.h"
+
+int main() {} // expected-no-diagnostics
Index: test/Modules/Inputs/PR31905/my-project/my-header.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/PR31905/my-project/my-header.h
@@ -0,0 +1,4 @@
+#ifndef MY_HEADER_H
+#define MY_HEADER_H
+#include <stddef.h>
+#endif
Index: test/Modules/Inputs/PR31905/my-project/module.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/PR31905/my-project/module.modulemap
@@ -0,0 +1,3 @@
+module my_project {
+ header "my-header.h"
+}
Index: lib/Serialization/ASTReader.cpp
===================================================================
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -2551,7 +2551,9 @@
"MODULE_DIRECTORY found before MODULE_NAME");
// If we've already loaded a module map file covering this module, we may
// have a better path for it (relative to the current build).
- Module *M = PP.getHeaderSearchInfo().lookupModule(F.ModuleName);
+ Module *M = PP.getHeaderSearchInfo().lookupModule(
+ F.ModuleName,
+ /*AllowSearch*/ true, /*SearchTopLevelOnly*/ true);
if (M && M->Directory) {
// If we're implicitly loading a module, the base directory can't
// change between the build and use.
Index: lib/Lex/HeaderSearch.cpp
===================================================================
--- lib/Lex/HeaderSearch.cpp
+++ lib/Lex/HeaderSearch.cpp
@@ -191,14 +191,15 @@
return Result.str().str();
}
-Module *HeaderSearch::lookupModule(StringRef ModuleName, bool AllowSearch) {
+Module *HeaderSearch::lookupModule(StringRef ModuleName, bool AllowSearch,
+ bool SearchTopLevelOnly) {
// Look in the module map to determine if there is a module by this name.
Module *Module = ModMap.findModule(ModuleName);
if (Module || !AllowSearch || !HSOpts->ImplicitModuleMaps)
return Module;
StringRef SearchName = ModuleName;
- Module = lookupModule(ModuleName, SearchName);
+ Module = lookupModule(ModuleName, SearchName, SearchTopLevelOnly);
// The facility for "private modules" -- adjacent, optional module maps named
// module.private.modulemap that are supposed to define private submodules --
@@ -208,11 +209,12 @@
// Foo.framework, when we previously looked and failed to find a
// FooPrivate.framework.
if (!Module && SearchName.consume_back("Private"))
- Module = lookupModule(ModuleName, SearchName);
+ Module = lookupModule(ModuleName, SearchName, SearchTopLevelOnly);
return Module;
}
-Module *HeaderSearch::lookupModule(StringRef ModuleName, StringRef SearchName) {
+Module *HeaderSearch::lookupModule(StringRef ModuleName, StringRef SearchName,
+ bool SearchTopLevelOnly) {
Module *Module = nullptr;
// Look through the various header search paths to load any available module
@@ -267,7 +269,7 @@
// If we've already performed the exhaustive search for module maps in this
// search directory, don't do it again.
- if (SearchDirs[Idx].haveSearchedAllModuleMaps())
+ if (SearchTopLevelOnly || SearchDirs[Idx].haveSearchedAllModuleMaps())
continue;
// Load all module maps in the immediate subdirectories of this search
Index: include/clang/Lex/HeaderSearch.h
===================================================================
--- include/clang/Lex/HeaderSearch.h
+++ include/clang/Lex/HeaderSearch.h
@@ -503,8 +503,12 @@
/// search directories to produce a module definition. If not, this lookup
/// will only return an already-known module.
///
+ /// \param SearchTopLevelOnly Whether we are allowed to search the
+ /// subdirectories to produce a module definition.
+ ///
/// \returns The module with the given name.
- Module *lookupModule(StringRef ModuleName, bool AllowSearch = true);
+ Module *lookupModule(StringRef ModuleName, bool AllowSearch = true,
+ bool SearchTopLevelOnly = false);
/// \brief Try to find a module map file in the given directory, returning
/// \c nullptr if none is found.
@@ -560,7 +564,6 @@
void loadTopLevelSystemModules();
private:
-
/// \brief Lookup a module with the given module name and search-name.
///
/// \param ModuleName The name of the module we're looking for.
@@ -570,8 +573,12 @@
/// but for compatibility with some buggy frameworks, additional attempts
/// may be made to find the module under a related-but-different search-name.
///
+ /// \param SearchTopLevelOnly Whether we are allowed to search the
+ /// subdirectories to produce a module definition.
+ ///
/// \returns The module named ModuleName.
- Module *lookupModule(StringRef ModuleName, StringRef SearchName);
+ Module *lookupModule(StringRef ModuleName, StringRef SearchName,
+ bool SearchTopLevelOnly);
/// \brief Retrieve a module with the given name, which may be part of the
/// given framework.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
