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

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added a comment.

In https://reviews.llvm.org/D54277#1293988, @vsapsai wrote:

> 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.


Yeah, I wasn't sure about this. Usually the client won't care what the error is, but there's no harm in returning an ErrorOr<>. I'll update the patch!


Repository:
  rL LLVM

https://reviews.llvm.org/D54277