kadircet added a comment. In D143093#4099623 <https://reviews.llvm.org/D143093#4099623>, @sammccall wrote:
> I can't understand from the description, code, or testcases what problem this > is fixing. Can you clarify, ideally by improving the testcases? Yeah should've elaborated on this one. As explained offline the issue is macro token contents are not preserved in the PCH and are re-lexed when needed (for diagnosing macro-redefined errors in our case). Since PCH contents are stale, when we retrieve token contents based on ranges stored in the PCH using the new file contents, we get a false-positive. This patch tries to avoid such false positive diagnostics by emitting an #undef statement before any macro definition we're going to emit. --- > It seems to introduce a false negative in the case that the preamble *does* > contain a definition of an already-defined macro, which probably needs to be > called out. I am failing to follow this one. I can think of two different cases: #define FOO 1 #define FOO 2 ; changes to #define BAR #define FOO 1 #define FOO 2 ; we won't have diagnostics here, not because of the `#undef` logic, but because we're not moving the diagnostic ranges around. Other case is we've #define FOO 1 ; #define FOO 2 and it gets changed to #define BAR #define FOO 1 ; #define FOO 2 now the issue is we're emitting re-definition diag to a location inside the preamble patch, but we don't translate the location back to main-file. again it has nothing to do with the `#undef` logic proposed here (and fixed by D143095 <https://reviews.llvm.org/D143095>). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143093/new/ https://reviews.llvm.org/D143093 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
