Add new diagnostic messages when too many arguments are presented to a
function-like macro. Clang will attempt to correct the arguments by detecting
braced initializer lists:
1) If possible, suggest parentheses around arguments
containing braced lists which will give the proper number of arguments.
2) If a braced list is detected at the start of a macro argument, it cannot be
corrected by parentheses. Instead, just point out the location of these
braced lists.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@186971 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/Basic/Diagnostic.h b/include/clang/Basic/Diagnostic.h
index 0fcfdf7..f2c3303 100644
--- a/include/clang/Basic/Diagnostic.h
+++ b/include/clang/Basic/Diagnostic.h
@@ -990,6 +990,10 @@
bool hasMaxRanges() const {
return NumRanges == DiagnosticsEngine::MaxRanges;
}
+
+ bool hasMaxFixItHints() const {
+ return NumFixits == DiagnosticsEngine::MaxFixItHints;
+ }
};
inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
diff --git a/include/clang/Basic/DiagnosticLexKinds.td b/include/clang/Basic/DiagnosticLexKinds.td
index 3639024..d7b35c9 100644
--- a/include/clang/Basic/DiagnosticLexKinds.td
+++ b/include/clang/Basic/DiagnosticLexKinds.td
@@ -460,6 +460,11 @@
"unterminated function-like macro invocation">;
def err_too_many_args_in_macro_invoc : Error<
"too many arguments provided to function-like macro invocation">;
+def note_suggest_parens_for_macro : Note<
+ "parentheses are required around macro argument containing braced "
+ "initializer list">;
+def note_init_list_at_beginning_of_macro_argument : Note<
+ "cannot use initializer list at the beginning of an macro argument">;
def err_too_few_args_in_macro_invoc : Error<
"too few arguments provided to function-like macro invocation">;
def err_pp_bad_paste : Error<
diff --git a/lib/Lex/PPMacroExpansion.cpp b/lib/Lex/PPMacroExpansion.cpp
index d21515c..8dccae1 100644
--- a/lib/Lex/PPMacroExpansion.cpp
+++ b/lib/Lex/PPMacroExpansion.cpp
@@ -390,6 +390,136 @@
return false;
}
+enum Bracket {
+ Brace,
+ Paren
+};
+
+/// CheckMatchedBrackets - Returns true if the braces and parentheses in the
+/// token vector are properly nested.
+static bool CheckMatchedBrackets(const SmallVectorImpl<Token> &Tokens) {
+ SmallVector<Bracket, 8> Brackets;
+ for (SmallVectorImpl<Token>::const_iterator I = Tokens.begin(),
+ E = Tokens.end();
+ I != E; ++I) {
+ if (I->is(tok::l_paren)) {
+ Brackets.push_back(Paren);
+ } else if (I->is(tok::r_paren)) {
+ if (Brackets.empty() || Brackets.back() == Brace)
+ return false;
+ Brackets.pop_back();
+ } else if (I->is(tok::l_brace)) {
+ Brackets.push_back(Brace);
+ } else if (I->is(tok::r_brace)) {
+ if (Brackets.empty() || Brackets.back() == Paren)
+ return false;
+ Brackets.pop_back();
+ }
+ }
+ if (!Brackets.empty())
+ return false;
+ return true;
+}
+
+/// GenerateNewArgTokens - Returns true if OldTokens can be converted to a new
+/// vector of tokens in NewTokens. The new number of arguments will be placed
+/// in NumArgs and the ranges which need to surrounded in parentheses will be
+/// in ParenHints.
+/// Returns false if the token stream cannot be changed. If this is because
+/// of an initializer list starting a macro argument, the range of those
+/// initializer lists will be place in InitLists.
+static bool GenerateNewArgTokens(Preprocessor &PP,
+ SmallVectorImpl<Token> &OldTokens,
+ SmallVectorImpl<Token> &NewTokens,
+ unsigned &NumArgs,
+ SmallVectorImpl<SourceRange> &ParenHints,
+ SmallVectorImpl<SourceRange> &InitLists) {
+ if (!CheckMatchedBrackets(OldTokens))
+ return false;
+
+ // Once it is known that the brackets are matched, only a simple count of the
+ // braces is needed.
+ unsigned Braces = 0;
+
+ // First token of a new macro argument.
+ SmallVectorImpl<Token>::iterator ArgStartIterator = OldTokens.begin();
+
+ // First closing brace in a new macro argument. Used to generate
+ // SourceRanges for InitLists.
+ SmallVectorImpl<Token>::iterator ClosingBrace = OldTokens.end();
+ NumArgs = 0;
+ Token TempToken;
+ // Set to true when a macro separator token is found inside a braced list.
+ // If true, the fixed argument spans multiple old arguments and ParenHints
+ // will be updated.
+ bool FoundSeparatorToken = false;
+ for (SmallVectorImpl<Token>::iterator I = OldTokens.begin(),
+ E = OldTokens.end();
+ I != E; ++I) {
+ if (I->is(tok::l_brace)) {
+ ++Braces;
+ } else if (I->is(tok::r_brace)) {
+ --Braces;
+ if (Braces == 0 && ClosingBrace == E && FoundSeparatorToken)
+ ClosingBrace = I;
+ } else if (I->is(tok::eof)) {
+ // EOF token is used to separate macro arguments
+ if (Braces != 0) {
+ // Assume comma separator is actually braced list separator and change
+ // it back to a comma.
+ FoundSeparatorToken = true;
+ I->setKind(tok::comma);
+ I->setLength(1);
+ } else { // Braces == 0
+ // Separator token still separates arguments.
+ ++NumArgs;
+
+ // If the argument starts with a brace, it can't be fixed with
+ // parentheses. A different diagnostic will be given.
+ if (FoundSeparatorToken && ArgStartIterator->is(tok::l_brace)) {
+ InitLists.push_back(
+ SourceRange(ArgStartIterator->getLocation(),
+ PP.getLocForEndOfToken(ClosingBrace->getLocation())));
+ ClosingBrace = E;
+ }
+
+ // Add left paren
+ if (FoundSeparatorToken) {
+ TempToken.startToken();
+ TempToken.setKind(tok::l_paren);
+ TempToken.setLocation(ArgStartIterator->getLocation());
+ TempToken.setLength(0);
+ NewTokens.push_back(TempToken);
+ }
+
+ // Copy over argument tokens
+ NewTokens.insert(NewTokens.end(), ArgStartIterator, I);
+
+ // Add right paren and store the paren locations in ParenHints
+ if (FoundSeparatorToken) {
+ SourceLocation Loc = PP.getLocForEndOfToken((I - 1)->getLocation());
+ TempToken.startToken();
+ TempToken.setKind(tok::r_paren);
+ TempToken.setLocation(Loc);
+ TempToken.setLength(0);
+ NewTokens.push_back(TempToken);
+ ParenHints.push_back(SourceRange(ArgStartIterator->getLocation(),
+ Loc));
+ }
+
+ // Copy separator token
+ NewTokens.push_back(*I);
+
+ // Reset values
+ ArgStartIterator = I + 1;
+ FoundSeparatorToken = false;
+ }
+ }
+ }
+
+ return !ParenHints.empty() && InitLists.empty();
+}
+
/// ReadFunctionLikeMacroArgs - After reading "MACRO" and knowing that the next
/// token is the '(' of the macro, this method is invoked to read all of the
/// actual arguments specified for the macro invocation. This returns null on
@@ -415,6 +545,8 @@
SmallVector<Token, 64> ArgTokens;
bool ContainsCodeCompletionTok = false;
+ SourceLocation TooManyArgsLoc;
+
unsigned NumActuals = 0;
while (Tok.isNot(tok::r_paren)) {
if (ContainsCodeCompletionTok && (Tok.is(tok::eof) || Tok.is(tok::eod)))
@@ -504,22 +636,15 @@
// If this is not a variadic macro, and too many args were specified, emit
// an error.
- if (!isVariadic && NumFixedArgsLeft == 0) {
+ if (!isVariadic && NumFixedArgsLeft == 0 && TooManyArgsLoc.isInvalid()) {
if (ArgTokens.size() != ArgTokenStart)
- ArgStartLoc = ArgTokens[ArgTokenStart].getLocation();
-
- if (!ContainsCodeCompletionTok) {
- // Emit the diagnostic at the macro name in case there is a missing ).
- // Emitting it at the , could be far away from the macro name.
- Diag(ArgStartLoc, diag::err_too_many_args_in_macro_invoc);
- Diag(MI->getDefinitionLoc(), diag::note_macro_here)
- << MacroName.getIdentifierInfo();
- return 0;
- }
+ TooManyArgsLoc = ArgTokens[ArgTokenStart].getLocation();
+ else
+ TooManyArgsLoc = ArgStartLoc;
}
- // Empty arguments are standard in C99 and C++0x, and are supported as an extension in
- // other modes.
+ // Empty arguments are standard in C99 and C++0x, and are supported as an
+ // extension in other modes.
if (ArgTokens.size() == ArgTokenStart && !LangOpts.C99)
Diag(Tok, LangOpts.CPlusPlus11 ?
diag::warn_cxx98_compat_empty_fnmacro_arg :
@@ -533,16 +658,66 @@
EOFTok.setLength(0);
ArgTokens.push_back(EOFTok);
++NumActuals;
- if (!ContainsCodeCompletionTok || NumFixedArgsLeft != 0) {
- assert(NumFixedArgsLeft != 0 && "Too many arguments parsed");
+ if (!ContainsCodeCompletionTok && NumFixedArgsLeft != 0)
--NumFixedArgsLeft;
- }
}
// Okay, we either found the r_paren. Check to see if we parsed too few
// arguments.
unsigned MinArgsExpected = MI->getNumArgs();
+ // If this is not a variadic macro, and too many args were specified, emit
+ // an error.
+ if (!isVariadic && NumActuals > MinArgsExpected &&
+ !ContainsCodeCompletionTok) {
+ // Emit the diagnostic at the macro name in case there is a missing ).
+ // Emitting it at the , could be far away from the macro name.
+ Diag(TooManyArgsLoc, diag::err_too_many_args_in_macro_invoc);
+ Diag(MI->getDefinitionLoc(), diag::note_macro_here)
+ << MacroName.getIdentifierInfo();
+
+ // Commas from braced initializer lists will be treated as argument
+ // separators inside macros. Attempt to correct for this with parentheses.
+ // TODO: See if this can be generalized to angle brackets for templates
+ // inside macro arguments.
+
+ SmallVector<Token, 64> FixedArgTokens;
+ unsigned FixedNumArgs = 0;
+ SmallVector<SourceRange, 4> ParenHints, InitLists;
+ if (!GenerateNewArgTokens(*this, ArgTokens, FixedArgTokens, FixedNumArgs,
+ ParenHints, InitLists)) {
+ if (!InitLists.empty()) {
+ DiagnosticBuilder DB =
+ Diag(MacroName,
+ diag::note_init_list_at_beginning_of_macro_argument);
+ for (SmallVector<SourceRange, 4>::iterator
+ Range = InitLists.begin(), RangeEnd = InitLists.end();
+ Range != RangeEnd; ++Range) {
+ if (DB.hasMaxRanges())
+ break;
+ DB << *Range;
+ }
+ }
+ return 0;
+ }
+ if (FixedNumArgs != MinArgsExpected)
+ return 0;
+
+ DiagnosticBuilder DB = Diag(MacroName, diag::note_suggest_parens_for_macro);
+ for (SmallVector<SourceRange, 4>::iterator
+ ParenLocation = ParenHints.begin(), ParenEnd = ParenHints.end();
+ ParenLocation != ParenEnd; ++ParenLocation) {
+ if (DB.hasMaxFixItHints())
+ break;
+ DB << FixItHint::CreateInsertion(ParenLocation->getBegin(), "(");
+ if (DB.hasMaxFixItHints())
+ break;
+ DB << FixItHint::CreateInsertion(ParenLocation->getEnd(), ")");
+ }
+ ArgTokens.swap(FixedArgTokens);
+ NumActuals = FixedNumArgs;
+ }
+
// See MacroArgs instance var for description of this.
bool isVarargsElided = false;
diff --git a/test/Preprocessor/macro_fn.c b/test/Preprocessor/macro_fn.c
index fcdb90a..f21ef51 100644
--- a/test/Preprocessor/macro_fn.c
+++ b/test/Preprocessor/macro_fn.c
@@ -13,7 +13,8 @@
one() /* ok */
one(a)
-one(a,) /* expected-error {{too many arguments provided to function-like macro invocation}} */
+one(a,) /* expected-error {{too many arguments provided to function-like macro invocation}} \
+ expected-warning {{empty macro arguments are a C99 feature}}*/
one(a, b) /* expected-error {{too many arguments provided to function-like macro invocation}} */
two() /* expected-error {{too few arguments provided to function-like macro invocation}} */
@@ -25,7 +26,7 @@
, /* expected-warning {{empty macro arguments are a C99 feature}} */
, /* expected-warning {{empty macro arguments are a C99 feature}} \
expected-error {{too many arguments provided to function-like macro invocation}} */
- )
+ ) /* expected-warning {{empty macro arguments are a C99 feature}} */
two(,) /* expected-warning 2 {{empty macro arguments are a C99 feature}} */
diff --git a/test/Preprocessor/macro_with_initializer_list.cpp b/test/Preprocessor/macro_with_initializer_list.cpp
new file mode 100644
index 0000000..4f5bf6d
--- /dev/null
+++ b/test/Preprocessor/macro_with_initializer_list.cpp
@@ -0,0 +1,182 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+namespace std {
+ template <class X>
+ class initializer_list {
+ public:
+ initializer_list();
+ };
+}
+
+class Foo {
+public:
+ Foo();
+ Foo(std::initializer_list<int>);
+ bool operator==(const Foo);
+ Foo operator+(const Foo);
+};
+
+#define EQ(x,y) (void)(x == y) // expected-note 6{{defined here}}
+void test_EQ() {
+ Foo F;
+ F = Foo{1,2};
+
+ EQ(F,F);
+ EQ(F,Foo());
+ EQ(F,Foo({1,2,3}));
+ EQ(Foo({1,2,3}),F);
+ EQ((Foo{1,2,3}),(Foo{1,2,3}));
+ EQ(F, F + F);
+ EQ(F, Foo({1,2,3}) + Foo({1,2,3}));
+ EQ(F, (Foo{1,2,3} + Foo{1,2,3}));
+
+ EQ(F,Foo{1,2,3});
+ // expected-error@-1 {{too many arguments provided}}
+ // expected-note@-2 {{parentheses are required}}
+ EQ(Foo{1,2,3},F);
+ // expected-error@-1 {{too many arguments provided}}
+ // expected-note@-2 {{parentheses are required}}
+ EQ(Foo{1,2,3},Foo{1,2,3});
+ // expected-error@-1 {{too many arguments provided}}
+ // expected-note@-2 {{parentheses are required}}
+
+ EQ(Foo{1,2,3} + Foo{1,2,3}, F);
+ // expected-error@-1 {{too many arguments provided}}
+ // expected-note@-2 {{parentheses are required}}
+ EQ(F, Foo({1,2,3}) + Foo{1,2,3});
+ // expected-error@-1 {{too many arguments provided}}
+ // expected-note@-2 {{parentheses are required}}
+ EQ(F, Foo{1,2,3} + Foo{1,2,3});
+ // expected-error@-1 {{too many arguments provided}}
+ // expected-note@-2 {{parentheses are required}}
+}
+
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{33:8-33:8}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{33:18-33:18}:")"
+
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{36:6-36:6}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{36:16-36:16}:")"
+
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{39:6-39:6}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{39:16-39:16}:")"
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{39:17-39:17}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{39:27-39:27}:")"
+
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{43:6-43:6}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{43:29-43:29}:")"
+
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{46:9-46:9}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{46:34-46:34}:")"
+
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{49:9-49:9}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{49:32-49:32}:")"
+
+#define NE(x,y) (void)(x != y) // expected-note 6{{defined here}}
+// Operator != isn't defined. This tests that the corrected macro arguments
+// are used in the macro expansion.
+void test_NE() {
+ Foo F;
+
+ NE(F,F);
+ // expected-error@-1 {{invalid operands}}
+ NE(F,Foo());
+ // expected-error@-1 {{invalid operands}}
+ NE(F,Foo({1,2,3}));
+ // expected-error@-1 {{invalid operands}}
+ NE((Foo{1,2,3}),(Foo{1,2,3}));
+ // expected-error@-1 {{invalid operands}}
+
+ NE(F,Foo{1,2,3});
+ // expected-error@-1 {{too many arguments provided}}
+ // expected-note@-2 {{parentheses are required}}
+ // expected-error@-3 {{invalid operands}}
+ NE(Foo{1,2,3},F);
+ // expected-error@-1 {{too many arguments provided}}
+ // expected-note@-2 {{parentheses are required}}
+ // expected-error@-3 {{invalid operands}}
+ NE(Foo{1,2,3},Foo{1,2,3});
+ // expected-error@-1 {{too many arguments provided}}
+ // expected-note@-2 {{parentheses are required}}
+ // expected-error@-3 {{invalid operands}}
+
+ NE(Foo{1,2,3} + Foo{1,2,3}, F);
+ // expected-error@-1 {{too many arguments provided}}
+ // expected-note@-2 {{parentheses are required}}
+ // expected-error@-3 {{invalid operands}}
+ NE(F, Foo({1,2,3}) + Foo{1,2,3});
+ // expected-error@-1 {{too many arguments provided}}
+ // expected-note@-2 {{parentheses are required}}
+ // expected-error@-3 {{invalid operands}}
+ NE(F, Foo{1,2,3} + Foo{1,2,3});
+ // expected-error@-1 {{too many arguments provided}}
+ // expected-note@-2 {{parentheses are required}}
+ // expected-error@-3 {{invalid operands}}
+}
+
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{89:8-89:8}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{89:18-89:18}:")"
+
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{93:6-93:6}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{93:16-93:16}:")"
+
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{97:6-97:6}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{97:16-97:16}:")"
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{97:17-97:17}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{97:27-97:27}:")"
+
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{102:6-102:6}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{102:29-102:29}:")"
+
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{106:9-106:9}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{106:34-106:34}:")"
+
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{110:9-110:9}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{110:32-110:32}:")"
+
+#define INIT(var, init) Foo var = init; // expected-note 3{{defined here}}
+// Can't use an initializer list as a macro argument. The commas in the list
+// will be interpretted as argument separaters and adding parenthesis will
+// make it no longer an initializer list.
+void test() {
+ INIT(a, Foo());
+ INIT(b, Foo({1, 2, 3}));
+ INIT(c, Foo());
+
+ INIT(d, Foo{1, 2, 3});
+ // expected-error@-1 {{too many arguments provided}}
+ // expected-note@-2 {{parentheses are required}}
+
+ // Can't be fixed by parentheses.
+ INIT(e, {1, 2, 3});
+ // expected-error@-1 {{too many arguments provided}}
+ // expected-error@-2 {{use of undeclared identifier}}
+ // expected-note@-3 {{cannot use initializer list at the beginning of an macro argument}}
+
+ // Can't be fixed by parentheses.
+ INIT(e, {1, 2, 3} + {1, 2, 3});
+ // expected-error@-1 {{too many arguments provided}}
+ // expected-error@-2 {{use of undeclared identifier}}
+ // expected-note@-3 {{cannot use initializer list at the beginning of an macro argument}}
+}
+
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{145:11-145:11}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{145:23-145:23}:")"
+
+#define M(name,a,b,c,d,e,f,g,h,i,j,k,l) \
+ Foo name = a + b + c + d + e + f + g + h + i + j + k + l;
+// expected-note@-2 2{{defined here}}
+void test2() {
+ M(F1, Foo(), Foo(), Foo(), Foo(), Foo(), Foo(),
+ Foo(), Foo(), Foo(), Foo(), Foo(), Foo());
+
+ M(F2, Foo{1,2,3}, Foo{1,2,3}, Foo{1,2,3}, Foo{1,2,3}, Foo{1,2,3}, Foo{1,2,3},
+ Foo{1,2,3}, Foo{1,2,3}, Foo{1,2,3}, Foo{1,2,3}, Foo{1,2,3}, Foo{1,2,3});
+ // expected-error@-2 {{too many arguments provided}}
+ // expected-note@-3 {{parentheses are required}}
+
+ M(F3, {1,2,3}, {1,2,3}, {1,2,3}, {1,2,3}, {1,2,3}, {1,2,3},
+ {1,2,3}, {1,2,3}, {1,2,3}, {1,2,3}, {1,2,3}, {1,2,3});
+ // expected-error@-2 {{too many arguments provided}}
+ // expected-error@-3 {{use of undeclared identifier}}
+ // expected-note@-4 {{cannot use initializer list at the beginning of an macro argument}}
+}
