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

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 7 04:00:02 PST 2017


labath added a comment.

I am not sure whether having things work "exactly like from a shell" is a feature (*), as there are other utilities doing similar things which don't do tilde expansion. Maybe I am being too pedantic, but I think it's a point worth bringing up.

(*) The rules for tilde expansion in posix shells are also quite tricky. E.g. `~/foo` expands `~`, but `"~/foo"` does not (but `"$HOME/foo"` expands `$HOME`, ...).



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


https://reviews.llvm.org/D30668





More information about the llvm-commits mailing list