benlangmuir added a comment.
In D134923#3825673 <https://reviews.llvm.org/D134923#3825673>, @jansvoboda11
wrote:
> IIUC this not only canonicalizes path to the main input file when compiling a
> PCM, but also `-fmodule-map-file=` arguments for dependencies, correct? Since
> that behavior is desirable, I think it would make sense to check for that in
> the test, WDYT?
Yep, makes sense to me!
================
Comment at: clang/lib/Lex/ModuleMap.cpp:1307
+ // Do not canonicalize within the framework; the module map parser expects
+ // Modules/ not Versions/A/Modules.
+ if (llvm::sys::path::filename(Dir) == "Modules") {
----------------
jansvoboda11 wrote:
> Is that because the parser is looking for headers in `../Headers` and
> `../PrivateHeaders`? If so, would it make sense to make the parser smarter
> and canonicalize even framework paths here?
The failure I am aware of is that `loadModuleMapFile` will not detect the
correct module `Directory`. I'm not sure if there are more failures after
that. I didn't want to complicate this logic, because it seems like there are
several possibilities:
* Modules/module.modulemap
* Versions/A/Modules/module.modulemap
* Versions/Current/Modules/module.modulemap
Also, if we get one of the versioned paths, but `Modules/module.modulemap` does
not point to that specific file then it would be incorrect to use it. I'm
inclined to leave this alone, at least for the current change.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134923/new/
https://reviews.llvm.org/D134923
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
