[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

[Dan McGregor via Phabricator via cfe-commits]

dankm marked 2 inline comments as done.
dankm added inline comments.


================
Comment at: llvm/include/llvm/Support/Path.h:172
+/// @param style The path separator style
+/// @param strict Strict prefix path checking
+/// @result true if \a Path begins with OldPrefix
----------------
Lekensteyn wrote:
> "strict checking" is ambiguous on its own. What about something like:
> 
>     If strict is true, a directory separator following \a OldPrefix will also be stripped. Otherwise, directory separators will only be matched and stripped when present in \a OldPrefix.
> 
> Or whatever semantics you would like to assign to "strict mode".
Thanks. I like your wording, and I've used it in the incoming patch.


================
Comment at: llvm/include/llvm/Support/Path.h:181
+  return replace_path_prefix(Path, OldPrefix, NewPrefix, style, strict);
+}
 
----------------
Lekensteyn wrote:
> Why have a variant with the parameters swapped, is it common in LLVM to have such convenience wrappers?
> 
> Why not require callers to pass `Style::native` whenever they want to modify "strict"?
Works for me. I did that to make my call sites somewhat more readable. It'll be changed too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466