Page MenuHomePhabricator

Print a note to the called macro when diagnosing err_embedded_directive
ClosedPublic

Authored by thakis on Feb 12 2019, 5:33 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

thakis created this revision.Feb 12 2019, 5:33 PM
rnk added inline comments.Feb 12 2019, 5:49 PM
clang/lib/Lex/PPMacroExpansion.cpp
497 ↗(On Diff #186574)

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.

thakis marked an inline comment as done.Feb 12 2019, 6:46 PM
thakis added inline comments.
clang/lib/Lex/PPMacroExpansion.cpp
497 ↗(On Diff #186574)

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.

rnk accepted this revision.Feb 13 2019, 1:11 PM

lgtm

clang/lib/Lex/PPMacroExpansion.cpp
497 ↗(On Diff #186574)

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.

This revision is now accepted and ready to land.Feb 13 2019, 1:11 PM
thakis marked 2 inline comments as done.Feb 13 2019, 7:36 PM

Thanks!

clang/lib/Lex/PPMacroExpansion.cpp
497 ↗(On Diff #186574)

Since we don't have a stack of InMacroArgs, it looks like this always peels away the leftmost macro call, so enter is always followed by leave, and there's no enter/enter sequence. (I might be wrong.)

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 13 2019, 8:17 PM