[PATCH] D56769: Path: enhance prefix mapping

[Dan McGregor via Phabricator via llvm-commits]

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