bnbarham added inline comments.
================
Comment at: clang/lib/Basic/FileManager.cpp:273
- if (Status.getName() == Filename) {
+ if (!Status.IsVFSMapped) {
// The name matches. Set the FileEntry.
----------------
bnbarham wrote:
> dexonsmith wrote:
> > An incremental patch you could try would be:
> > ```
> > lang=c++
> > if (Status.getName() == Filename || !Status.IsVFSMapped)
> > ```
> > ... dropping all the other changes.
> >
> > This limits the redirection hack to only apply when a RedirectingFS is
> > involved (leaving until later the fine-tuning of when `IsVFSMapped` gets
> > set). If this smaller change still causes a test failure, it might be
> > easier to reason about why / how to fix it / be sure that the fix is sound.
> >
> > Eventually we might want something like:
> > ```
> > lang=c++
> > if (!Status.IsVFSMapped) {
> > assert(Status.getName() == Filename);
> > ```
> > I imagine that's not going to succeed yet due to the CWD behaviour in the
> > VFSes, but as a speculative patch it might help track down whatever the
> > problem is.
> That assertion currently won't succeed for any relative path, since
> `getStatValue` does the absolute pathing for relative paths. So
> `Status.getName()` is guaranteed to be different to `Filename` in that case.
>
> Possibly even more odd is that the failing test passes locally on MacOSX.
> I'll try the suggestion above and see if they fail again.
Actually, thinking about this a bit more - the failing test doesn't use an
overlay at all, so changing to the above wouldn't help at all.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122549/new/
https://reviews.llvm.org/D122549
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
