[PATCH] D54435: [VFS] Add "expand tilde" argument to getRealPath.

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added a comment.

In https://reviews.llvm.org/rL346453#296733, @sammccall wrote:

> I think this change was landed without review, and it's a complicated interface change, so I'm going to revert it for now.
>
> I also have some specific concerns about the design:
>
> - this doesn't seem to be a per-filesystem concern. e.g. in an overlaid filesystem, whose responsibility is it to resolve ~? It's not clear this belongs in the virtualization layer at all.


I'm not sure this is true. It the overlay specifies a different home directory (this functionality doesn't exist yet) it should take that one, otherwise it should defer to the underlying one.

> - implementing this is a substantial burden on VFSes, many of which (by their nature) live out-of-tree. It's not clear what the consequences of ignoring the parameter are, and what callers can expect.
> - use of optional-args in virtual methods would be nice to avoid
> - the use of boolean params like this in general doesn't scale well

Would a separate method address those 3 concerns? My goal was to keep things simple and mirror llvm's API, which helps with migrating between the two. I don't have a strong opinion on this though, as long as the functionality is available.

> Happy to help with review if you do want to move forward with this!

Thanks, I look forward to your input!


Repository:
  rL LLVM

https://reviews.llvm.org/D54435