[PATCH] D54277: Move RedirectingFileSystem interface into header.

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added a comment.

In D54277#1332261 <https://reviews.llvm.org/D54277#1332261>, @kristina wrote:

> I'm not sure if this is way out of scope for this patch, but if I understand correctly this could be used for extending it to, for example, handle things like Perforce-style paths which could indeed be useful, although I'm not sure if that is possible (ie. would `//depot/asdf` behave differently on Windows and Linux/Darwin/any other OS that uses Unix-style paths?). If this is with the intention to allow extending `RedirectingFileSystem` downstream then I think it may be worth splitting it up from the YAML based implementation altogether, allowing custom path handling where needed (as a separate downstream extension), with the concrete reference implementation of `RedirectingFileSystem` (the YAML based one) being a separate thing. This would make it more extensible from the POV of downstream forks that may desire to implement path redirection that is too complicated to do using YAML mappings, but still fundamentally rely on some sort of path remapping.
>
> Do you think it would be worth decoupling `RedirectingFileSystem` from YAML for such use cases and exposing both `RedirectingFileSystem` and, for your use case, something like `YAMLRedirectingFileSystem`? Just a stray thought, not really sure if it makes sense or is out of scope here (I don't believe it is, since if the interface is being exposed, it may be worth rethinking how it is layered if it's to be exposed through LLVMSupport headers).


It sounds like the scenario you're describing could be handled by just having a custom VFS implementation. What part of the RedirectingFileSystem would you repurpose for this? The reason I want to move it into the header is because I actually need the YAML stuff for LLDB's reproducers.


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

https://reviews.llvm.org/D54277