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

Reply via email to