jansvoboda11 requested review of this revision. jansvoboda11 added a comment.
Regarding the failing `Modules/crash-vfs-umbrella-frameworks.m` test, this is my understanding after debugging it. Whenever we find that `A.framework/Frameworks/B.framework` is a symlink, we treat `B` as a separate top-level framework, not as a subframework of `A`. We detect this in `::getTopFrameworkDir()` (in `HeaderSearch.cpp`) by calling `FileManager::getCanonicalPath()`, which resolves the symlink. We use the returned directory when calling the module map parser. Before this change, the `DirectoryEntry::getName()` ended up being the path Clang first accessed: `A.framework/Frameworks/B.framework`. After this change, it's the as-requested canonical path instead: `B.framework`. Since this canonicalization takes place and doesn't have any side-effect (I believe & hope), I think it should be fine to allow dropping the `A.framework/Frameworks/B.framework` directory from the reproducer VFS. Interestingly, before this change, the reproducer generated PCMs only for `A` and `I`, which makes me believe `B` ended up being treated as a subframework somehow (maybe the VFS interferes with the symlink canonicalization?). With this patch, the reproducer generates PCMs for all `A`, `B` and `I`, which matches the original (crashing) invocation on line 13. For the record, the test was introduced in D20194 <https://reviews.llvm.org/D20194>. @benlangmuir, does that sound like a reasonable thing to do? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151855/new/ https://reviews.llvm.org/D151855 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
