[PATCH] D54277: Extend VFS with function to get external path.

[Volodymyr Sapsai via Phabricator via llvm-commits]

vsapsai added a comment.

In https://reviews.llvm.org/D54277#1292267, @JDevlieghere wrote:

> In https://reviews.llvm.org/D54277#1292180, @vsapsai wrote:
>
> > How do you want it to work with symlinks? Would `getRealPath` be sufficient for your purpose?
>
>
> I'm not sure how those two are related, other than the confusing name. As far as I can tell this only wraps the `realpath(3) ` call, which is definitely much too expensive to do in the "real" file system case. For symlinks I wouldn't really care, it's up to the user to resolve them if they matter.


OK. Now I understand that you prefer to avoid `realpath` as it is too expensive for this purpose.

Do you think `Optional` is sufficient for handling errors? On one hand it is a simpler interface but on the other hand it loses information why external path might be unavailable.



================
Comment at: lib/Support/VirtualFileSystem.cpp:1906-1907
 
+Optional<std::string>
+RedirectingFileSystem::getExternalPath(const Twine &Path) {
+  ErrorOr<Entry *> E = lookupPath(Path);
----------------
Need to double check but at the first glance I have reservations about a case with nested  `RedirectingFileSystem`. Should `getExternalPath` go only through 1 level of indirection or all the way to real file system?


Repository:
  rL LLVM

https://reviews.llvm.org/D54277