[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:11:43 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);
+  };
----------------
ilya-biryukov wrote:

I've updated to use `LHS` and `RHS`. However, I actually wanted to argue that `L` and `R` are better choices for small functions. I have found that because of the `HS` suffix, it is actually **easier** to confuse the two and misread the code. 
It usually does not matter, but if there's an error, it becomes harder to spot, e.g.
```cpp
LHS.a < RHS.a && LHS.b < RHS.b && LHS.c < LHS.c && LHS.d < RHS.d
```
vs
```cpp
L.a < R.a && L.b < R.b && L.c < L.c && L.d < R.d
```

At least for me, the second variant stands out much more clearly.
That's the reason why I started using this pattern instead of the more common LHS and RHS.

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


More information about the cfe-commits mailing list