[PATCH] D58161: Print a note to the called macro when diagnosing err_embedded_directive

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: clang/lib/Lex/PPMacroExpansion.cpp:497
+
     Args = ReadMacroCallArgumentList(Identifier, MI, ExpansionEnd);
 
----------------
thakis wrote:
> rnk wrote:
> > Can this re-enter, something like:
> >   MACRO1(MACRO2(a1), MACRO3(
> >   #include "asdf"
> >   )
> > In this case, I think your diagnostic might do a null-deref. Maybe just add this test case and add a null test for it.
> This works fine:
> 
> ```
> #define MACRO1(a, b, c)
> #define MACRO2(a, b, c)
> #define MACRO3(a, b, c)
> 
> MACRO1(MACRO2(a1), MACRO3(
> #include "asdf"
> )
> 
> $ out/gn/bin/clang -c test.cc
> test.cc:30:2: error: embedding a #include directive within macro arguments is not supported
> #include "asdf"
>  ^
> test.cc:29:1: note: expansion of macro 'MACRO1' requested here
> MACRO1(MACRO2(a1), MACRO3(
> ^
> test.cc:29:1: error: unterminated function-like macro invocation
> MACRO1(MACRO2(a1), MACRO3(
> ^
> test.cc:25:9: note: macro 'MACRO1' defined here
> #define MACRO1(a, b, c)
>         ^
> 2 errors generated.
> ```
> 
> Note that the diag is only emitted if inMacroArgs is true, and ArgMacro is always set to non-nullptr exactly when that's true.
> 
> 
Right, so even if it could re-enter (sequence being "enter, enter, leave, directive, enter, leave, leave", we wouldn't emit this diagnostic, because InMacroArgs will be false.


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

https://reviews.llvm.org/D58161