[PATCH] D56545: [VFS] Allow multiple RealFileSystem instances with independent CWDs.

Jonas Devlieghere via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 10 09:53:23 PST 2019


JDevlieghere accepted this revision.
JDevlieghere added a comment.

In D56545#1352995 <https://reviews.llvm.org/D56545#1352995>, @sammccall wrote:

> In D56545#1352933 <https://reviews.llvm.org/D56545#1352933>, @JDevlieghere wrote:
>
> > Why did you choose to make this a property of the RealFileSystem as opposed to subclassing the RealFileSystem to a PhysicalFileSystem? It seems like the latter would be more in line with the VFS design.
>
>
> The reason the logic is interleaved like this: the operations aren't a simple "translation" into equivalent operations with absolute paths. Note that the returned files and statuses must report the relative paths provided, not the absolute ones we translate the actual IO into. So building the FS-with-own-WD as an aggregate or derived class of the FS-with-native-WD is going to be awkward and fragile. (And semantically, inheritance seems dubious here).
>  This could be a hierarchy of three classes where the abstract base tries not to express an opinion (or CRTP, or a template over a policy object), but I didn't find a pattern that seemed less confusing/indirect than just using a single class with if(). I don't expect the performance aspect of having to branch to matter at all.


Fair enough, thanks for the explanation!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56545





More information about the llvm-commits mailing list