[PATCH] D122549: [VFS] RedirectingFileSystem only replace path if not already mapped

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D122549#3412021 <https://reviews.llvm.org/D122549#3412021>, @bnbarham wrote:

> `clang-apply-replacements/relative-paths.cpp` is failing, I haven't looked into it but my guess would be that it's from the `Status.getName() == Filename` -> `!Status.IsVFSMapped` change. That seems very odd to me.

Is it just failing on Windows? I wonder (rather speculatively...) whether https://reviews.llvm.org/D121733 would help.



================
Comment at: clang/lib/Basic/FileManager.cpp:316-318
+    // case above is removed. That one can be removed once we always return
+    // virtual paths and anywhere that needs external paths specifically
+    // requests them.
----------------
bnbarham wrote:
> dexonsmith wrote:
> > It's not obvious to me that the redirection case above depends on this logic sticking around.
> > - If that's what you mean, can you explain in more detail why the redirection above depends on this `UFE.Dir` logic?
> > - If you're just talking about `IsVFSMapped` having to stick around, I think that part should be covered by the comment for the redirection.
> I actually meant the reverse - the UFE.Dir logic being removed depends on the redirection logic above being removed. That's because for this to be removed, anywhere that cares about the requested path needs to be changed to use Refs. But even if they change to use Refs, the redirection logic above would itself replace the path with the external one. So it needs to be removed as well.
Okay, I suggest just making the link between them more clear.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122549/new/

https://reviews.llvm.org/D122549