[PATCH] D121295: [clang][deps] Modules don't contribute to search path usage

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

To reduce the number of modules we build in explicit builds (which use strict context hash), we prune unused header search paths. This essentially merges parts of the dependency graph.

Determining whether a search path was used to discover a module (through implicit module maps) proved to be somewhat complicated. Initial support landed in D102923 <https://reviews.llvm.org/D102923>, while D113676 <https://reviews.llvm.org/D113676> attempts to fix some bugs.

However, now that we don't use implicit module maps in explicit builds (since D120465 <https://reviews.llvm.org/D120465>), we don't need to consider such search paths as used anymore. Modules are no longer discovered through the header search mechanism, so we can drop such search paths (provided they are not needed for other reasons).

This patch removes whatever support for detecting such usage we had, since it's buggy and not required anymore.

Depends on D120465 <https://reviews.llvm.org/D120465>.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121295

Files:
  clang/lib/Lex/HeaderSearch.cpp
  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
@@ -129,7 +129,7 @@
 #endif
 #endif
 
-// Check that search paths with module maps are reported.
+// Check that search paths with module maps are NOT reported.
 //
 // RUN: mkdir %t/modulemap_abs
 // RUN: sed "s|DIR|%/S/Inputs/search-path-usage|g"                            \
@@ -142,5 +142,5 @@
 // RUN:   -DMODMAP_ABS -verify
 #ifdef MODMAP_ABS
 @import b; // \
-// expected-remark-re {{search path used: '{{.*}}/modulemap_abs'}}
+// expected-no-diagnostics
 #endif
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -299,20 +299,19 @@
                                    SourceLocation ImportLoc,
                                    bool AllowExtraModuleMapSearch) {
   Module *Module = nullptr;
-  SearchDirIterator It = nullptr;
 
   // Look through the various header search paths to load any available module
   // maps, searching for a module map that describes this module.
-  for (It = search_dir_begin(); It != search_dir_end(); ++It) {
-    if (It->isFramework()) {
+  for (DirectoryLookup Dir : search_dir_range()) {
+    if (Dir.isFramework()) {
       // Search for or infer a module map for a framework. Here we use
       // SearchName rather than ModuleName, to permit finding private modules
       // named FooPrivate in buggy frameworks named Foo.
       SmallString<128> FrameworkDirName;
-      FrameworkDirName += It->getFrameworkDir()->getName();
+      FrameworkDirName += Dir.getFrameworkDir()->getName();
       llvm::sys::path::append(FrameworkDirName, SearchName + ".framework");
       if (auto FrameworkDir = FileMgr.getDirectory(FrameworkDirName)) {
-        bool IsSystem = It->getDirCharacteristic() != SrcMgr::C_User;
+        bool IsSystem = Dir.getDirCharacteristic() != SrcMgr::C_User;
         Module = loadFrameworkModule(ModuleName, *FrameworkDir, IsSystem);
         if (Module)
           break;
@@ -322,12 +321,12 @@
     // FIXME: Figure out how header maps and module maps will work together.
 
     // Only deal with normal search directories.
-    if (!It->isNormalDir())
+    if (!Dir.isNormalDir())
       continue;
 
-    bool IsSystem = It->isSystemHeaderDirectory();
+    bool IsSystem = Dir.isSystemHeaderDirectory();
     // Search for a module map file in this directory.
-    if (loadModuleMapFile(It->getDir(), IsSystem,
+    if (loadModuleMapFile(Dir.getDir(), IsSystem,
                           /*IsFramework*/false) == LMM_NewlyLoaded) {
       // We just loaded a module map file; check whether the module is
       // available now.
@@ -339,7 +338,7 @@
     // Search for a module map in a subdirectory with the same name as the
     // module.
     SmallString<128> NestedModuleMapDirName;
-    NestedModuleMapDirName = It->getDir()->getName();
+    NestedModuleMapDirName = Dir.getDir()->getName();
     llvm::sys::path::append(NestedModuleMapDirName, ModuleName);
     if (loadModuleMapFile(NestedModuleMapDirName, IsSystem,
                           /*IsFramework*/false) == LMM_NewlyLoaded){
@@ -351,13 +350,13 @@
 
     // If we've already performed the exhaustive search for module maps in this
     // search directory, don't do it again.
-    if (It->haveSearchedAllModuleMaps())
+    if (Dir.haveSearchedAllModuleMaps())
       continue;
 
     // Load all module maps in the immediate subdirectories of this search
     // directory if ModuleName was from @import.
     if (AllowExtraModuleMapSearch)
-      loadSubdirectoryModuleMaps(*It);
+      loadSubdirectoryModuleMaps(Dir);
 
     // Look again for the module.
     Module = ModMap.findModule(ModuleName);
@@ -365,9 +364,6 @@
       break;
   }
 
-  if (Module)
-    noteLookupUsage(It.Idx, ImportLoc);
-
   return Module;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D121295.414106.patch
Type: text/x-patch
Size: 4007 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220309/a550d776/attachment-0001.bin>