From 79b45389c3cd2f1ab0904b636f8451849038cddf Mon Sep 17 00:00:00 2001 From: Richard Trieu Date: Tue, 23 Jul 2013 18:01:49 +0000 Subject: [PATCH] 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. llvm-svn: 186971 --- clang/include/clang/Basic/Diagnostic.h | 4 + clang/include/clang/Basic/DiagnosticLexKinds.td | 5 + clang/lib/Lex/PPMacroExpansion.cpp | 207 +++++++++++++++++++-- clang/test/Preprocessor/macro_fn.c | 5 +- .../Preprocessor/macro_with_initializer_list.cpp | 182 ++++++++++++++++++ 5 files changed, 385 insertions(+), 18 deletions(-) create mode 100644 clang/test/Preprocessor/macro_with_initializer_list.cpp diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index 0fcfdf7..f2c3303 100644 --- a/clang/include/clang/Basic/Diagnostic.h +++ b/clang/include/clang/Basic/Diagnostic.h @@ -990,6 +990,10 @@ public: bool hasMaxRanges() const { return NumRanges == DiagnosticsEngine::MaxRanges; } + + bool hasMaxFixItHints() const { + return NumFixits == DiagnosticsEngine::MaxFixItHints; + } }; inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB, diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index 3639024..d7b35c9 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -460,6 +460,11 @@ def err_unterm_macro_invoc : Error< "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/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp index d21515c..8dccae1 100644 --- a/clang/lib/Lex/PPMacroExpansion.cpp +++ b/clang/lib/Lex/PPMacroExpansion.cpp @@ -390,6 +390,136 @@ bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier, 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 &Tokens) { + SmallVector Brackets; + for (SmallVectorImpl::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 &OldTokens, + SmallVectorImpl &NewTokens, + unsigned &NumArgs, + SmallVectorImpl &ParenHints, + SmallVectorImpl &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::iterator ArgStartIterator = OldTokens.begin(); + + // First closing brace in a new macro argument. Used to generate + // SourceRanges for InitLists. + SmallVectorImpl::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::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 @@ MacroArgs *Preprocessor::ReadFunctionLikeMacroArgs(Token &MacroName, SmallVector 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 @@ MacroArgs *Preprocessor::ReadFunctionLikeMacroArgs(Token &MacroName, // 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 @@ MacroArgs *Preprocessor::ReadFunctionLikeMacroArgs(Token &MacroName, 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 FixedArgTokens; + unsigned FixedNumArgs = 0; + SmallVector 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::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::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/clang/test/Preprocessor/macro_fn.c b/clang/test/Preprocessor/macro_fn.c index fcdb90a..f21ef51 100644 --- a/clang/test/Preprocessor/macro_fn.c +++ b/clang/test/Preprocessor/macro_fn.c @@ -13,7 +13,8 @@ zero(1, 2, 3); /* expected-error {{too many arguments provided to function-li 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 @@ two( , /* 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/clang/test/Preprocessor/macro_with_initializer_list.cpp b/clang/test/Preprocessor/macro_with_initializer_list.cpp new file mode 100644 index 0000000..4f5bf6d --- /dev/null +++ b/clang/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 initializer_list { + public: + initializer_list(); + }; +} + +class Foo { +public: + Foo(); + Foo(std::initializer_list); + 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}} +} -- 2.7.4