From: Richard Smith Date: Wed, 19 Oct 2011 21:33:05 +0000 (+0000) Subject: Improve the diagnostic when a comma ends up at the end of a declarator group X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0706df40064d4d7559b4304af79d519033414b84;p=clang Improve the diagnostic when a comma ends up at the end of a declarator group instead of a semicolon (as sometimes happens during refactorings). When such a comma is seen at the end of a line, and is followed by something which can't possibly be a declarator (or even something which might be a plausible typo for a declarator), suggest that a semicolon was intended. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@142544 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index 0046f88d26..e28bcf2001 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -1546,6 +1546,7 @@ private: ParsedAttributes &attrs, bool RequireSemi, ForRangeInit *FRI = 0); + bool MightBeDeclarator(unsigned Context); DeclGroupPtrTy ParseDeclGroup(ParsingDeclSpec &DS, unsigned Context, bool AllowFunctionDefinitions, SourceLocation *DeclEnd = 0, diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 57766f926f..8ce3bc1746 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -965,6 +965,61 @@ Parser::DeclGroupPtrTy Parser::ParseSimpleDeclaration(StmtVector &Stmts, return ParseDeclGroup(DS, Context, /*FunctionDefs=*/ false, &DeclEnd, FRI); } +/// Returns true if this might be the start of a declarator, or a common typo +/// for a declarator. +bool Parser::MightBeDeclarator(unsigned Context) { + switch (Tok.getKind()) { + case tok::annot_cxxscope: + case tok::annot_template_id: + case tok::caret: + case tok::code_completion: + case tok::coloncolon: + case tok::ellipsis: + case tok::kw___attribute: + case tok::kw_operator: + case tok::l_paren: + case tok::star: + return true; + + case tok::amp: + case tok::ampamp: + case tok::colon: // Might be a typo for '::'. + return getLang().CPlusPlus; + + case tok::identifier: + switch (NextToken().getKind()) { + case tok::code_completion: + case tok::coloncolon: + case tok::comma: + case tok::equal: + case tok::equalequal: // Might be a typo for '='. + case tok::kw_alignas: + case tok::kw_asm: + case tok::kw___attribute: + case tok::l_brace: + case tok::l_paren: + case tok::l_square: + case tok::less: + case tok::r_brace: + case tok::r_paren: + case tok::r_square: + case tok::semi: + return true; + + case tok::colon: + // At namespace scope, 'identifier:' is probably a typo for 'identifier::' + // and in block scope it's probably a label. + return getLang().CPlusPlus && Context == Declarator::FileContext; + + default: + return false; + } + + default: + return false; + } +} + /// ParseDeclGroup - Having concluded that this is either a function /// definition or a group of object declarations, actually parse the /// result. @@ -1042,11 +1097,22 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS, if (FirstDecl) DeclsInGroup.push_back(FirstDecl); + bool ExpectSemi = Context != Declarator::ForContext; + // If we don't have a comma, it is either the end of the list (a ';') or an // error, bail out. while (Tok.is(tok::comma)) { - // Consume the comma. - ConsumeToken(); + SourceLocation CommaLoc = ConsumeToken(); + + if (Tok.isAtStartOfLine() && ExpectSemi && !MightBeDeclarator(Context)) { + // This comma was followed by a line-break and something which can't be + // the start of a declarator. The comma was probably a typo for a + // semicolon. + Diag(CommaLoc, diag::err_expected_semi_declaration) + << FixItHint::CreateReplacement(CommaLoc, ";"); + ExpectSemi = false; + break; + } // Parse the next declarator. D.clear(); @@ -1071,7 +1137,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS, if (DeclEnd) *DeclEnd = Tok.getLocation(); - if (Context != Declarator::ForContext && + if (ExpectSemi && ExpectAndConsume(tok::semi, Context == Declarator::FileContext ? diag::err_invalid_token_after_toplevel_declarator @@ -3571,6 +3637,9 @@ void Parser::ParseDeclarator(Declarator &D) { /// isn't parsed at all, making this function effectively parse the C++ /// ptr-operator production. /// +/// If the grammar of this construct is extended, matching changes must also be +/// made to TryParseDeclarator and MightBeDeclarator. +/// /// declarator: [C99 6.7.5] [C++ 8p4, dcl.decl] /// [C] pointer[opt] direct-declarator /// [C++] direct-declarator diff --git a/test/FixIt/fixit-cxx0x.cpp b/test/FixIt/fixit-cxx0x.cpp index 73316457b1..9fb647d03f 100644 --- a/test/FixIt/fixit-cxx0x.cpp +++ b/test/FixIt/fixit-cxx0x.cpp @@ -52,3 +52,9 @@ namespace Constexpr { // -> constexpr static char *const p = 0; }; } + +namespace SemiCommaTypo { + int m {}, + n [[]], // expected-error {{expected ';' at end of declaration}} + int o; +} diff --git a/test/FixIt/fixit.c b/test/FixIt/fixit.c index 5ba0aac450..967ae23c18 100644 --- a/test/FixIt/fixit.c +++ b/test/FixIt/fixit.c @@ -70,3 +70,10 @@ void removeUnusedLabels(char c) { : c++; c = c + 3; L4: return; } + +int oopsAComma = 0, +void oopsMoreCommas() { + static int a[] = { 0, 1, 2 }, + static int b[] = { 3, 4, 5 }, + &a == &b ? oopsMoreCommas() : removeUnusedLabels(a[0]); +} diff --git a/test/FixIt/fixit.cpp b/test/FixIt/fixit.cpp index 96ce2ce097..dff8a37820 100644 --- a/test/FixIt/fixit.cpp +++ b/test/FixIt/fixit.cpp @@ -111,3 +111,16 @@ void foo1() const {} // expected-error {{type qualifier is not allowed on this f void foo2() volatile {} // expected-error {{type qualifier is not allowed on this function}} void foo3() const volatile {} // expected-error {{type qualifier is not allowed on this function}} +struct S { void f(int, char); }; +int itsAComma, +itsAComma2 = 0, +oopsAComma(42), // expected-error {{expected ';' after declaration}} +AD oopsMoreCommas() { + static int n = 0, + static char c, + &d = c, // expected-error {{expected ';' after declaration}} + S s, // expected-error {{expected ';' after declaration}} + s.f(n, d); + AD ad, // expected-error {{expected ';' after declaration}} + return ad; +} diff --git a/test/Parser/cxx-decl.cpp b/test/Parser/cxx-decl.cpp index 70eff973e0..57f33d826f 100644 --- a/test/Parser/cxx-decl.cpp +++ b/test/Parser/cxx-decl.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -verify -fsyntax-only %s +// RUN: %clang_cc1 -verify -fsyntax-only -triple i386-linux %s int x(*g); // expected-error {{use of undeclared identifier 'g'}} @@ -66,6 +66,36 @@ struct test4 { int z // expected-error {{expected ';' at end of declaration list}} }; +// Make sure we know these are legitimate commas and not typos for ';'. +namespace Commas { + struct S { + static int a; + int c, + operator()(); + }; + + int global1, + __attribute__(()) global2, + (global5), + *global6, + &global7 = global1, + &&global8 = static_cast(global1), // expected-warning 2{{rvalue reference}} + S::a, + global9, + global10 = 0, + global11 == 0, // expected-error {{did you mean '='}} + global12 __attribute__(()), + global13(0), + global14[2], + global15; + + void g() { + static int a, + b __asm__("ebx"), // expected-error {{expected ';' at end of declaration}} + Statics:return; + } +} + // PR5825 struct test5 {}; ::new(static_cast(0)) test5; // expected-error {{expected unqualified-id}} diff --git a/test/Parser/cxx0x-decl.cpp b/test/Parser/cxx0x-decl.cpp new file mode 100644 index 0000000000..73aa3fdc40 --- /dev/null +++ b/test/Parser/cxx0x-decl.cpp @@ -0,0 +1,8 @@ +// RUN: %clang_cc1 -verify -fsyntax-only -std=c++0x %s + +// Make sure we know these are legitimate commas and not typos for ';'. +namespace Commas { + int a, + b [[ ]], + c alignas(double); +}