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

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn requested changes to this revision.
Lekensteyn added a comment.
This revision now requires changes to proceed.

The functionality looks correct to me, but could you include some tests in test/Driver/ and test/Preprocessor/ just to be sure?
test/Driver/debug-prefix-map.c and test/CodeGen/debug-prefix-map.c could serve as inspiration.

The documentation should probable be updated too: docs/ClangCommandLineReference.rst

(It would be nice to have this feature for Reproducible Builds)



================
Comment at: lib/Lex/PPMacroExpansion.cpp:1460
+  for (const auto &Entry : MacroPrefixMap)
+    if (Path.startswith(Entry.first))
+      return (Twine(Entry.second) + Path.substr(Entry.first.size())).str();
----------------
dankm wrote:
> dankm wrote:
> > joerg wrote:
> > > This doesn't handle directory vs string prefix prefix correctly, does it? At the very least, it should have a test case :)
> > Good catch. I expect not. But on the other hand, it's exactly what debug-prefix-map does :)
> > 
> > I'll add test cases in a future review. My first goal was getting something sort-of working.
> There should be a test, but apparently the debug prefix map code also does this.
> 
> What do you think the correct behaviour should be? a string prefix, or a directory prefix?
It should be a string prefix (like GCC)


Repository:
  rC Clang

https://reviews.llvm.org/D49466