jansvoboda11 updated this revision to Diff 397902. jansvoboda11 added a comment.
Rebase on top of extracted patches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113676/new/ https://reviews.llvm.org/D113676 Files: clang/include/clang/Lex/HeaderSearch.h clang/include/clang/Lex/ModuleMap.h clang/lib/Lex/HeaderSearch.cpp clang/lib/Lex/ModuleMap.cpp clang/lib/Serialization/ASTReader.cpp clang/test/Preprocessor/search-path-usage-modules.m clang/test/Preprocessor/search-path-usage.m
Index: clang/test/Preprocessor/search-path-usage.m =================================================================== --- clang/test/Preprocessor/search-path-usage.m +++ clang/test/Preprocessor/search-path-usage.m @@ -128,19 +128,3 @@ // expected-remark-re {{search path used: '{{.*}}/b-missing.hmap'}} #endif #endif - -// Check that search paths with module maps are reported. -// -// RUN: mkdir %t/modulemap_abs -// RUN: sed "s|DIR|%/S/Inputs/search-path-usage|g" \ -// RUN: %S/Inputs/search-path-usage/modulemap_abs/module.modulemap.template \ -// RUN: > %t/modulemap_abs/module.modulemap -// RUN: %clang_cc1 -Eonly %s -Rsearch-path-usage \ -// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules \ -// RUN: -I %t/modulemap_abs \ -// RUN: -I %S/Inputs/search-path-usage/a \ -// RUN: -DMODMAP_ABS -verify -#ifdef MODMAP_ABS -@import b; // \ -// expected-remark-re {{search path used: '{{.*}}/modulemap_abs'}} -#endif Index: clang/test/Preprocessor/search-path-usage-modules.m =================================================================== --- /dev/null +++ clang/test/Preprocessor/search-path-usage-modules.m @@ -0,0 +1,40 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t + +// RUN: %clang_cc1 -Eonly %t/test-both.m -I %t/sp1 -I %t/sp2 -Rsearch-path-usage \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache 2>&1 | FileCheck %t/test-both.m +// RUN: %clang_cc1 -Eonly %t/test-both.m -I %t/sp2 -I %t/sp1 -Rsearch-path-usage \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache 2>&1 | FileCheck %t/test-both.m + +// RUN: %clang_cc1 -Eonly %t/test-first.m -I %t/sp2 -I %t/sp1 -Rsearch-path-usage \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache 2>&1 | FileCheck %t/test-first.m +// RUN: %clang_cc1 -Eonly %t/test-second.m -I %t/sp1 -I %t/sp2 -Rsearch-path-usage \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache 2>&1 | FileCheck %t/test-second.m + +//--- sp1/module.modulemap +module mod1 { header "mod1.h" } +//--- sp1/mod1.h +int module1(); + +//--- sp2/module.modulemap +module mod2 { header "mod2.h" } +//--- sp2/mod2.h +int module2(); + +//--- test-both.m +@import mod1; +@import mod2; +// CHECK: search path used: '{{.*}}/sp1' +// CHECK: search path used: '{{.*}}/sp2' + +//--- test-first.m +@import mod1; +// CHECK-NOT: search path used: '{{.*}}/sp2' +// CHECK: search path used: '{{.*}}/sp1' +// CHECK-NOT: search path used: '{{.*}}/sp2' + +//--- test-second.m +@import mod2; +// CHECK-NOT: search path used: '{{.*}}/sp1' +// CHECK: search path used: '{{.*}}/sp2' +// CHECK-NOT: search path used: '{{.*}}/sp1' Index: clang/lib/Serialization/ASTReader.cpp =================================================================== --- clang/lib/Serialization/ASTReader.cpp +++ clang/lib/Serialization/ASTReader.cpp @@ -3907,7 +3907,7 @@ // An implicitly-loaded module file should have its module listed in some // module map file that we've already loaded. Module *M = - PP.getHeaderSearchInfo().lookupModule(F.ModuleName, F.ImportLoc); + PP.getHeaderSearchInfo().lookupModule(F.ModuleName, SourceLocation()); auto &Map = PP.getHeaderSearchInfo().getModuleMap(); const FileEntry *ModMap = M ? Map.getModuleMapFileForUniquing(M) : nullptr; // Don't emit module relocation error if we have -fno-validate-pch Index: clang/lib/Lex/ModuleMap.cpp =================================================================== --- clang/lib/Lex/ModuleMap.cpp +++ clang/lib/Lex/ModuleMap.cpp @@ -107,8 +107,13 @@ Module *ModuleMap::makeModule(StringRef Name, SourceLocation DefinitionLoc, Module *Parent, bool IsFramework, bool IsExplicit, unsigned VisibilityID) { - return new Module(Name, DefinitionLoc, Parent, IsFramework, IsExplicit, - VisibilityID); + Module *Result = new Module(Name, DefinitionLoc, Parent, IsFramework, + IsExplicit, VisibilityID); + + for (const auto &Callback : Callbacks) + Callback->moduleMapModuleCreated(Result); + + return Result; } Module::ExportDecl Index: clang/lib/Lex/HeaderSearch.cpp =================================================================== --- clang/lib/Lex/HeaderSearch.cpp +++ clang/lib/Lex/HeaderSearch.cpp @@ -36,6 +36,7 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" +#include "llvm/Support/SaveAndRestore.h" #include "llvm/Support/VirtualFileSystem.h" #include <algorithm> #include <cassert> @@ -84,8 +85,19 @@ const LangOptions &LangOpts, const TargetInfo *Target) : HSOpts(std::move(HSOpts)), Diags(Diags), - FileMgr(SourceMgr.getFileManager()), FrameworkMap(64), - ModMap(SourceMgr, Diags, LangOpts, Target, *this) {} + FileMgr(SourceMgr.getFileManager()), CurrentSearchPathIdx(~0U), + FrameworkMap(64), ModMap(SourceMgr, Diags, LangOpts, Target, *this) { + struct MMCallback : ModuleMapCallbacks { + HeaderSearch &HS; + MMCallback(HeaderSearch &HS) : HS(HS) {} + void moduleMapModuleCreated(Module *M) override { + // Module map parsing initiated by header search. + if (HS.CurrentSearchPathIdx != ~0U) + HS.ModuleToSearchDirIdx[M] = HS.CurrentSearchPathIdx; + } + }; + ModMap.addModuleMapCallbacks(std::make_unique<MMCallback>(*this)); +} void HeaderSearch::PrintStats() { llvm::errs() << "\n*** HeaderSearch Stats:\n" @@ -117,6 +129,7 @@ "Directory indices are unordered"); SearchDirs = std::move(dirs); SearchDirsUsage.assign(SearchDirs.size(), false); + ModuleToSearchDirIdx.clear(); AngledDirIdx = angledDirIdx; SystemDirIdx = systemDirIdx; NoCurDirSearch = noCurDirSearch; @@ -124,6 +137,14 @@ //LookupFileCache.clear(); } +/// Increment values in th map greater or equal to the given threshold. +static void incrementValuesGte(llvm::DenseMap<Module *, unsigned> &Map, + unsigned Threshold) { + for (auto &Entry : Map) + if (Entry.second >= Threshold) + ++Entry.second; +} + /// Get a copy of the given map with keys greater or equal to the given /// threshold incremented. static llvm::DenseMap<unsigned, unsigned> @@ -142,6 +163,7 @@ unsigned idx = isAngled ? SystemDirIdx : AngledDirIdx; SearchDirs.insert(SearchDirs.begin() + idx, dir); SearchDirsUsage.insert(SearchDirsUsage.begin() + idx, false); + incrementValuesGte(ModuleToSearchDirIdx, idx); if (!isAngled) AngledDirIdx++; SystemDirIdx++; @@ -288,8 +310,10 @@ 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) + if (Module || !AllowSearch || !HSOpts->ImplicitModuleMaps) { + noteModuleLookupUsage(Module, ImportLoc); return Module; + } StringRef SearchName = ModuleName; Module = lookupModule(ModuleName, SearchName, ImportLoc, @@ -309,6 +333,7 @@ if (!Module && SearchName.consume_back("Private")) Module = lookupModule(ModuleName, SearchName, ImportLoc, AllowExtraModuleMapSearch); + noteModuleLookupUsage(Module, ImportLoc); return Module; } @@ -316,11 +341,12 @@ SourceLocation ImportLoc, bool AllowExtraModuleMapSearch) { Module *Module = nullptr; - unsigned Idx; // Look through the various header search paths to load any available module // maps, searching for a module map that describes this module. - for (Idx = 0; Idx != SearchDirs.size(); ++Idx) { + for (unsigned Idx = 0; Idx != SearchDirs.size(); ++Idx) { + llvm::SaveAndRestore<unsigned> X(CurrentSearchPathIdx, Idx); + if (SearchDirs[Idx].isFramework()) { // Search for or infer a module map for a framework. Here we use // SearchName rather than ModuleName, to permit finding private modules @@ -383,9 +409,6 @@ break; } - if (Module) - noteLookupUsage(Idx, ImportLoc); - return Module; } @@ -721,11 +744,18 @@ noteLookupUsage(HitIdx, Loc); } +void HeaderSearch::noteModuleLookupUsage(Module *M, SourceLocation Loc) { + auto It = ModuleToSearchDirIdx.find(M); + if (It != ModuleToSearchDirIdx.end()) + noteLookupUsage(It->second, Loc); +} + void HeaderSearch::noteLookupUsage(unsigned HitIdx, SourceLocation Loc) { SearchDirsUsage[HitIdx] = true; auto UserEntryIdxIt = SearchDirToHSEntry.find(HitIdx); - if (UserEntryIdxIt != SearchDirToHSEntry.end()) + // TODO: Avoid emitting duplicates. + if (UserEntryIdxIt != SearchDirToHSEntry.end() && Loc.isValid()) Diags.Report(Loc, diag::remark_pp_search_path_usage) << HSOpts->UserEntries[UserEntryIdxIt->second].Path; } @@ -1777,6 +1807,8 @@ if (HSOpts->ImplicitModuleMaps) { // Load module maps for each of the header search directories. for (unsigned Idx = 0, N = SearchDirs.size(); Idx != N; ++Idx) { + llvm::SaveAndRestore<unsigned> X(CurrentSearchPathIdx, Idx); + bool IsSystem = SearchDirs[Idx].isSystemHeaderDirectory(); if (SearchDirs[Idx].isFramework()) { std::error_code EC; @@ -1829,6 +1861,8 @@ // Load module maps for each of the header search directories. for (unsigned Idx = 0, N = SearchDirs.size(); Idx != N; ++Idx) { + llvm::SaveAndRestore<unsigned> X(CurrentSearchPathIdx, Idx); + // We only care about normal header directories. if (!SearchDirs[Idx].isNormalDir()) { continue; Index: clang/include/clang/Lex/ModuleMap.h =================================================================== --- clang/include/clang/Lex/ModuleMap.h +++ clang/include/clang/Lex/ModuleMap.h @@ -70,6 +70,11 @@ /// \param Header The umbrella header to collect. virtual void moduleMapAddUmbrellaHeader(FileManager *FileMgr, const FileEntry *Header) {} + + /// Called when new module is created. + /// + /// \param M The newly created module. + virtual void moduleMapModuleCreated(Module *M) {} }; class ModuleMap { Index: clang/include/clang/Lex/HeaderSearch.h =================================================================== --- clang/include/clang/Lex/HeaderSearch.h +++ clang/include/clang/Lex/HeaderSearch.h @@ -181,6 +181,12 @@ /// Whether the DirectoryLookup at the corresponding index in SearchDirs has /// been successfully used to lookup a file. std::vector<bool> SearchDirsUsage; + /// While iterating through `SearchDirs`, this is the current index. + unsigned CurrentSearchPathIdx; + /// Mapping from module to index of the search directory that discovered its + /// module map file. + llvm::DenseMap<Module *, unsigned> ModuleToSearchDirIdx; + unsigned AngledDirIdx = 0; unsigned SystemDirIdx = 0; bool NoCurDirSearch = false; @@ -704,6 +710,7 @@ /// using the search path at index `HitIdx`. void cacheLookupSuccess(LookupFileCacheInfo &CacheLookup, unsigned HitIdx, SourceLocation IncludeLoc); + void noteModuleLookupUsage(Module *M, SourceLocation Loc); /// Note that a lookup at the given include location was successful using the /// search path at index `HitIdx`. void noteLookupUsage(unsigned HitIdx, SourceLocation IncludeLoc);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits