Skip to content

[clang] Expose normalized module cache path in HeaderSearch#185746

Merged
jansvoboda11 merged 3 commits intollvm:mainfrom
jansvoboda11:separate-module-cache-path
Mar 12, 2026
Merged

[clang] Expose normalized module cache path in HeaderSearch#185746
jansvoboda11 merged 3 commits intollvm:mainfrom
jansvoboda11:separate-module-cache-path

Conversation

@jansvoboda11
Copy link
Contributor

Previously, the normalized module cache path was only accessible via HeaderSearch::getSpecificModuleCachePath() which may or may not also contain the context hash. Clients would need to parse the result to learn the normalized module cache path. What ASTWriter does instead is normalize the as-written module cache path redundantly.

Instead, this PR exposes the normalized module cache path in the HeaderSearch interface and moves the computation of specific module cache path into the clangLex library.

This is motivated by another patch that would've needed to redundantly perform the module cache path canonicalization or parse the specific module cache path.

@jansvoboda11 jansvoboda11 requested a review from Bigcheese March 10, 2026 20:25
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Mar 10, 2026
@llvmbot
Copy link
Member

llvmbot commented Mar 10, 2026

@llvm/pr-subscribers-clang-modules

Author: Jan Svoboda (jansvoboda11)

Changes

Previously, the normalized module cache path was only accessible via HeaderSearch::getSpecificModuleCachePath() which may or may not also contain the context hash. Clients would need to parse the result to learn the normalized module cache path. What ASTWriter does instead is normalize the as-written module cache path redundantly.

Instead, this PR exposes the normalized module cache path in the HeaderSearch interface and moves the computation of specific module cache path into the clangLex library.

This is motivated by another patch that would've needed to redundantly perform the module cache path canonicalization or parse the specific module cache path.


Patch is 20.56 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/185746.diff

11 Files Affected:

  • (modified) clang/include/clang/Frontend/CompilerInstance.h (-5)
  • (modified) clang/include/clang/Lex/HeaderSearch.h (+17-8)
  • (modified) clang/include/clang/Serialization/ASTReader.h (+3-6)
  • (modified) clang/lib/DependencyScanning/DependencyScannerImpl.cpp (+1-2)
  • (modified) clang/lib/Frontend/ASTUnit.cpp (+11-12)
  • (modified) clang/lib/Frontend/CompilerInstance.cpp (+3-19)
  • (modified) clang/lib/Frontend/FrontendAction.cpp (+4-1)
  • (modified) clang/lib/Frontend/FrontendActions.cpp (+5-1)
  • (modified) clang/lib/Lex/HeaderSearch.cpp (+28)
  • (modified) clang/lib/Serialization/ASTReader.cpp (+20-17)
  • (modified) clang/lib/Serialization/ASTWriter.cpp (+3-7)
diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h
index 266e0826b38f4..f7a0d1064110a 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -746,11 +746,6 @@ class CompilerInstance : public ModuleLoader {
     GetDependencyDirectives = std::move(Getter);
   }
 
-  std::string getSpecificModuleCachePath(StringRef ContextHash);
-  std::string getSpecificModuleCachePath() {
-    return getSpecificModuleCachePath(getInvocation().computeContextHash());
-  }
-
   /// Create the AST context.
   void createASTContext();
 
diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h
index d7000da682c6e..2bb78fbf608f3 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -282,6 +282,10 @@ class HeaderSearch {
   /// The specific module cache path containing ContextHash (unless suppressed).
   std::string SpecificModuleCachePath;
 
+  /// The length of the normalized module cache path at the start of \c
+  /// SpecificModuleCachePath.
+  size_t NormalizedModuleCachePathLen = 0;
+
   /// All of the preprocessor-specific data about files that are
   /// included, indexed by the FileEntry's UID.
   mutable std::vector<HeaderFileInfo> FileInfo;
@@ -467,20 +471,20 @@ class HeaderSearch {
     return {};
   }
 
-  /// Set the context hash to use for module cache paths.
-  void setContextHash(StringRef Hash) { ContextHash = std::string(Hash); }
+  /// Initialize the module cache path.
+  void initializeModuleCachePath(std::string ContextHash);
 
-  /// Set the module cache path with the context hash (unless suppressed).
-  void setSpecificModuleCachePath(StringRef Path) {
-    SpecificModuleCachePath = std::string(Path);
+  /// Retrieve the module cache path with the context hash (unless suppressed).
+  StringRef getSpecificModuleCachePath() const {
+    return SpecificModuleCachePath;
   }
 
   /// Retrieve the context hash.
   StringRef getContextHash() const { return ContextHash; }
 
-  /// Retrieve the module cache path with the context hash (unless suppressed).
-  StringRef getSpecificModuleCachePath() const {
-    return SpecificModuleCachePath;
+  /// Retrieve the normalized module cache path.
+  StringRef getNormalizedModuleCachePath() const {
+    return getSpecificModuleCachePath().substr(0, NormalizedModuleCachePathLen);
   }
 
   /// Forget everything we know about headers so far.
@@ -1042,6 +1046,11 @@ void ApplyHeaderSearchOptions(HeaderSearch &HS,
 void normalizeModuleCachePath(FileManager &FileMgr, StringRef Path,
                               SmallVectorImpl<char> &NormalizedPath);
 
+std::string createSpecificModuleCachePath(FileManager &FileMgr,
+                                          StringRef ModuleCachePath,
+                                          bool DisableModuleHash,
+                                          std::string ContextHash);
+
 } // namespace clang
 
 #endif // LLVM_CLANG_LEX_HEADERSEARCH_H
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index f254459ce933d..14f7d8a69a1b3 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -185,8 +185,7 @@ class ASTReaderListener {
   /// otherwise.
   virtual bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
                                        StringRef ModuleFilename,
-                                       StringRef SpecificModuleCachePath,
-                                       bool Complain) {
+                                       StringRef ContextHash, bool Complain) {
     return false;
   }
 
@@ -304,8 +303,7 @@ class ChainedASTReaderListener : public ASTReaderListener {
                              bool Complain) override;
 
   bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
-                               StringRef ModuleFilename,
-                               StringRef SpecificModuleCachePath,
+                               StringRef ModuleFilename, StringRef ContextHash,
                                bool Complain) override;
   bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
                                StringRef ModuleFilename, bool ReadMacros,
@@ -349,8 +347,7 @@ class PCHValidator : public ASTReaderListener {
                                bool Complain,
                                std::string &SuggestedPredefines) override;
   bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
-                               StringRef ModuleFilename,
-                               StringRef SpecificModuleCachePath,
+                               StringRef ModuleFilename, StringRef ContextHash,
                                bool Complain) override;
   void ReadCounter(const serialization::ModuleFile &M, uint32_t Value) override;
 };
