Author: bruno Date: Wed Jul 18 16:21:19 2018 New Revision: 337430 URL: http://llvm.org/viewvc/llvm-project?rev=337430&view=rev Log: Reapply r336660: [Modules] 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 Added: cfe/trunk/test/Modules/Inputs/autoload-subdirectory/a.h cfe/trunk/test/Modules/Inputs/autoload-subdirectory/b.h cfe/trunk/test/Modules/Inputs/autoload-subdirectory/c.h cfe/trunk/test/Modules/Inputs/autoload-subdirectory/include/module.modulemap cfe/trunk/test/Modules/Inputs/autoload-subdirectory/module.modulemap cfe/trunk/test/Modules/autoload-subdirectory.cpp Modified: cfe/trunk/include/clang/Lex/HeaderSearch.h cfe/trunk/lib/Frontend/CompilerInstance.cpp cfe/trunk/lib/Lex/HeaderSearch.cpp cfe/trunk/lib/Serialization/ASTReader.cpp Modified: cfe/trunk/include/clang/Lex/HeaderSearch.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearch.h?rev=337430&r1=337429&r2=337430&view=diff ============================================================================== --- cfe/trunk/include/clang/Lex/HeaderSearch.h (original) +++ cfe/trunk/include/clang/Lex/HeaderSearch.h Wed Jul 18 16:21:19 2018 @@ -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. Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=337430&r1=337429&r2=337430&view=diff ============================================================================== --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original) +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Wed Jul 18 16:21:19 2018 @@ -1653,8 +1653,10 @@ CompilerInstance::loadModule(SourceLocat // 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(SourceLocat 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(SourceLocat 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(SourceLocat 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) { Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=337430&r1=337429&r2=337430&view=diff ============================================================================== --- cfe/trunk/lib/Lex/HeaderSearch.cpp (original) +++ cfe/trunk/lib/Lex/HeaderSearch.cpp Wed Jul 18 16:21:19 2018 @@ -198,14 +198,15 @@ std::string HeaderSearch::getCachedModul 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(Strin // 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(Strin 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); Modified: cfe/trunk/lib/Serialization/ASTReader.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=337430&r1=337429&r2=337430&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTReader.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Jul 18 16:21:19 2018 @@ -2626,7 +2626,9 @@ ASTReader::ReadControlBlock(ModuleFile & "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. Added: cfe/trunk/test/Modules/Inputs/autoload-subdirectory/a.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/autoload-subdirectory/a.h?rev=337430&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/autoload-subdirectory/a.h (added) +++ cfe/trunk/test/Modules/Inputs/autoload-subdirectory/a.h Wed Jul 18 16:21:19 2018 @@ -0,0 +1,9 @@ +#include "b.h" + +class foo { + int x, y; + +public: + foo(){}; + ~foo(){}; +}; Added: cfe/trunk/test/Modules/Inputs/autoload-subdirectory/b.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/autoload-subdirectory/b.h?rev=337430&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/autoload-subdirectory/b.h (added) +++ cfe/trunk/test/Modules/Inputs/autoload-subdirectory/b.h Wed Jul 18 16:21:19 2018 @@ -0,0 +1 @@ +class bar {}; Added: cfe/trunk/test/Modules/Inputs/autoload-subdirectory/c.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/autoload-subdirectory/c.h?rev=337430&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/autoload-subdirectory/c.h (added) +++ cfe/trunk/test/Modules/Inputs/autoload-subdirectory/c.h Wed Jul 18 16:21:19 2018 @@ -0,0 +1,7 @@ +class nyan { + bool x, y; + +public: + nyan(){}; + ~nyan(){}; +}; Added: cfe/trunk/test/Modules/Inputs/autoload-subdirectory/include/module.modulemap URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/autoload-subdirectory/include/module.modulemap?rev=337430&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/autoload-subdirectory/include/module.modulemap (added) +++ cfe/trunk/test/Modules/Inputs/autoload-subdirectory/include/module.modulemap Wed Jul 18 16:21:19 2018 @@ -0,0 +1,3 @@ +module a { header "a.h" } +module b { header "b.h" } +module c { header "c.h" } Added: cfe/trunk/test/Modules/Inputs/autoload-subdirectory/module.modulemap URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/autoload-subdirectory/module.modulemap?rev=337430&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/autoload-subdirectory/module.modulemap (added) +++ cfe/trunk/test/Modules/Inputs/autoload-subdirectory/module.modulemap Wed Jul 18 16:21:19 2018 @@ -0,0 +1,3 @@ +module a { header "a.h" } +module b { header "b.h" } +module c { header "c.h" } Added: cfe/trunk/test/Modules/autoload-subdirectory.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/autoload-subdirectory.cpp?rev=337430&view=auto ============================================================================== --- cfe/trunk/test/Modules/autoload-subdirectory.cpp (added) +++ cfe/trunk/test/Modules/autoload-subdirectory.cpp Wed Jul 18 16:21:19 2018 @@ -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; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
