[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