[clang] [clang][modules] Allow including module maps to be non-affecting (PR #89992)

Jan Svoboda via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 26 15:46:44 PDT 2024


================
@@ -249,9 +245,27 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
 
     for (const auto &KH : HS.findResolvedModulesForHeader(*File))
       if (const Module *M = KH.getModule())
-        CollectIncludingMapsFromAncestors(M);
+        CollectModuleMapsForHierarchy(M);
   }
 
+  // FIXME: This algorithm is not correct for module map hierarchies where
----------------
jansvoboda11 wrote:

> Does this cause a failure when it tries to include MSub1.modulemap? In some sense this scenario could be "fine" since you have all the modulemaps that contributed to the module definition.

Yes, exactly.

I thought this will break the AST file replay mode, but that doesn't actually reparse the module map file:

```
// RUN: %clang_cc1 -fmodules -fmodules-embed-all-files -emit-module -fmodule-name=M %t/M.modulemap -o %t/M.pcm
// RUN: rm %t/*.modulemap
// RUN: %clang_cc1 -E %t/M.pcm
```

One way of actually hitting this issue is compiling the module with a pruned file system that only contains files reported by the scanner. There are lots of different ways people compile modules, so maybe that is relevant.

> > However, cases like the following were broken before and will remain broken with this patch:
> 
> Would being lazier about the module definitions solve this?

What do you mean by this? Ignoring missing files in `extern X "<file>"` until we actually need `X`?

> 
> I'm not clear what the path to actually fixing these looks like

I'm a fan of defining this away by posing restrictions on the shape of the module map file hierarchies (so that they more ore less correspond to the module graph they describe).

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


More information about the cfe-commits mailing list