[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 14:58:58 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:

The whole concept of mapping modules to module map files is broken. However, we would previously correctly mark `MSub1.modulemap` as affecting and we won't with this patch:

```
//--- M.modulemap
module M {}
extern module M.Sub "MSub1.modulemap"
//--- MSub1.modulemap
extern module M.Sub "MSub2.modulemap"
//--- MSub2.modulemap
module M.Sub {}
```

Previously we would walk up the module map file include hierarchy from `MSub2.modulemap` to `MSub1.modulemap` to `M.modulemap`.

Now, we will only walk the module hierarchy and only mark the defining `MSub2.modulemap` and `M.modulemap` as affecting, effectively creating a "hole" in the tree of module map file includes by omitting `MSub1.modulemap`.

Similar thing happens in this case (assuming `M` is affecting and `N` is not):

```
//--- N.modulemap
module N {}
extern module N.Sub "NSub.modulemap"
//--- NSub.modulemap
module N.Sub {}
module M {}
```

---

However, cases like the following were broken before and will remain broken with this patch:

```
//--- M.modulemap
module M {}
extern module M.Sub "MSub.modulemap"
//--- MSub.modulemap
module M.Sub {}
extern module N "N.modulemap"
//--- N.modulemap
module N {}
```

```
//--- M.modulemap
module M {}
module N {}
extern module M.Sub "MSub.modulemap"
extern module N.Sub "NSub.modulemap"
//--- MSub.modulemap
module M.Sub {}
//--- NSub.modulemap
module N.Sub {}
```

etc...

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


More information about the cfe-commits mailing list