diff --git a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp
index 0e345af8817ae..a38ee690d42de 100644
--- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp
+++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp
@@ -150,8 +150,7 @@ class PrebuiltModuleListener : public ASTReaderListener {
   /// Check the header search options for a given module when considering
   /// if the module comes from stable directories.
   bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
-                               StringRef ModuleFilename,
-                               StringRef SpecificModuleCachePath,
+                               StringRef ModuleFilename, StringRef ContextHash,
                                bool Complain) override {
 
     auto PrebuiltMapEntry = PrebuiltModulesASTMap.try_emplace(CurrentFile);
diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index 9fcaf1806fcb1..1d249ebaa1492 100644
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -505,7 +505,7 @@ namespace {
 /// a Preprocessor.
 class ASTInfoCollector : public ASTReaderListener {
   HeaderSearchOptions &HSOpts;
-  std::string &SpecificModuleCachePath;
+  std::string &ContextHash;
   PreprocessorOptions &PPOpts;
   LangOptions &LangOpts;
   CodeGenOptions &CodeGenOpts;
@@ -513,14 +513,13 @@ class ASTInfoCollector : public ASTReaderListener {
   uint32_t &Counter;
 
 public:
-  ASTInfoCollector(HeaderSearchOptions &HSOpts,
-                   std::string &SpecificModuleCachePath,
+  ASTInfoCollector(HeaderSearchOptions &HSOpts, std::string &ContextHash,
                    PreprocessorOptions &PPOpts, LangOptions &LangOpts,
                    CodeGenOptions &CodeGenOpts, TargetOptions &TargetOpts,
                    uint32_t &Counter)
-      : HSOpts(HSOpts), SpecificModuleCachePath(SpecificModuleCachePath),
-        PPOpts(PPOpts), LangOpts(LangOpts), CodeGenOpts(CodeGenOpts),
-        TargetOpts(TargetOpts), Counter(Counter) {}
+      : HSOpts(HSOpts), ContextHash(ContextHash), PPOpts(PPOpts),
+        LangOpts(LangOpts), CodeGenOpts(CodeGenOpts), TargetOpts(TargetOpts),
+        Counter(Counter) {}
 
   bool ReadLanguageOptions(const LangOptions &NewLangOpts,
                            StringRef ModuleFilename, bool Complain,
@@ -538,10 +537,10 @@ class ASTInfoCollector : public ASTReaderListener {
 
   bool ReadHeaderSearchOptions(const HeaderSearchOptions &NewHSOpts,
                                StringRef ModuleFilename,
-                               StringRef NewSpecificModuleCachePath,
+                               StringRef NewContextHash,
                                bool Complain) override {
     HSOpts = NewHSOpts;
-    SpecificModuleCachePath = NewSpecificModuleCachePath;
+    ContextHash = NewContextHash;
     return false;
   }
 
@@ -733,13 +732,13 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile(
   AST->ModCache = createCrossProcessModuleCache();
 
   // Gather info for preprocessor construction later on.
-  std::string SpecificModuleCachePath;
+  std::string ContextHash;
   unsigned Counter = 0;
   // Using a temporary FileManager since the AST file might specify custom
   // HeaderSearchOptions::VFSOverlayFiles that affect the underlying VFS.
   FileManager TmpFileMgr(FileSystemOpts, VFS);
-  ASTInfoCollector Collector(*AST->HSOpts, SpecificModuleCachePath,
-                             *AST->PPOpts, *AST->LangOpts, *AST->CodeGenOpts,
+  ASTInfoCollector Collector(*AST->HSOpts, ContextHash, *AST->PPOpts,
+                             *AST->LangOpts, *AST->CodeGenOpts,
                              *AST->TargetOpts, Counter);
   if (ASTReader::readASTFileControlBlock(
           Filename, TmpFileMgr, *AST->ModCache, PCHContainerRdr,
@@ -763,7 +762,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile(
       AST->getHeaderSearchOpts(), AST->getSourceManager(),
       AST->getDiagnostics(), AST->getLangOpts(),
       /*Target=*/nullptr);
-  AST->HeaderInfo->setSpecificModuleCachePath(SpecificModuleCachePath);
+  AST->HeaderInfo->initializeModuleCachePath(std::move(ContextHash));
 
   AST->PP = std::make_shared<Preprocessor>(
       *AST->PPOpts, AST->getDiagnostics(), *AST->LangOpts,
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 60914d9b2cbc7..4c7a5a66254a5 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -486,12 +486,9 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
 
   PP->setPreprocessedOutput(getPreprocessorOutputOpts().ShowCPP);
 
-  if (PP->getLangOpts().Modules && PP->getLangOpts().ImplicitModules) {
-    std::string ContextHash = getInvocation().computeContextHash();
-    PP->getHeaderSearchInfo().setContextHash(ContextHash);
-    PP->getHeaderSearchInfo().setSpecificModuleCachePath(
-        getSpecificModuleCachePath(ContextHash));
-  }
+  if (PP->getLangOpts().Modules && PP->getLangOpts().ImplicitModules)
+    PP->getHeaderSearchInfo().initializeModuleCachePath(
+        getInvocation().computeContextHash());
 
   // Handle generating dependencies, if requested.
   const DependencyOutputOptions &DepOpts = getDependencyOutputOpts();
@@ -546,19 +543,6 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
     PP->setDependencyDirectivesGetter(*GetDependencyDirectives);
 }
 
-std::string
-CompilerInstance::getSpecificModuleCachePath(StringRef ContextHash) {
-  assert(FileMgr && "Specific module cache path requires a FileManager");
-
-  // Set up the module path, including the hash for the module-creation options.
-  SmallString<256> SpecificModuleCache;
-  normalizeModuleCachePath(*FileMgr, getHeaderSearchOpts().ModuleCachePath,
-                           SpecificModuleCache);
-  if (!SpecificModuleCache.empty() && !getHeaderSearchOpts().DisableModuleHash)
-    llvm::sys::path::append(SpecificModuleCache, ContextHash);
-  return std::string(SpecificModuleCache);
-}
-
 // ASTContext
 
 void CompilerInstance::createASTContext() {
diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp
index 73f092521546f..84ceb22208801 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -1040,7 +1040,10 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
       llvm::sys::path::native(PCHDir->getName(), DirNative);
       bool Found = false;
       llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem();
-      std::string SpecificModuleCachePath = CI.getSpecificModuleCachePath();
+      std::string SpecificModuleCachePath = createSpecificModuleCachePath(
+          CI.getFileManager(), CI.getHeaderSearchOpts().ModuleCachePath,
+          CI.getHeaderSearchOpts().DisableModuleHash,
+          CI.getInvocation().computeContextHash());
       for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC),
                                          DirEnd;
            Dir != DirEnd && !EC; Dir.increment(EC)) {
diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index 492f7b1742bee..15d19dae966ac 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -716,8 +716,12 @@ namespace {
 
     bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
                                  StringRef ModuleFilename,
-                                 StringRef SpecificModuleCachePath,
+                                 StringRef ContextHash,
                                  bool Complain) override {
+      SmallString<128> SpecificModuleCachePath(HSOpts.ModuleCachePath);
+      if (!SpecificModuleCachePath.empty() && !HSOpts.DisableModuleHash)
+        llvm::sys::path::append(SpecificModuleCachePath, ContextHash);
+
       Out.indent(2) << "Header search options:\n";
       Out.indent(4) << "System root [-isysroot=]: '" << HSOpts.Sysroot << "'\n";
       Out.indent(4) << "Resource dir [ -resource-dir=]: '" << HSOpts.ResourceDir << "'\n";
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 5aee19cd14c51..de26b999683d1 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -2471,3 +2471,31 @@ void clang::normalizeModuleCachePath(FileManager &FileMgr, StringRef Path,
     llvm::sys::path::remove_dots(NormalizedPath);
   }
 }
+
+static std::string createSpecificModuleCachePathImpl(
+    FileManager &FileMgr, StringRef ModuleCachePath, bool DisableModuleHash,
+    std::string ContextHash, size_t &NormalizedModuleCachePathLen) {
+  SmallString<256> SpecificModuleCachePath;
+  normalizeModuleCachePath(FileMgr, ModuleCachePath, SpecificModuleCachePath);
+  NormalizedModuleCachePathLen = SpecificModuleCachePath.size();
+  if (!SpecificModuleCachePath.empty() && !DisableModuleHash)
+    llvm::sys::path::append(SpecificModuleCachePath, ContextHash);
+  return std::string(SpecificModuleCachePath);
+}
+
+void HeaderSearch::initializeModuleCachePath(std::string NewContextHash) {
+  ContextHash = std::move(NewContextHash);
+  SpecificModuleCachePath = createSpecificModuleCachePathImpl(
+      FileMgr, HSOpts.ModuleCachePath, HSOpts.DisableModuleHash, ContextHash,
+      NormalizedModuleCachePathLen);
+}
+
+std::string clang::createSpecificModuleCachePath(FileManager &FileMgr,
+                                                 StringRef ModuleCachePath,
+                                                 bool DisableModuleHash,
+                                                 std::string ContextHash) {
+  size_t NormalizedModuleCachePathLen;
+  return createSpecificModuleCachePathImpl(
+      FileMgr, ModuleCachePath, DisableModuleHash, std::move(ContextHash),
+      NormalizedModuleCachePathLen);
+}
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index b82ae971bc84d..812f27989c129 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -208,11 +208,11 @@ ChainedASTReaderListener::ReadFileSystemOptions(const FileSystemOptions &FSOpts,
 
 bool ChainedASTReaderListener::ReadHeaderSearchOptions(
     const HeaderSearchOptions &HSOpts, StringRef ModuleFilename,
-    StringRef SpecificModuleCachePath, bool Complain) {
-  return First->ReadHeaderSearchOptions(HSOpts, ModuleFilename,
-                                        SpecificModuleCachePath, Complain) ||
-         Second->ReadHeaderSearchOptions(HSOpts, ModuleFilename,
-                                         SpecificModuleCachePath, Complain);
+    StringRef ContextHash, bool Complain) {
+  return First->ReadHeaderSearchOptions(HSOpts, ModuleFilename, ContextHash,
+                                        Complain) ||
+         Second->ReadHeaderSearchOptions(HSOpts, ModuleFilename, ContextHash,
+                                         Complain);
 }
 
 bool ChainedASTReaderListener::ReadPreprocessorOptions(
@@ -961,11 +961,15 @@ bool SimpleASTReaderListener::ReadPreprocessorOptions(
 /// \param Diags If non-null, produce diagnostics for any mismatches incurred.
 /// \returns true when the module cache paths differ.
 static bool checkModuleCachePath(
-    llvm::vfs::FileSystem &VFS, StringRef SpecificModuleCachePath,
+    llvm::vfs::FileSystem &VFS, StringRef ContextHash,
     StringRef ExistingSpecificModuleCachePath, StringRef ASTFilename,
     DiagnosticsEngine *Diags, const LangOptions &LangOpts,
     const PreprocessorOptions &PPOpts, const HeaderSearchOptions &HSOpts,
     const HeaderSearchOptions &ASTFileHSOpts) {
+  SmallString<128> SpecificModuleCachePath(ASTFileHSOpts.ModuleCachePath);
+  if (!SpecificModuleCachePath.empty() && !ASTFileHSOpts.DisableModuleHash)
+    llvm::sys::path::append(SpecificModuleCachePath, ContextHash);
+
   if (!LangOpts.Modules || PPOpts.AllowPCHWithDifferentModulesCachePath ||
       SpecificModuleCachePath == ExistingSpecificModuleCachePath)
     return false;
@@ -991,11 +995,11 @@ static bool checkModuleCachePath(
 
 bool PCHValidator::ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
                                            StringRef ASTFilename,
-                                           StringRef SpecificModuleCachePath,
+                                           StringRef ContextHash,
                                            bool Complain) {
   const HeaderSearch &HeaderSearchInfo = PP.getHeaderSearchInfo();
   return checkModuleCachePath(
-      Reader.getFileManager().getVirtualFileSystem(), SpecificModuleCachePath,
+      Reader.getFileManager().getVirtualFileSystem(), ContextHash,
       HeaderSearchInfo.getSpecificModuleCachePath(), ASTFilename,
       Complain ? &Reader.Diags : nullptr, PP.getLangOpts(),
       PP.getPreprocessorOpts(), HeaderSearchInfo.getHeaderSearchOpts(), HSOpts);
@@ -5877,13 +5881,12 @@ namespace {
     }
 
     bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
-                                 StringRef ASTFilename,
-                                 StringRef SpecificModuleCachePath,
+                                 StringRef ASTFilename, StringRef ContextHash,
                                  bool Complain) override {
-      return checkModuleCachePath(
-          FileMgr.getVirtualFileSystem(), SpecificModuleCachePath,
-          ExistingSpecificModuleCachePath, ASTFilename, nullptr,
-          ExistingLangOpts, ExistingPPOpts, ExistingHSOpts, HSOpts);
+      return checkModuleCachePath(FileMgr.getVirtualFileSystem(), ContextHash,
+                                  ExistingSpecificModuleCachePath, ASTFilename,
+                                  nullptr, ExistingLangOpts, ExistingPPOpts,
+                                  ExistingHSOpts, HSOpts);
     }
 
     bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
@@ -6705,10 +6708,10 @@ bool ASTReader::ParseHeaderSearchOptions(const RecordData &Record,
   HSOpts.UseStandardSystemIncludes = Record[Idx++];
   HSOpts.UseStandardCXXIncludes = Record[Idx++];
   HSOpts.UseLibcxx = Record[Idx++];
-  std::string SpecificModuleCachePath = ReadString(Record, Idx);
+  std::string ContextHash = ReadString(Record, Idx);
 
-  return Listener.ReadHeaderSearchOptions(HSOpts, ModuleFilename,
-                                          SpecificModuleCachePath, Complain);
+  return Listener.ReadHeaderSearchOptions(HSOpts, ModuleFilename, ContextHash,
+                                          Complain);
 }
 
 bool ASTReader::ParseHeaderSearchPaths(const RecordData &Record, bool Complain,
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index ec718169550aa..2744d70b89aac 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1694,9 +1694,8 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) {
   const HeaderSearchOptions &HSOpts =
       PP.getHeaderSearchInfo().getHeaderSearchOpts();
 
-  SmallString<256> HSOpts_ModuleCachePath;
-  normalizeModuleCachePath(PP.getFileManager(), HSOpts.ModuleCachePath,
-                           HSOpts_ModuleCachePath);
+  StringRef HSOpts_ModuleCachePath =
+      PP.getHeaderSearchInfo().getNormalizedModuleCachePath();
 
   AddString(HSOpts.Sysroot, Record);
   AddString(HSOpts.ResourceDir, Record);
@@ -1710,10 +1709,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroo...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Mar 10, 2026

@llvm/pr-subscribers-clang

Author: Jan Svoboda (jansvoboda11)

Changes

Previously, the normalized module cache path was only accessible via HeaderSearch::getSpecificModuleCachePath() which may or may not also contain the context hash. Clients would need to parse the result to learn the normalized module cache path. What ASTWriter does instead is normalize the as-written module cache path redundantly.

Instead, this PR exposes the normalized module cache path in the HeaderSearch interface and moves the computation of specific module cache path into the clangLex library.

This is motivated by another patch that would've needed to redundantly perform the module cache path canonicalization or parse the specific module cache path.


Patch is 20.56 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/185746.diff

11 Files Affected:

  • (modified) clang/include/clang/Frontend/CompilerInstance.h (-5)
  • (modified) clang/include/clang/Lex/HeaderSearch.h (+17-8)
  • (modified) clang/include/clang/Serialization/ASTReader.h (+3-6)
  • (modified) clang/lib/DependencyScanning/DependencyScannerImpl.cpp (+1-2)
  • (modified) clang/lib/Frontend/ASTUnit.cpp (+11-12)
  • (modified) clang/lib/Frontend/CompilerInstance.cpp (+3-19)
  • (modified) clang/lib/Frontend/FrontendAction.cpp (+4-1)
  • (modified) clang/lib/Frontend/FrontendActions.cpp (+5-1)
  • (modified) clang/lib/Lex/HeaderSearch.cpp (+28)
  • (modified) clang/lib/Serialization/ASTReader.cpp (+20-17)
  • (modified) clang/lib/Serialization/ASTWriter.cpp (+3-7)
diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h
index 266e0826b38f4..f7a0d1064110a 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -746,11 +746,6 @@ class CompilerInstance : public ModuleLoader {
     GetDependencyDirectives = std::move(Getter);
   }
 
-  std::string getSpecificModuleCachePath(StringRef ContextHash);
-  std::string getSpecificModuleCachePath() {
-    return getSpecificModuleCachePath(getInvocation().computeContextHash());
-  }
-
   /// Create the AST context.
   void createASTContext();
 
diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h
index d7000da682c6e..2bb78fbf608f3 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -282,6 +282,10 @@ class HeaderSearch {
   /// The specific module cache path containing ContextHash (unless suppressed).
   std::string SpecificModuleCachePath;
 
+  /// The length of the normalized module cache path at the start of \c
+  /// SpecificModuleCachePath.
+  size_t NormalizedModuleCachePathLen = 0;
+
   /// All of the preprocessor-specific data about files that are
   /// included, indexed by the FileEntry's UID.
   mutable std::vector<HeaderFileInfo> FileInfo;
@@ -467,20 +471,20 @@ class HeaderSearch {
     return {};
   }
 
-  /// Set the context hash to use for module cache paths.
-  void setContextHash(StringRef Hash) { ContextHash = std::string(Hash); }
+  /// Initialize the module cache path.
+  void initializeModuleCachePath(std::string ContextHash);
 
-  /// Set the module cache path with the context hash (unless suppressed).
-  void setSpecificModuleCachePath(StringRef Path) {
-    SpecificModuleCachePath = std::string(Path);
+  /// Retrieve the module cache path with the context hash (unless suppressed).
+  StringRef getSpecificModuleCachePath() const {
+    return SpecificModuleCachePath;
   }
 
   /// Retrieve the context hash.
   StringRef getContextHash() const { return ContextHash; }
 
-  /// Retrieve the module cache path with the context hash (unless suppressed).
-  StringRef getSpecificModuleCachePath() const {
-    return SpecificModuleCachePath;
+  /// Retrieve the normalized module cache path.
+  StringRef getNormalizedModuleCachePath() const {
+    return getSpecificModuleCachePath().substr(0, NormalizedModuleCachePathLen);
   }
 
   /// Forget everything we know about headers so far.
@@ -1042,6 +1046,11 @@ void ApplyHeaderSearchOptions(HeaderSearch &HS,
 void normalizeModuleCachePath(FileManager &FileMgr, StringRef Path,
                               SmallVectorImpl<char> &NormalizedPath);
 
+std::string createSpecificModuleCachePath(FileManager &FileMgr,
+                                          StringRef ModuleCachePath,
+                                          bool DisableModuleHash,
+                                          std::string ContextHash);
+
 } // namespace clang
 
 #endif // LLVM_CLANG_LEX_HEADERSEARCH_H
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index f254459ce933d..14f7d8a69a1b3 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -185,8 +185,7 @@ class ASTReaderListener {
   /// otherwise.
   virtual bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
                                        StringRef ModuleFilename,
-                                       StringRef SpecificModuleCachePath,
-                                       bool Complain) {
+                                       StringRef ContextHash, bool Complain) {
     return false;
   }
 
@@ -304,8 +303,7 @@ class ChainedASTReaderListener : public ASTReaderListener {
                              bool Complain) override;
 
   bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
-                               StringRef ModuleFilename,
-                               StringRef SpecificModuleCachePath,
+                               StringRef ModuleFilename, StringRef ContextHash,
                                bool Complain) override;
   bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
                                StringRef ModuleFilename, bool ReadMacros,
@@ -349,8 +347,7 @@ class PCHValidator : public ASTReaderListener {
                                bool Complain,
                                std::string &SuggestedPredefines) override;
   bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
-                               StringRef ModuleFilename,
-                               StringRef SpecificModuleCachePath,
+                               StringRef ModuleFilename, StringRef ContextHash,
                                bool Complain) override;
   void ReadCounter(const serialization::ModuleFile &M, uint32_t Value) override;
 };
diff --git a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp
index 0e345af8817ae..a38ee690d42de 100644
--- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp
+++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp
@@ -150,8 +150,7 @@ class PrebuiltModuleListener : public ASTReaderListener {
   /// Check the header search options for a given module when considering
   /// if the module comes from stable directories.
   bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
-                               StringRef ModuleFilename,
-                               StringRef SpecificModuleCachePath,
+                               StringRef ModuleFilename, StringRef ContextHash,
                                bool Complain) override {
 
     auto PrebuiltMapEntry = PrebuiltModulesASTMap.try_emplace(CurrentFile);
diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index 9fcaf1806fcb1..1d249ebaa1492 100644
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -505,7 +505,7 @@ namespace {
 /// a Preprocessor.
 class ASTInfoCollector : public ASTReaderListener {
   HeaderSearchOptions &HSOpts;
-  std::string &SpecificModuleCachePath;
+  std::string &ContextHash;
   PreprocessorOptions &PPOpts;
   LangOptions &LangOpts;
   CodeGenOptions &CodeGenOpts;
@@ -513,14 +513,13 @@ class ASTInfoCollector : public ASTReaderListener {
   uint32_t &Counter;
 
 public:
-  ASTInfoCollector(HeaderSearchOptions &HSOpts,
-                   std::string &SpecificModuleCachePath,
+  ASTInfoCollector(HeaderSearchOptions &HSOpts, std::string &ContextHash,
                    PreprocessorOptions &PPOpts, LangOptions &LangOpts,
                    CodeGenOptions &CodeGenOpts, TargetOptions &TargetOpts,
                    uint32_t &Counter)
-      : HSOpts(HSOpts), SpecificModuleCachePath(SpecificModuleCachePath),
-        PPOpts(PPOpts), LangOpts(LangOpts), CodeGenOpts(CodeGenOpts),
-        TargetOpts(TargetOpts), Counter(Counter) {}
+      : HSOpts(HSOpts), ContextHash(ContextHash), PPOpts(PPOpts),
+        LangOpts(LangOpts), CodeGenOpts(CodeGenOpts), TargetOpts(TargetOpts),
+        Counter(Counter) {}
 
   bool ReadLanguageOptions(const LangOptions &NewLangOpts,
                            StringRef ModuleFilename, bool Complain,
@@ -538,10 +537,10 @@ class ASTInfoCollector : public ASTReaderListener {
 
   bool ReadHeaderSearchOptions(const HeaderSearchOptions &NewHSOpts,
                                StringRef ModuleFilename,
-                               StringRef NewSpecificModuleCachePath,
+                               StringRef NewContextHash,
                                bool Complain) override {
     HSOpts = NewHSOpts;
-    SpecificModuleCachePath = NewSpecificModuleCachePath;
+    ContextHash = NewContextHash;
     return false;
   }
 
@@ -733,13 +732,13 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile(
   AST->ModCache = createCrossProcessModuleCache();
 
   // Gather info for preprocessor construction later on.
-  std::string SpecificModuleCachePath;
+  std::string ContextHash;
   unsigned Counter = 0;
   // Using a temporary FileManager since the AST file might specify custom
   // HeaderSearchOptions::VFSOverlayFiles that affect the underlying VFS.
   FileManager TmpFileMgr(FileSystemOpts, VFS);
-  ASTInfoCollector Collector(*AST->HSOpts, SpecificModuleCachePath,
-                             *AST->PPOpts, *AST->LangOpts, *AST->CodeGenOpts,
+  ASTInfoCollector Collector(*AST->HSOpts, ContextHash, *AST->PPOpts,
+                             *AST->LangOpts, *AST->CodeGenOpts,
                              *AST->TargetOpts, Counter);
   if (ASTReader::readASTFileControlBlock(
           Filename, TmpFileMgr, *AST->ModCache, PCHContainerRdr,
@@ -763,7 +762,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile(
       AST->getHeaderSearchOpts(), AST->getSourceManager(),
       AST->getDiagnostics(), AST->getLangOpts(),
       /*Target=*/nullptr);
-  AST->HeaderInfo->setSpecificModuleCachePath(SpecificModuleCachePath);
+  AST->HeaderInfo->initializeModuleCachePath(std::move(ContextHash));
 
   AST->PP = std::make_shared<Preprocessor>(
       *AST->PPOpts, AST->getDiagnostics(), *AST->LangOpts,
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 60914d9b2cbc7..4c7a5a66254a5 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -486,12 +486,9 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
 
   PP->setPreprocessedOutput(getPreprocessorOutputOpts().ShowCPP);
 
-  if (PP->getLangOpts().Modules && PP->getLangOpts().ImplicitModules) {
-    std::string ContextHash = getInvocation().computeContextHash();
-    PP->getHeaderSearchInfo().setContextHash(ContextHash);
-    PP->getHeaderSearchInfo().setSpecificModuleCachePath(
-        getSpecificModuleCachePath(ContextHash));
-  }
+  if (PP->getLangOpts().Modules && PP->getLangOpts().ImplicitModules)
+    PP->getHeaderSearchInfo().initializeModuleCachePath(
+        getInvocation().computeContextHash());
 
   // Handle generating dependencies, if requested.
   const DependencyOutputOptions &DepOpts = getDependencyOutputOpts();
@@ -546,19 +543,6 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
     PP->setDependencyDirectivesGetter(*GetDependencyDirectives);
 }
 
-std::string
-CompilerInstance::getSpecificModuleCachePath(StringRef ContextHash) {
-  assert(FileMgr && "Specific module cache path requires a FileManager");
-
-  // Set up the module path, including the hash for the module-creation options.
-  SmallString<256> SpecificModuleCache;
-  normalizeModuleCachePath(*FileMgr, getHeaderSearchOpts().ModuleCachePath,
-                           SpecificModuleCache);
-  if (!SpecificModuleCache.empty() && !getHeaderSearchOpts().DisableModuleHash)
-    llvm::sys::path::append(SpecificModuleCache, ContextHash);
-  return std::string(SpecificModuleCache);
-}
-
 // ASTContext
 
 void CompilerInstance::createASTContext() {
diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp
index 73f092521546f..84ceb22208801 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -1040,7 +1040,10 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
       llvm::sys::path::native(PCHDir->getName(), DirNative);
       bool Found = false;
       llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem();
-      std::string SpecificModuleCachePath = CI.getSpecificModuleCachePath();
+      std::string SpecificModuleCachePath = createSpecificModuleCachePath(
+          CI.getFileManager(), CI.getHeaderSearchOpts().ModuleCachePath,
+          CI.getHeaderSearchOpts().DisableModuleHash,
+          CI.getInvocation().computeContextHash());
       for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC),
                                          DirEnd;
            Dir != DirEnd && !EC; Dir.increment(EC)) {
diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index 492f7b1742bee..15d19dae966ac 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -716,8 +716,12 @@ namespace {
 
     bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
                                  StringRef ModuleFilename,
-                                 StringRef SpecificModuleCachePath,
+                                 StringRef ContextHash,
                                  bool Complain) override {
+      SmallString<128> SpecificModuleCachePath(HSOpts.ModuleCachePath);
+      if (!SpecificModuleCachePath.empty() && !HSOpts.DisableModuleHash)
+        llvm::sys::path::append(SpecificModuleCachePath, ContextHash);
+
       Out.indent(2) << "Header search options:\n";
       Out.indent(4) << "System root [-isysroot=]: '" << HSOpts.Sysroot << "'\n";
       Out.indent(4) << "Resource dir [ -resource-dir=]: '" << HSOpts.ResourceDir << "'\n";
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 5aee19cd14c51..de26b999683d1 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -2471,3 +2471,31 @@ void clang::normalizeModuleCachePath(FileManager &FileMgr, StringRef Path,
     llvm::sys::path::remove_dots(NormalizedPath);
   }
 }
+
+static std::string createSpecificModuleCachePathImpl(
+    FileManager &FileMgr, StringRef ModuleCachePath, bool DisableModuleHash,
+    std::string ContextHash, size_t &NormalizedModuleCachePathLen) {
+  SmallString<256> SpecificModuleCachePath;
+  normalizeModuleCachePath(FileMgr, ModuleCachePath, SpecificModuleCachePath);
+  NormalizedModuleCachePathLen = SpecificModuleCachePath.size();
+  if (!SpecificModuleCachePath.empty() && !DisableModuleHash)
+    llvm::sys::path::append(SpecificModuleCachePath, ContextHash);
+  return std::string(SpecificModuleCachePath);
+}
+
+void HeaderSearch::initializeModuleCachePath(std::string NewContextHash) {
+  ContextHash = std::move(NewContextHash);
+  SpecificModuleCachePath = createSpecificModuleCachePathImpl(
+      FileMgr, HSOpts.ModuleCachePath, HSOpts.DisableModuleHash, ContextHash,
+      NormalizedModuleCachePathLen);
+}
+
+std::string clang::createSpecificModuleCachePath(FileManager &FileMgr,
+                                                 StringRef ModuleCachePath,
+                                                 bool DisableModuleHash,
+                                                 std::string ContextHash) {
+  size_t NormalizedModuleCachePathLen;
+  return createSpecificModuleCachePathImpl(
+      FileMgr, ModuleCachePath, DisableModuleHash, std::move(ContextHash),
+      NormalizedModuleCachePathLen);
+}
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index b82ae971bc84d..812f27989c129 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -208,11 +208,11 @@ ChainedASTReaderListener::ReadFileSystemOptions(const FileSystemOptions &FSOpts,
 
 bool ChainedASTReaderListener::ReadHeaderSearchOptions(
     const HeaderSearchOptions &HSOpts, StringRef ModuleFilename,
-    StringRef SpecificModuleCachePath, bool Complain) {
-  return First->ReadHeaderSearchOptions(HSOpts, ModuleFilename,
-                                        SpecificModuleCachePath, Complain) ||
-         Second->ReadHeaderSearchOptions(HSOpts, ModuleFilename,
-                                         SpecificModuleCachePath, Complain);
+    StringRef ContextHash, bool Complain) {
+  return First->ReadHeaderSearchOptions(HSOpts, ModuleFilename, ContextHash,
+                                        Complain) ||
+         Second->ReadHeaderSearchOptions(HSOpts, ModuleFilename, ContextHash,
+                                         Complain);
 }
 
 bool ChainedASTReaderListener::ReadPreprocessorOptions(
@@ -961,11 +961,15 @@ bool SimpleASTReaderListener::ReadPreprocessorOptions(
 /// \param Diags If non-null, produce diagnostics for any mismatches incurred.
 /// \returns true when the module cache paths differ.
 static bool checkModuleCachePath(
-    llvm::vfs::FileSystem &VFS, StringRef SpecificModuleCachePath,
+    llvm::vfs::FileSystem &VFS, StringRef ContextHash,
     StringRef ExistingSpecificModuleCachePath, StringRef ASTFilename,
     DiagnosticsEngine *Diags, const LangOptions &LangOpts,
     const PreprocessorOptions &PPOpts, const HeaderSearchOptions &HSOpts,
     const HeaderSearchOptions &ASTFileHSOpts) {
+  SmallString<128> SpecificModuleCachePath(ASTFileHSOpts.ModuleCachePath);
+  if (!SpecificModuleCachePath.empty() && !ASTFileHSOpts.DisableModuleHash)
+    llvm::sys::path::append(SpecificModuleCachePath, ContextHash);
+
   if (!LangOpts.Modules || PPOpts.AllowPCHWithDifferentModulesCachePath ||
       SpecificModuleCachePath == ExistingSpecificModuleCachePath)
     return false;
@@ -991,11 +995,11 @@ static bool checkModuleCachePath(
 
 bool PCHValidator::ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
                                            StringRef ASTFilename,
-                                           StringRef SpecificModuleCachePath,
+                                           StringRef ContextHash,
                                            bool Complain) {
   const HeaderSearch &HeaderSearchInfo = PP.getHeaderSearchInfo();
   return checkModuleCachePath(
-      Reader.getFileManager().getVirtualFileSystem(), SpecificModuleCachePath,
+      Reader.getFileManager().getVirtualFileSystem(), ContextHash,
       HeaderSearchInfo.getSpecificModuleCachePath(), ASTFilename,
       Complain ? &Reader.Diags : nullptr, PP.getLangOpts(),
       PP.getPreprocessorOpts(), HeaderSearchInfo.getHeaderSearchOpts(), HSOpts);
@@ -5877,13 +5881,12 @@ namespace {
     }
 
     bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
-                                 StringRef ASTFilename,
-                                 StringRef SpecificModuleCachePath,
+                                 StringRef ASTFilename, StringRef ContextHash,
                                  bool Complain) override {
-      return checkModuleCachePath(
-          FileMgr.getVirtualFileSystem(), SpecificModuleCachePath,
-          ExistingSpecificModuleCachePath, ASTFilename, nullptr,
-          ExistingLangOpts, ExistingPPOpts, ExistingHSOpts, HSOpts);
+      return checkModuleCachePath(FileMgr.getVirtualFileSystem(), ContextHash,
+                                  ExistingSpecificModuleCachePath, ASTFilename,
+                                  nullptr, ExistingLangOpts, ExistingPPOpts,
+                                  ExistingHSOpts, HSOpts);
     }
 
     bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
@@ -6705,10 +6708,10 @@ bool ASTReader::ParseHeaderSearchOptions(const RecordData &Record,
   HSOpts.UseStandardSystemIncludes = Record[Idx++];
   HSOpts.UseStandardCXXIncludes = Record[Idx++];
   HSOpts.UseLibcxx = Record[Idx++];
-  std::string SpecificModuleCachePath = ReadString(Record, Idx);
+  std::string ContextHash = ReadString(Record, Idx);
 
-  return Listener.ReadHeaderSearchOptions(HSOpts, ModuleFilename,
-                                          SpecificModuleCachePath, Complain);
+  return Listener.ReadHeaderSearchOptions(HSOpts, ModuleFilename, ContextHash,
+                                          Complain);
 }
 
 bool ASTReader::ParseHeaderSearchPaths(const RecordData &Record, bool Complain,
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index ec718169550aa..2744d70b89aac 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1694,9 +1694,8 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) {
   const HeaderSearchOptions &HSOpts =
       PP.getHeaderSearchInfo().getHeaderSearchOpts();
 
-  SmallString<256> HSOpts_ModuleCachePath;
-  normalizeModuleCachePath(PP.getFileManager(), HSOpts.ModuleCachePath,
-                           HSOpts_ModuleCachePath);
+  StringRef HSOpts_ModuleCachePath =
+      PP.getHeaderSearchInfo().getNormalizedModuleCachePath();
 
   AddString(HSOpts.Sysroot, Record);
   AddString(HSOpts.ResourceDir, Record);
@@ -1710,10 +1709,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroo...
[truncated]

@jansvoboda11 jansvoboda11 changed the title [clang][modules] Provide normalized module cache path from HeaderSearch [clang] Expose normalized module cache path in HeaderSearch Mar 11, 2026
Copy link
Member

@cyndyishida cyndyishida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple questions/nits but overall is helpful cleanup.

void normalizeModuleCachePath(FileManager &FileMgr, StringRef Path,
SmallVectorImpl<char> &NormalizedPath);

std::string createSpecificModuleCachePath(FileManager &FileMgr,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice some callsites now call this vs previously calling getSpecificModuleCachePath which always computes the SpecificModuleCachePath. How come? Naively I'd expect we only need to compute it once. Is it because HeaderSearch is not always reachable? IMO it's better to always reuse to reduce chances of misalignment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the call to getSpecificModuleCachePath() (now createSpecificModuleCachePath()) is in "clang/lib/Frontend/FrontendAction.cpp". There we don't have a Preprocessor or HeaderSearch set up yet, since we're only in the process of massaging PreprocessorOptions. This is why we need to compute it ourselves.

The second call is done from HeaderSearch::initializeModuleCachePath() which we call in "clang/lib/Frontend/CompilerInstance.cpp". I think this is indeed redundant with the first call, but I'm not 100% sure.

I personally don't mind this that much, but if we can find an elegant way to de-duplicate these calls, that'd be a good follow-up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the path be saved within HeaderSearchOptions objects? AFAICT, that's always available at callsites of createSpecificModuleCachePath. That way HeaderSearch still has access to it since it's passed in the constructor. If it ever needed to be updated, it would need to go through the HeaderSearchOption instance.

This is not a big enough concern to block this PR, but I'm more generally worried about what kinds of bugs would happen if accidental misalignment happened.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, my long-term goal is to prevent any mutation of CompilerInvocation within the compiler, so that we can consistently leverage its CoW behavior, at least in the scanner. We can't do this earlier, during command-line parsing, because we haven't initialized the VFS yet.

TBH I'm not 100% sure these two calls do (or are supposed to) produce the same specific module cache path, so I'd like to defer this change to a separate PR if possible.

Copy link
Member

@cyndyishida cyndyishida Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure these two calls do (or are supposed to)

Can you add a FIXME in that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in ad7c29d.

void normalizeModuleCachePath(FileManager &FileMgr, StringRef Path,
SmallVectorImpl<char> &NormalizedPath);

std::string createSpecificModuleCachePath(FileManager &FileMgr,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the path be saved within HeaderSearchOptions objects? AFAICT, that's always available at callsites of createSpecificModuleCachePath. That way HeaderSearch still has access to it since it's passed in the constructor. If it ever needed to be updated, it would need to go through the HeaderSearchOption instance.

This is not a big enough concern to block this PR, but I'm more generally worried about what kinds of bugs would happen if accidental misalignment happened.

@jansvoboda11 jansvoboda11 merged commit 983269b into llvm:main Mar 12, 2026
10 checks passed
@jansvoboda11 jansvoboda11 deleted the separate-module-cache-path branch March 12, 2026 20:52
albertbolt1 pushed a commit to albertbolt1/llvm-project that referenced this pull request Mar 13, 2026
…85746)

Previously, the normalized module cache path was only accessible via
`HeaderSearch::getSpecificModuleCachePath()` which may or may not also
contain the context hash. Clients would need to parse the result to
learn the normalized module cache path. What `ASTWriter` does instead is
normalize the as-written module cache path redundantly.

Instead, this PR exposes the normalized module cache path in the
`HeaderSearch` interface and moves the computation of specific module
cache path into the clangLex library.

This is motivated by another patch that would've needed to redundantly
perform the module cache path canonicalization or parse the specific
module cache path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants