Print a note to the called macro when diagnosing err_embedded_directive

[thakis]

Summary

Fixes PR40713, see there for the motivation for this.

[rnk]

clang/lib/Lex/PPMacroExpansion.cpp

497

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]

clang/lib/Lex/PPMacroExpansion.cpp

497

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]

lgtm

clang/lib/Lex/PPMacroExpansion.cpp

497

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.

[thakis]

Thanks!

clang/lib/Lex/PPMacroExpansion.cpp

497

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.)

clang/include/clang/Basic/DiagnosticLexKinds.td

def ext_missing_varargs_arg : Extension<
   "must specify at least one argument for '...' parameter of variadic macro">,
   InGroup<GNUZeroVariadicMacroArguments>;
 def ext_empty_fnmacro_arg : Extension<
   "empty macro arguments are a C99 feature">, InGroup<C99>;
 def warn_cxx98_compat_empty_fnmacro_arg : Warning<
   "empty macro arguments are incompatible with C++98">,
   InGroup<CXX98CompatPedantic>, DefaultIgnore;
 def note_macro_here : Note<"macro %0 defined here">;
+def note_macro_expansion_here : Note<"expansion of macro %0 requested here">;
 
 def err_pp_opencl_variadic_macros :
   Error<"variadic macros not supported in OpenCL">;
 
 def err_pp_invalid_directive : Error<"invalid preprocessing directive">;
 def err_pp_directive_required : Error<
   "%0 must be used within a preprocessing directive">;
 def err_pp_file_not_found : Error<"'%0' file not found">, DefaultFatal;

clang/include/clang/Lex/Preprocessor.h

class Preprocessor {
   IdentifierInfo *Ident__has_cpp_attribute;        // __has_cpp_attribute
   IdentifierInfo *Ident__has_c_attribute;          // __has_c_attribute
   IdentifierInfo *Ident__has_declspec;             // __has_declspec_attribute
   IdentifierInfo *Ident__is_target_arch;           // __is_target_arch
   IdentifierInfo *Ident__is_target_vendor;         // __is_target_vendor
   IdentifierInfo *Ident__is_target_os;             // __is_target_os
   IdentifierInfo *Ident__is_target_environment;    // __is_target_environment
 
+  // Weak, only valid (and set) while InMacroArgs is true.
+  Token* ArgMacro;
+
   SourceLocation DATELoc, TIMELoc;
 
   // Next __COUNTER__ value, starts at 0.
   unsigned CounterValue = 0;
 
   enum {
     /// Maximum depth of \#includes.
     MaxAllowedIncludeStackDepth = 200

clang/lib/Lex/PPDirectives.cpp

if (InMacroArgs) {
     if (IdentifierInfo *II = Result.getIdentifierInfo()) {
       switch (II->getPPKeywordID()) {
       case tok::pp_include:
       case tok::pp_import:
       case tok::pp_include_next:
       case tok::pp___include_macros:
       case tok::pp_pragma:
         Diag(Result, diag::err_embedded_directive) << II->getName();
+        Diag(*ArgMacro, diag::note_macro_expansion_here)
+            << ArgMacro->getIdentifierInfo();
         DiscardUntilEndOfDirective();
         return;
       default:
         break;
       }
     }
     Diag(Result, diag::ext_embedded_directive);
   }

clang/lib/Lex/PPMacroExpansion.cpp

bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier,
   SourceLocation ExpansionEnd = Identifier.getLocation();
 
   // If this is a function-like macro, read the arguments.
   if (MI->isFunctionLike()) {
     // Remember that we are now parsing the arguments to a macro invocation.
     // Preprocessor directives used inside macro arguments are not portable, and
     // this enables the warning.
     InMacroArgs = true;
+    ArgMacro = &Identifier;
+
     Args = ReadMacroCallArgumentList(Identifier, MI, ExpansionEnd);
 
     // Finished parsing args.
     InMacroArgs = false;
+    ArgMacro = nullptr;
 
     // If there was an error parsing the arguments, bail out.
     if (!Args) return true;
 
     ++NumFnMacroExpanded;
   } else {
     ++NumMacroExpanded;
   }

clang/lib/Lex/Preprocessor.cpp

Preprocessor::Preprocessor(std::shared_ptr<PreprocessorOptions> PPOpts,
   KeepComments = false;
   KeepMacroComments = false;
   SuppressIncludeNotFoundError = false;
 
   // Macro expansion is enabled.
   DisableMacroExpansion = false;
   MacroExpansionInDirectivesOverride = false;
   InMacroArgs = false;
+  ArgMacro = nullptr;
   InMacroArgPreExpansion = false;
   NumCachedTokenLexers = 0;
   PragmasEnabled = true;
   ParsingIfOrElifDirective = false;
   PreprocessedOutput = false;
 
   // We haven't read anything from the external source.
   ReadMacrosFromExternalSource = false;

clang/test/Preprocessor/macro_arg_directive.c

 // RUN: %clang_cc1 %s -fsyntax-only -verify
 
 #define a(x) enum { x }
 a(n =
 #undef a
 #define a 5
   a);
 _Static_assert(n == 5, "");
 
 #define M(A)
-M(
+M( // expected-note {{expansion of macro 'M' requested here}}
 #pragma pack(pop) // expected-error {{embedding a #pragma directive within macro arguments is not supported}}
 )
 
 // header1.h
 void fail(const char *);
 #define MUNCH(...) \
  ({ int result = 0; __VA_ARGS__; if (!result) { fail(#__VA_ARGS__); }; result })
 
 static inline int f(int k) {
-  return MUNCH( // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-error {{returning 'void'}}
+  return MUNCH( // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-error {{returning 'void'}} expected-note {{expansion of macro 'MUNCH' requested here}}
     if (k < 3)
       result = 24;
     else if (k > 4)
       result = k - 4;
 }
 
 #include "macro_arg_directive.h" // expected-error {{embedding a #include directive within macro arguments is not supported}}
 
 int g(int k) {
   return f(k) + f(k-1));
 }