[PATCH] D109128: [VFS] Use original path when falling back to external FS

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 14 11:59:53 PDT 2021


dexonsmith added a comment.

In D109128#3000157 <https://reviews.llvm.org/D109128#3000157>, @JDevlieghere wrote:

> Yes, Keith and I came to the same conclusion yesterday. I was worried about tracking both paths at all times, but I like your suggestion of only changing the path when requested.

Another idea that could be useful would be to add a type:

  struct CanonicalizedPath {
    StringRef OriginalPath;
    StringRef CanonicalPath;
  };

and add an overload for `vfs::FileSystem::openFileForRead` that takes this type.

- The default implementation in `vfs::FileSystem` could call `openFileForRead(.Canonical)` and then do a `File::getWithPath(.Original)` (as with my idea above, just when `File::getPath` matches `.Canonical` and `.Canonical!=.Original`).
- CanonicalizedPath-aware derived classes would invert the relationship. `openFileForRead(FilesystemLookupPath)` would use `CanonicalPath` for lookup, use `OriginalPath` for creating `File`, and pass through (a potentially updated!) `CanonicalizedPath` to base filesystems... and implement `openFileForRead(Twine)` by calling `canonicalizePath`.

Seems like a bit more code, but maybe not much, and the resulting logic might(?) be easier to reason about. (maybe a different name than openFileForRead would be better for clarity, rather than an overload?)

(Probably similar to your idea about tracking both paths at all times, but I'm not sure that needs to be mutually exclusive)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109128



More information about the llvm-commits mailing list