maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[modules] Fix 37878; Autoload subdirectory modulemaps with specific LangOpts 

Summary:
Reproducer and errors:
https://bugs.llvm.org/show_bug.cgi?id=37878

lookupModule was falling back to loadSubdirectoryModuleMaps when it couldn't
find ModuleName in (proper) search paths. This was causing iteration over all
files in the search path subdirectories for example "/usr/include/foobar" in
bugzilla case.

Users don't expect Clang to load modulemaps in subdirectories implicitly, and
also the disk access is not cheap.

if (AllowExtraModuleMapSearch) true with ObjC with @import ModuleName.

Reviewers: rsmith, aprantl, bruno

Subscribers: cfe-commits, teemperor, v.g.vassilev

Differential Revision: https://reviews.llvm.org/D48367

llvm-svn: 336660
This commit is contained in:
Yuka Takahashi 2018-07-10 12:17:34 +00:00
parent 1ffeb5d7f0
commit a15364152c
10 changed files with 66 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
clang/include/clang/Lex/HeaderSearch.h
View File

@ -529,8 +529,12 @@ public:
/// search directories to produce a module definition. If not, this lookup
/// will only return an already-known module.
///
/// \param AllowExtraModuleMapSearch Whether we allow to search modulemaps
/// in subdirectories.
///
/// \returns The module with the given name.
Module *lookupModule(StringRef ModuleName, bool AllowSearch = true);
Module *lookupModule(StringRef ModuleName, bool AllowSearch = true,
bool AllowExtraModuleMapSearch = false);
/// Try to find a module map file in the given directory, returning
/// \c nullptr if none is found.
@ -595,8 +599,12 @@ private:
/// but for compatibility with some buggy frameworks, additional attempts
/// may be made to find the module under a related-but-different search-name.
///
/// \param AllowExtraModuleMapSearch Whether we allow to search modulemaps
/// in subdirectories.
///
/// \returns The module named ModuleName.
Module *lookupModule(StringRef ModuleName, StringRef SearchName);
Module *lookupModule(StringRef ModuleName, StringRef SearchName,
bool AllowExtraModuleMapSearch = false);
/// Retrieve a module with the given name, which may be part of the
/// given framework.

15
clang/lib/Frontend/CompilerInstance.cpp
View File

