[PATCH] D30668: Add llvm::sys::fs::real_path

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/lib/Support/Unix/Path.inc:845
+  std::error_code EC;
+  if (S[0] == '~') {
+    // open() doesn't accept tilde expressions, they are expanded by the shell.
----------------
rnk wrote:
> labath wrote:
> > I am wondering whether the tilde expansion should not be controlled by an argument. As the comment says, tilde expansion is done at a completely different level than path canonicalization. `~` is a perfectly valid path component and it's not clear to me that every user of this function would want the magic tilde expansion behavior as well. I think that can create some confusion, as this is not the behavior of any of the standard utilities and functions elsewhere (`realpath` unix utility, `realpath` POSIX function, llvm's `openFileForRead`, ...). For that reason it may be safer for the caller to explicitly opt-in into this behavior.
> Does the tilde expansion work on Windows? If not, let's leave it out like Pavel suggests.
No, but I thought he just suggested to make it opt-in rather than leave it out entirely.  The "equivalent" thing on Windows would be to expand environment variables.  That said, maybe there's a case to be made for allowing tilde expansion on Windows too.  I mean, Windows users do have a home directory, so it's not like it doesn't make sense at all.


https://reviews.llvm.org/D30668