Index: include/clang/Basic/DiagnosticParseKinds.td =================================================================== --- include/clang/Basic/DiagnosticParseKinds.td +++ include/clang/Basic/DiagnosticParseKinds.td @@ -441,6 +441,8 @@ "cannot use %select{dot|arrow}0 operator on a type">; def err_expected_unqualified_id : Error< "expected %select{identifier|unqualified-id}0">; +def err_brackets_go_after_unqualified_id : Error< + "brackets go after the %select{identifier|unqualified-id}0">; def err_unexpected_unqualified_id : Error<"type-id cannot have a name">; def err_func_def_no_params : Error< "function definition does not declare parameters">; Index: include/clang/Parse/Parser.h =================================================================== --- include/clang/Parse/Parser.h +++ include/clang/Parse/Parser.h @@ -2207,7 +2207,11 @@ ParsedAttributes &attrs, SmallVectorImpl &ParamInfo, SourceLocation &EllipsisLoc); - void ParseBracketDeclarator(Declarator &D); + + typedef SmallVector, 4> + SavedBracketInfo; + void ParseBracketDeclarator(Declarator &D, + SavedBracketInfo *SavedInfo = 0); //===--------------------------------------------------------------------===// // C++ 7: Declarations [dcl.dcl] Index: lib/Parse/ParseDecl.cpp =================================================================== --- lib/Parse/ParseDecl.cpp +++ lib/Parse/ParseDecl.cpp @@ -4700,6 +4700,22 @@ void Parser::ParseDirectDeclarator(Declarator &D) { DeclaratorScopeObj DeclScopeObj(*this, D.getCXXScopeSpec()); + // Store information for recovery from misplaced brackets. + std::unique_ptr SavedInfo; + bool UnhandledError = false; + SourceRange BracketRange; + + // Found square brackets instead of an identifier. Parse them first and + // save some information for a diagnostic later if the identifer is found. + if (Tok.is(tok::l_square) && !D.mayOmitIdentifier()) { + SavedInfo.reset(new SavedBracketInfo); + UnhandledError = true; + BracketRange.setBegin(Tok.getLocation()); + while (Tok.is(tok::l_square)) { + ParseBracketDeclarator(D, SavedInfo.get()); + } + } + if (getLangOpts().CPlusPlus && D.mayHaveIdentifier()) { // ParseDeclaratorInternal might already have parsed the scope. if (D.getCXXScopeSpec().isEmpty()) { @@ -4832,29 +4848,69 @@ } else { if (Tok.getKind() == tok::annot_pragma_parser_crash) LLVM_BUILTIN_TRAP; - if (D.getContext() == Declarator::MemberContext) - Diag(Tok, diag::err_expected_member_name_or_semi) + if (D.getContext() == Declarator::MemberContext) { + SourceLocation DiagLoc = Tok.getLocation(); + if (UnhandledError) { + // Place the error before square brackets. + assert(BracketRange.getBegin().isValid()); + DiagLoc = BracketRange.getBegin(); + UnhandledError = false; + } + Diag(DiagLoc, diag::err_expected_member_name_or_semi) << (D.getDeclSpec().isEmpty() ? SourceRange() : D.getDeclSpec().getSourceRange()); - else if (getLangOpts().CPlusPlus) { - if (Tok.is(tok::period) || Tok.is(tok::arrow)) + } else if (getLangOpts().CPlusPlus) { + if (!UnhandledError && (Tok.is(tok::period) || Tok.is(tok::arrow))) Diag(Tok, diag::err_invalid_operator_on_type) << Tok.is(tok::arrow); else { - SourceLocation Loc = D.getCXXScopeSpec().getEndLoc(); - if (Tok.isAtStartOfLine() && Loc.isValid()) - Diag(PP.getLocForEndOfToken(Loc), diag::err_expected_unqualified_id) - << getLangOpts().CPlusPlus; - else - Diag(Tok, diag::err_expected_unqualified_id) - << getLangOpts().CPlusPlus; + SourceLocation DiagLoc = Tok.getLocation(); + if (UnhandledError) { + UnhandledError = false; + assert(BracketRange.getBegin().isValid()); + DiagLoc = BracketRange.getBegin(); + } else { + SourceLocation EndLoc = D.getCXXScopeSpec().getEndLoc(); + if (Tok.isAtStartOfLine() && EndLoc.isValid()) { + // Point to the end of the current line instead of the beginning of + // a new line to give better context for the error. + DiagLoc = PP.getLocForEndOfToken(EndLoc); + } + } + Diag(DiagLoc, diag::err_expected_unqualified_id) + << getLangOpts().CPlusPlus; } - } else - Diag(Tok, diag::err_expected_either) << tok::identifier << tok::l_paren; + } else { + SourceLocation DiagLoc = Tok.getLocation(); + if (UnhandledError) { + UnhandledError = false; + DiagLoc = BracketRange.getBegin(); + assert(BracketRange.getBegin().isValid()); + } + Diag(DiagLoc, diag::err_expected_either) << tok::identifier + << tok::l_paren; + } D.SetIdentifier(0, Tok.getLocation()); D.setInvalidType(true); } PastIdentifier: + if (UnhandledError) { + // Found brackets before the identifier. Suggest to move the brackets + // after the identifier. Also, add the bracket info to the Declarator. + UnhandledError = false; + for (const auto &TypeInfo : *SavedInfo) { + ParsedAttributes attrs(AttrFactory); + attrs.set(TypeInfo.second); + D.AddTypeInfo(TypeInfo.first, attrs, SourceLocation()); + } + BracketRange.setEnd(SavedInfo->back().first.EndLoc); + SourceLocation Loc = PP.getLocForEndOfToken(D.getLocEnd()); + Diag(Loc, diag::err_brackets_go_after_unqualified_id) + << getLangOpts().CPlusPlus + << FixItHint::CreateInsertionFromRange( + Loc, CharSourceRange(BracketRange, true)) + << FixItHint::CreateRemoval(BracketRange); + } assert(D.isPastIdentifier() && "Haven't past the location of the identifier yet?"); @@ -5447,7 +5503,10 @@ /// [C99] direct-declarator '[' type-qual-list[opt] '*' ']' /// [C++11] direct-declarator '[' constant-expression[opt] ']' /// attribute-specifier-seq[opt] -void Parser::ParseBracketDeclarator(Declarator &D) { +/// When SavedInfo is non-null, store the parsed bracket information in it +/// instead of passing it to the Declarator. +void Parser::ParseBracketDeclarator(Declarator &D, + SavedBracketInfo *SavedInfo) { if (CheckProhibitedCXX11Attribute()) return; @@ -5462,10 +5521,17 @@ MaybeParseCXX11Attributes(attrs); // Remember that we parsed the empty array type. - D.AddTypeInfo(DeclaratorChunk::getArray(0, false, false, 0, - T.getOpenLocation(), - T.getCloseLocation()), - attrs, T.getCloseLocation()); + if (SavedInfo) { + SavedInfo->push_back( + {DeclaratorChunk::getArray(0, false, false, 0, T.getOpenLocation(), + T.getCloseLocation()), + attrs.getList()}); + } else { + D.AddTypeInfo(DeclaratorChunk::getArray(0, false, false, 0, + T.getOpenLocation(), + T.getCloseLocation()), + attrs, T.getCloseLocation()); + } return; } else if (Tok.getKind() == tok::numeric_constant && GetLookAheadToken(1).is(tok::r_square)) { @@ -5478,11 +5544,18 @@ MaybeParseCXX11Attributes(attrs); // Remember that we parsed a array type, and remember its features. - D.AddTypeInfo(DeclaratorChunk::getArray(0, false, false, - ExprRes.release(), - T.getOpenLocation(), - T.getCloseLocation()), - attrs, T.getCloseLocation()); + if (SavedInfo) { + SavedInfo->push_back( + {DeclaratorChunk::getArray(0, false, false, ExprRes.release(), + T.getOpenLocation(), T.getCloseLocation()), + attrs.getList()}); + } else { + D.AddTypeInfo(DeclaratorChunk::getArray(0, false, false, + ExprRes.release(), + T.getOpenLocation(), + T.getCloseLocation()), + attrs, T.getCloseLocation()); + } return; } @@ -5547,12 +5620,20 @@ MaybeParseCXX11Attributes(attrs); // Remember that we parsed a array type, and remember its features. - D.AddTypeInfo(DeclaratorChunk::getArray(DS.getTypeQualifiers(), - StaticLoc.isValid(), isStar, - NumElements.release(), - T.getOpenLocation(), - T.getCloseLocation()), - attrs, T.getCloseLocation()); + if (SavedInfo) { + SavedInfo->push_back( + {DeclaratorChunk::getArray(DS.getTypeQualifiers(), StaticLoc.isValid(), + isStar, NumElements.release(), + T.getOpenLocation(), T.getCloseLocation()), + attrs.getList()}); + } else { + D.AddTypeInfo(DeclaratorChunk::getArray(DS.getTypeQualifiers(), + StaticLoc.isValid(), isStar, + NumElements.release(), + T.getOpenLocation(), + T.getCloseLocation()), + attrs, T.getCloseLocation()); + } } /// [GNU] typeof-specifier: Index: test/Parser/brackets.c =================================================================== --- test/Parser/brackets.c +++ test/Parser/brackets.c @@ -0,0 +1,51 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: cp %s %t +// RUN: not %clang_cc1 -fixit %t -x c -DFIXIT +// RUN: %clang_cc1 -fsyntax-only %t -x c -DFIXIT +// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -strict-whitespace + +void test1() { + int a[] = {0,1,1,2,3}; + int []b = {0,1,4,9,16}; + // expected-error@-1{{brackets go after the identifier}} + // CHECK: {{^}} int []b = {0,1,4,9,16}; + // CHECK: {{^}} ~~ ^ + // CHECK: {{^}} [] + // CHECK: fix-it:"{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:9}:"" + // CHECK: fix-it:"{{.*}}:{[[@LINE-6]]:10-[[@LINE-6]]:10}:"[]" + + int c = a[0]; + int d = b[0]; // No undeclared identifer error here. + + int *e = a; + int *f = b; // No undeclared identifer error here. +} + +struct S { + int [1][1]x; + // expected-error@-1{{brackets go after the identifier}} + // CHECK: {{^}} int [1][1]x; + // CHECK: {{^}} ~~~~~~ ^ + // CHECK: {{^}} [1][1] + // CHECK: fix-it:"{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:13}:"" + // CHECK: fix-it:"{{.*}}:{[[@LINE-6]]:14-[[@LINE-6]]:14}:"[1][1]" +} s; + +#ifndef FIXIT +void test2() { + int [][][]; + // expected-error@-1{{expected identifier or '('}} + // CHECK: {{^}} int [][][]; + // CHECK: {{^}} ^ + // CHECK-NOT: fix-it + struct T { + int []; + // expected-error@-1{{expected member name or ';' after declaration specifiers}} + // CHECK: {{^}} int []; + // CHECK: {{^}} ~~~ ^ + // CHECK-NOT: fix-it + }; +} +#endif + +// CHECK: 4 errors generated. Index: test/Parser/brackets.cpp =================================================================== --- test/Parser/brackets.cpp +++ test/Parser/brackets.cpp @@ -0,0 +1,67 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: cp %s %t +// RUN: not %clang_cc1 -fixit %t -x c++ -DFIXIT +// RUN: %clang_cc1 -fsyntax-only %t -x c++ -DFIXIT +// RUN: not %clang_cc1 -fsyntax-only %s 2>&1 | FileCheck %s -strict-whitespace + +int a[] = {0,1,1,2,3}; +int []b = {0,1,4,9,16}; +// expected-error@-1{{brackets go after the unqualified-id}} +// CHECK: {{^}}int []b = {0,1,4,9,16}; +// CHECK: {{^}} ~~ ^ +// CHECK: {{^}} [] + +int c = a[0]; +int d = b[0]; // No undeclared identifer error here. + +int *e = a; +int *f = b; // No undeclared identifer error here. + +struct S { + static int [1][1]x; + // expected-error@-1{{brackets go after the unqualified-id}} + // CHECK: {{^}} static int [1][1]x; + // CHECK: {{^}} ~~~~~~ ^ + // CHECK: {{^}} [1][1] +}; +int [1][1]S::x = { {42} }; +// expected-error@-1{{brackets go after the unqualified-id}} +// CHECK: {{^}}int [1][1]S::x = { {42} }; +// CHECK: {{^}} ~~~~~~ ^ +// CHECK: {{^}} [1][1] + +int[1] g[2]; +// expected-error@-1{{brackets go after the unqualified-id}} +// CHECK: {{^}}int[1] g[2]; +// CHECK: {{^}} ~~~ ^ +// CHECK: {{^}} [1] + +int [3] (*x) = 0; +// expected-error@-1{{brackets go after the unqualified-id}} +// CHECK: {{^}}int [3] (*x) = 0; +// CHECK: {{^}} ~~~~ ^ +// CHECK: {{^}} [3] + +#ifndef FIXIT +// Make sure x is corrected to be like type y, instead of like type z. +int (*y)[3] = x; +int (*z[3]) = x; // expected-error{{}} + +int [][][]; +// expected-error@-1{{expected unqualified-id}} +// CHECK: {{^}}int [][][]; +// CHECK: {{^}} ^ +struct T { + int []; + // expected-error@-1{{expected member name or ';' after declaration specifiers}} + // CHECK: {{^}} int []; + // CHECK: {{^}} ~~~ ^ +}; + + int [] T:: + // expected-error@-1{{expected unqualified-id}} + // CHECK: {{^}} int [] T:: + // CHECK: {{^}} ^ +#endif + +// CHECK: 9 errors generated.