@ -1653,8 +1653,10 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
// Retrieve the cached top-level module.
Module = Known->second;
} else if (ModuleName == getLangOpts().CurrentModule) {
// This is the module we're building.
Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
// This is the module we're building.
Module = PP->getHeaderSearchInfo().lookupModule(
ModuleName, /*AllowSearch*/ true,
/*AllowExtraModuleMapSearch*/ !IsInclusionDirective);
/// FIXME: perhaps we should (a) look for a module using the module name
// to file map (PrebuiltModuleFiles) and (b) diagnose if still not found?
//if (Module == nullptr) {
@ -1666,7 +1668,8 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
Known = KnownModules.insert(std::make_pair(Path[0].first, Module)).first;
} else {
// Search for a module with the given name.
Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
Module = PP->getHeaderSearchInfo().lookupModule(ModuleName, true,
!IsInclusionDirective);
HeaderSearchOptions &HSOpts =
PP->getHeaderSearchInfo().getHeaderSearchOpts();
@ -1743,7 +1746,8 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
ImportLoc, ARRFlags)) {
case ASTReader::Success: {
if (Source != ModuleCache && !Module) {
Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
Module = PP->getHeaderSearchInfo().lookupModule(ModuleName, true,
!IsInclusionDirective);
if (!Module || !Module->getASTFile() ||
FileMgr->getFile(ModuleFileName) != Module->getASTFile()) {
// Error out if Module does not refer to the file in the prebuilt
@ -1874,7 +1878,8 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
PrivateModule, PP->getIdentifierInfo(Module->Name)->getTokenID());
PrivPath.push_back(std::make_pair(&II, Path[0].second));
if (PP->getHeaderSearchInfo().lookupModule(PrivateModule))
if (PP->getHeaderSearchInfo().lookupModule(PrivateModule, true,
!IsInclusionDirective))
Sub =
loadModule(ImportLoc, PrivPath, Visibility, IsInclusionDirective);
if (Sub) {

17
clang/lib/Lex/HeaderSearch.cpp
View File

@ -198,14 +198,15 @@ std::string HeaderSearch::getCachedModuleFileName(StringRef ModuleName,
return Result.str().str();
}
Module *HeaderSearch::lookupModule(StringRef ModuleName, bool AllowSearch) {
Module *HeaderSearch::lookupModule(StringRef ModuleName, bool AllowSearch,
bool AllowExtraModuleMapSearch) {
// 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, AllowExtraModuleMapSearch);
// The facility for "private modules" -- adjacent, optional module maps named
// module.private.modulemap that are supposed to define private submodules --
@ -216,13 +217,14 @@ Module *HeaderSearch::lookupModule(StringRef ModuleName, bool AllowSearch) {
// could force building unwanted dependencies into the parent module and cause
// dependency cycles.
if (!Module && SearchName.consume_back("_Private"))
Module = lookupModule(ModuleName, SearchName);
Module = lookupModule(ModuleName, SearchName, AllowExtraModuleMapSearch);
if (!Module && SearchName.consume_back("Private"))
Module = lookupModule(ModuleName, SearchName);
Module = lookupModule(ModuleName, SearchName, AllowExtraModuleMapSearch);
return Module;
}
Module *HeaderSearch::lookupModule(StringRef ModuleName, StringRef SearchName) {
Module *HeaderSearch::lookupModule(StringRef ModuleName, StringRef SearchName,
bool AllowExtraModuleMapSearch) {
Module *Module = nullptr;
// Look through the various header search paths to load any available module
@ -281,8 +283,9 @@ Module *HeaderSearch::lookupModule(StringRef ModuleName, StringRef SearchName) {
continue;
// Load all module maps in the immediate subdirectories of this search
// directory.
loadSubdirectoryModuleMaps(SearchDirs[Idx]);
// directory if ModuleName was from @import.
if (AllowExtraModuleMapSearch)
loadSubdirectoryModuleMaps(SearchDirs[Idx]);
// Look again for the module.
Module = ModMap.findModule(ModuleName);

4
clang/lib/Serialization/ASTReader.cpp
View File

@ -2626,7 +2626,9 @@ ASTReader::ReadControlBlock(ModuleFile &F,
"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,
/*AllowExtraModuleMapSearch*/ true);
if (M && M->Directory) {
// If we're implicitly loading a module, the base directory can't
// change between the build and use.

9
clang/test/Modules/Inputs/autoload-subdirectory/a.h Normal file
View File

@ -0,0 +1,9 @@
#include "b.h"
class foo {
int x, y;
public:
foo(){};
~foo(){};
};

1
clang/test/Modules/Inputs/autoload-subdirectory/b.h Normal file
View File

@ -0,0 +1 @@
class bar {};

7
clang/test/Modules/Inputs/autoload-subdirectory/c.h Normal file
View File

@ -0,0 +1,7 @@
class nyan {
bool x, y;
public:
nyan(){};
~nyan(){};
};

3
clang/test/Modules/Inputs/autoload-subdirectory/include/module.modulemap Normal file
View File

@ -0,0 +1,3 @@
module a { header "a.h" }
module b { header "b.h" }
module c { header "c.h" }

3
clang/test/Modules/Inputs/autoload-subdirectory/module.modulemap Normal file
View File

@ -0,0 +1,3 @@
module a { header "a.h" }
module b { header "b.h" }
module c { header "c.h" }

10
clang/test/Modules/autoload-subdirectory.cpp Normal file
View File

@ -0,0 +1,10 @@
// RUN: %clang_cc1 -fmodules -fmodule-name=Foo -I %S/Inputs/autoload-subdirectory/ %s -verify
// expected-no-diagnostics
#include "a.h"
#import "c.h"
int main() {
foo neko;
return 0;
}