[clang] [ASTWriter] Do not allocate source location space for module maps used only for textual headers (PR #116374)

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 29 08:03:54 PST 2024


================
@@ -184,14 +184,30 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
   const SourceManager &SM = PP.getSourceManager();
   const ModuleMap &MM = HS.getModuleMap();
 
-  llvm::DenseSet<FileID> ModuleMaps;
-
-  llvm::DenseSet<const Module *> ProcessedModules;
-  auto CollectModuleMapsForHierarchy = [&](const Module *M) {
+  // Module maps used only by textual headers are special. Their FileID is
+  // non-affecting, but their FileEntry is (i.e. must be written as InputFile).
+  enum AffectedReason : bool {
+    ARTextualHeader = 0,
+    ARImportOrTextualHeader = 1,
+  };
+  auto AssignMostImportant = [](AffectedReason &L, AffectedReason R) {
+    L = std::max(L, R);
+  };
+  llvm::DenseMap<FileID, AffectedReason> ModuleMaps;
+  llvm::DenseMap<const Module *, AffectedReason> ProcessedModules;
+  auto CollectModuleMapsForHierarchy = [&](const Module *M,
+                                           AffectedReason Reason) {
     M = M->getTopLevelModule();
 
-    if (!ProcessedModules.insert(M).second)
+    // We need to process the header either when it was not present of when we
+    // previously flagged module map as textual headers and now we found a
+    // proper import.
+    if (auto [It, Inserted] = ProcessedModules.insert({M, Reason});
+        !Inserted && Reason <= It->second) {
       return;
+    } else {
+      It->second = Reason;
+    }
----------------
ilya-biryukov wrote:

I cannot in that case, because `It` is only valid for the scope of the `if`.
Increasing the visibility scope of `It` seems like a worse trade-off. But let me know if you have other suggestions how to rewrite the code better, I might be missing some ideas.

https://github.com/llvm/llvm-project/pull/116374


More information about the cfe-commits mailing list