[clang][modules][deps] Avoid checks for relocated modules

[jansvoboda11]

Summary

Currently, ASTReader performs some checks to diagnose relocated modules. This can add quite a bit of overhead to the scanner: it requires looking up, parsing and resolving module maps for all transitively loaded module files (and all the module maps encountered in the search paths on the way). Most of those checks are not really useful in the scanner anyway, since it uses strict context hash and immutable filesystem, which prevent those scenarios in the first place.

This can speed up scanning by up to 30%.

Depends on D150292.

[benlangmuir]

With my naming comment addressed, LGTM

clang/include/clang/Lex/PreprocessorOptions.h

73

Suggestion: CheckRelocatableModules. The current name sounds like a compiler feature rather than controlling diagnostics.

[jansvoboda11]

Rename the new preprocessor option, fix failing test

clang/include/clang/Lex/PreprocessorOptions.h

 /// PreprocessorOptions - This class is used for passing the various options
 /// used in preprocessor initialization to InitializePreprocessor().
 class PreprocessorOptions {
 public:
   std::vector<std::pair<std::string, bool/*isUndef*/>> Macros;
   std::vector<std::string> Includes;
   std::vector<std::string> MacroIncludes;
 
+  /// Perform extra checks when loading PCM files for mutable file systems.
+  bool ModulesCheckRelocated = true;
+
   /// Initialize the preprocessor with the compiler and target specific
   /// predefines.
   bool UsePredefines = true;
 
   /// Whether we should maintain a detailed record of all macro
   /// definitions and expansions.
   bool DetailedRecord = false;
 

clang/lib/Serialization/ASTReader.cpp

case MODULE_NAME:
       break;
 
     case MODULE_DIRECTORY: {
       // Save the BaseDirectory as written in the PCM for computing the module
       // filename for the ModuleCache.
       BaseDirectoryAsWritten = Blob;
       assert(!F.ModuleName.empty() &&
              "MODULE_DIRECTORY found before MODULE_NAME");
+      F.BaseDirectory = std::string(Blob);
+      if (!PP.getPreprocessorOpts().ModulesCheckRelocated)
+        break;
       // 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, SourceLocation(), /*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.
         // Don't emit module relocation error if we have -fno-validate-pch
         if (!bool(PP.getPreprocessorOpts().DisablePCHOrModuleValidation &
                   DisableValidationForModuleKind::Module) &&
             F.Kind != MK_ExplicitModule && F.Kind != MK_PrebuiltModule) {
           auto BuildDir = PP.getFileManager().getDirectory(Blob);
           if (!BuildDir || *BuildDir != M->Directory) {
             if (!canRecoverFromOutOfDate(F.FileName, ClientLoadCapabilities))
               Diag(diag::err_imported_module_relocated)
                   << F.ModuleName << Blob << M->Directory->getName();
             return OutOfDate;
           }
         }
         F.BaseDirectory = std::string(M->Directory->getName());
-      } else {
-        F.BaseDirectory = std::string(Blob);
       }
       break;
     }
 
     case MODULE_MAP_FILE:
       if (ASTReadResult Result =
               ReadModuleMapFileBlock(Record, F, ImportedBy, ClientLoadCapabilities))
         return Result;
ASTReader::ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F,
   F.ModuleMapPath = ReadPath(F, Record, Idx);
 
   // Try to resolve ModuleName in the current header search context and
   // verify that it is found in the same module map file as we saved. If the
   // top-level AST file is a main file, skip this check because there is no
   // usable header search context.
   assert(!F.ModuleName.empty() &&
          "MODULE_NAME should come before MODULE_MAP_FILE");
-  if (F.Kind == MK_ImplicitModule && ModuleMgr.begin()->Kind != MK_MainFile) {
+  if (PP.getPreprocessorOpts().ModulesCheckRelocated &&
+      F.Kind == MK_ImplicitModule && ModuleMgr.begin()->Kind != MK_MainFile) {
     // 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);
     auto &Map = PP.getHeaderSearchInfo().getModuleMap();
     OptionalFileEntryRef ModMap =
         M ? Map.getModuleMapFileForUniquing(M) : std::nullopt;
     // Don't emit module relocation error if we have -fno-validate-pch

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
 
     // Consider different header search and diagnostic options to create
     // different modules. This avoids the unsound aliasing of module PCMs.
     //
     // TODO: Implement diagnostic bucketing to reduce the impact of strict
     // context hashing.
     ScanInstance.getHeaderSearchOpts().ModulesStrictContextHash = true;
 
+    // Avoid some checks and module map parsing when loading PCM files.
+    ScanInstance.getPreprocessorOpts().ModulesCheckRelocated = false;
+
     std::unique_ptr<FrontendAction> Action;
 
     if (ModuleName)
       Action = std::make_unique<GetDependenciesByModuleNameAction>(*ModuleName);
     else
       Action = std::make_unique<ReadPCHAndPreprocessAction>();
 
     const bool Result = ScanInstance.ExecuteAction(*Action);

clang/test/ClangScanDeps/header-search-pruning-transitive.c

 // CHECK-NEXT:           "module-name": "Y"
 // CHECK-NEXT:         }
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
 // CHECK-NEXT:       "command-line": [
 // CHECK:            ],
 // CHECK-NEXT:       "context-hash": "[[HASH_X:.*]]",
 // CHECK-NEXT:       "file-deps": [
-// CHECK-NEXT:         "[[PREFIX]]/./X.h",
-// CHECK-NEXT:         "[[PREFIX]]/./module.modulemap"
+// CHECK-NEXT:         "[[PREFIX]]/X.h",
+// CHECK-NEXT:         "[[PREFIX]]/module.modulemap"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "name": "X"
 // CHECK-NEXT:     },
 // CHECK-NEXT:     {
 // CHECK-NEXT:       "clang-module-deps": [],
 // CHECK-NEXT:       "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
 // CHECK-NEXT:       "command-line": [
 // CHECK:            ],
 // CHECK-NEXT:       "context-hash": "[[HASH_Y_WITH_A]]",
 // CHECK-NEXT:       "file-deps": [
-// CHECK-NEXT:         "[[PREFIX]]/./Y.h",
-// CHECK-NEXT:         "[[PREFIX]]/./a/a.h",
-// CHECK-NEXT:         "[[PREFIX]]/./begin/begin.h",
-// CHECK-NEXT:         "[[PREFIX]]/./end/end.h",
-// CHECK-NEXT:         "[[PREFIX]]/./module.modulemap"
+// CHECK-NEXT:         "[[PREFIX]]/Y.h",
+// CHECK-NEXT:         "[[PREFIX]]/a/a.h",
+// CHECK-NEXT:         "[[PREFIX]]/begin/begin.h",
+// CHECK-NEXT:         "[[PREFIX]]/end/end.h",
+// CHECK-NEXT:         "[[PREFIX]]/module.modulemap"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "name": "Y"
 // CHECK-NEXT:     }
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "translation-units": [
 // CHECK-NEXT:     {
 // CHECK:            "clang-context-hash": "{{.*}}",
 // CHECK-NEXT:       "clang-module-deps": [

 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
 // CHECK-NEXT:       "command-line": [
 // CHECK:            ],
 // Here is the actual check that this module X (which imports different version of Y)
 // also has a different context hash from the first version of module X.
 // CHECK-NOT:        "context-hash": "[[HASH_X]]",
 // CHECK:            "file-deps": [
-// CHECK-NEXT:         "[[PREFIX]]/./X.h",
-// CHECK-NEXT:         "[[PREFIX]]/./module.modulemap"
+// CHECK-NEXT:         "[[PREFIX]]/X.h",
+// CHECK-NEXT:         "[[PREFIX]]/module.modulemap"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "name": "X"
 // CHECK-NEXT:     },
 // CHECK-NEXT:     {
 // CHECK-NEXT:       "clang-module-deps": [],
 // CHECK-NEXT:       "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
 // CHECK-NEXT:       "command-line": [
 // CHECK:            ],
 // CHECK-NEXT:       "context-hash": "[[HASH_Y_WITHOUT_A]]",
 // CHECK-NEXT:       "file-deps": [
-// CHECK-NEXT:         "[[PREFIX]]/./Y.h",
-// CHECK-NEXT:         "[[PREFIX]]/./begin/begin.h",
-// CHECK-NEXT:         "[[PREFIX]]/./end/end.h",
-// CHECK-NEXT:         "[[PREFIX]]/./module.modulemap"
+// CHECK-NEXT:         "[[PREFIX]]/Y.h",
+// CHECK-NEXT:         "[[PREFIX]]/begin/begin.h",
+// CHECK-NEXT:         "[[PREFIX]]/end/end.h",
+// CHECK-NEXT:         "[[PREFIX]]/module.modulemap"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "name": "Y"
 // CHECK-NEXT:     }
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "translation-units": [
 // CHECK-NEXT:     {
 // CHECK:            "clang-context-hash": "{{.*}}",
 // CHECK-NEXT:       "clang-module-deps": [