[PATCH] D56769: Path: enhance prefix mapping
Dan McGregor via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 16 12:28:39 PST 2019
dankm marked 6 inline comments as done.
dankm added inline comments.
================
Comment at: lib/Support/Path.cpp:532
+ if ((is_separator(OldPrefix.back(), style) &&
+ !OrigPath.startswith(parent_path(OldPrefix, style))) ||
----------------
Lekensteyn wrote:
> Wouldn't this crash if `OldPrefix` is empty? Both if blocks should probably be wrapped in `if (!OldPrefix.empty()) {`
Yeah it should, but didn't for me when testing. Fixed in the new patch.
================
Comment at: lib/Support/Path.cpp:533
+ if ((is_separator(OldPrefix.back(), style) &&
+ !OrigPath.startswith(parent_path(OldPrefix, style))) ||
+ (!is_separator(OldPrefix.back(), style) &&
----------------
Lekensteyn wrote:
> Should return false for Path="/foobar" and OldPrefix="/foo/", but it does not.
Yup, changed all the logic here, and now it does what I expect.
================
Comment at: lib/Support/Path.cpp:557
+ while (is_separator(NewPath.back(), style))
+ NewPath.set_size(NewPath.size() - 1);
Path.swap(NewPath);
----------------
Lekensteyn wrote:
> Trimming *all* slashes is probably beyond the scope for this function.
Agreed, it's in the only consumer where this matters now.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56769/new/
https://reviews.llvm.org/D56769
More information about the llvm-commits
mailing list