Lekensteyn added inline comments.
================ 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(); ---------------- joerg wrote: > Lekensteyn wrote: > > 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) > I disagree. I consider it a bug in GCC that it is a string prefix. It's quite > inconsistent as well. I agree with you, it should have been a directory prefix but GCC implements it as a string prefix although the GCC documents it as: "-fdebug-prefix-map=old=new When compiling files residing in **directory old**, record debugging information describing them as if the files resided in **directory new** instead." If you decide to fix `-fmacro-prefix-map` to use a directory prefix match, then the `-fdebug-prefix-map` should also be fixed for consistency. What about implementing the (buggy) GCC-compatible behavior first and then fixing both cases in a future patch? (I don't mind when the buggy behavior is fixed, I just want to see this functionality moving forward.) Another edge case that I ran into is when using the option to drop directories. When using `-ffile-prefix-map=/src=`, the command `cd /src && cc /src/foo.c` would have `__FILE__` equal to `/foo.c`. As a native "fix", one would try `-ffile-prefix-map=/src/=` which indeed produces `__FILE__` equal to `foo.c`. Matching with a trailing slash however fails to correctly remap some debug information, namely `DW_AT_comp_dir`. This contains the working directory (`/src`) which is not matched by `/src/`. By using a proper directory prefix match, this would be nicely fixed. Repository: rC Clang https://reviews.llvm.org/D49466 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
