Refactoring the earlier patch to take into account comments.
- Instead of attempting a correction after every failure, only try after a
"too many arguments" failure.
- Rewrite the code that generates a new vector of token arguments to use the
old vector of tokens. This cuts back the amount of token copying to only when
a correction is attempted.
- Give a note on all arguments started with a brace list instead of only when
the brace list is the entire token.
http://llvm-reviews.chandlerc.com/D986
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D986?vs=2432&id=2920#toc
Files:
test/Preprocessor/macro_fn.c
test/Preprocessor/macro_with_initializer_list.cpp
include/clang/Basic/DiagnosticLexKinds.td
lib/Lex/PPMacroExpansion.cpp
Index: test/Preprocessor/macro_fn.c
===================================================================
--- test/Preprocessor/macro_fn.c
+++ 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}} */
Index: test/Preprocessor/macro_with_initializer_list.cpp
===================================================================
--- test/Preprocessor/macro_with_initializer_list.cpp
+++ test/Preprocessor/macro_with_initializer_list.cpp
@@ -0,0 +1,164 @@
+// 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.
+// TODO: Add note for initializer list at beginning of macro argument.
+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":{146:11-146:11}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{146:23-146:23}:")"
Index: include/clang/Basic/DiagnosticLexKinds.td
===================================================================
--- include/clang/Basic/DiagnosticLexKinds.td
+++ 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<
Index: lib/Lex/PPMacroExpansion.cpp
===================================================================
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -390,6 +390,140 @@
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, a simple count instead of
+ // a stack is needed.
+ unsigned Braces = 0;
+ unsigned Parens = 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::l_paren)) {
+ ++Parens;
+ } else if (I->is(tok::r_paren)) {
+ --Parens;
+ } 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,
+ 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
@@ -416,15 +550,16 @@
bool ContainsCodeCompletionTok = false;
unsigned NumActuals = 0;
+ SourceLocation ArgStartLoc;
while (Tok.isNot(tok::r_paren)) {
if (ContainsCodeCompletionTok && (Tok.is(tok::eof) || Tok.is(tok::eod)))
break;
assert((Tok.is(tok::l_paren) || Tok.is(tok::comma)) &&
"only expect argument separators here");
unsigned ArgTokenStart = ArgTokens.size();
- SourceLocation ArgStartLoc = Tok.getLocation();
+ ArgStartLoc = Tok.getLocation();
// C99 6.10.3p11: Keep track of the number of l_parens we have seen. Note
// that we already consumed the first one.
@@ -504,22 +639,12 @@
// If this is not a variadic macro, and too many args were specified, emit
// an error.
- if (!isVariadic && NumFixedArgsLeft == 0) {
- 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;
- }
- }
+ if (!isVariadic && NumFixedArgsLeft == 0 &&
+ ArgTokens.size() != ArgTokenStart)
+ ArgStartLoc = ArgTokens[ArgTokenStart].getLocation();
- // 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,59 @@
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(ArgStartLoc, 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)
+ 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) {
+ DB << FixItHint::CreateInsertion(ParenLocation->getBegin(), "(")
+ << FixItHint::CreateInsertion(ParenLocation->getEnd(), ")");
+ }
+ ArgTokens.swap(FixedArgTokens);
+ NumActuals = FixedNumArgs;
+ }
+
// See MacroArgs instance var for description of this.
bool isVarargsElided = false;
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
