Improve diagnostic message for misplaced square brackets

[rtrieu]

Summary

It is a common mistake to place square brackets before the identifier. However, when this occurs, Clang merely says "expected unqualified-id" pointing at the first open square bracket. Also, since the identifier isn't properly parsed, attempting to use it will result in additional undeclared identifier errors.

test.cc:

const char[] str = "foo";
char a = str[1];

Clang:

test.cc:1:11: error: expected unqualified-id
const char[] str = "foo";
          ^
test.cc:2:10: error: use of undeclared identifier 'str'
char a = str[1];
         ^
2 errors generated.

With patch:

test.cc:1:17: error: brackets go after the unqualified-id
const char[] str = "foo";
          ~~    ^
                []
1 error generated.

Currently, the parser will process the identifier then the brackets. Now, when a bracket appears instead of an identifier, the parser can process the brackets first, holding the location information. Then when it finds the identifier, it can will produce a better diagnostic to move the brackets and also include a fix-it hint. Other error messages have been updated to display in the correct spot. Since the identifier can now be attached to the Declarator, this will also remove the undeclared identifier errors, too.

[gribozavr]

test/Parser/brackets.c

6

-fdiagnostics-parseable-fixits?

[rtrieu]

Add parseable fix-its to the test case.

[rsmith]

lib/Parse/ParseDecl.cpp

4717

In this case, I think you probably shouldn't allow square brackets. That is, if someone writes:

int foo::[3]

we shouldn't activate the special case.

4722–4729

It looks like this will behave strangely if the remaining declarator is not just a simple identifier:

int[3] (*p);

... will form an 'array of three pointers to int' type, rather than a 'pointer to array of three ints' type. Instead, to get this sort of thing right, I suggest you consume the array bound, then call ParseDeclarator, then add the array bound chunks to the declarator and issue your diagnostic.

To get the diagnostic location right, you could store the location of the '[' as the identifier location in the Declarator object or similar.

[thakis]

Oh, nice feature! I didn't look at the patch, but I tried to write this
patch a while ago in http://llvm.org/bugs/show_bug.cgi?id=11739 (which this
patch will fix) and there are a few comments on that bug that might be
interesting (including review comments from zygoloid, test cases, etc). So
if you haven't see that bug, maybe give it a look :-)

[rtrieu]

Cache the results of the bracket parsing, then later add the info the Declarator to get the correct type.

[rtrieu]

lib/Parse/ParseDecl.cpp

4717

This patch doesn't affect the diagnostic messages for this code.

4722–4729

Switch to storing the bracket data, then letting the identifier parsing happen. Right before the diagnostic is emitted, the bracket data is added to the Declarator, which now treats:

int[3] (*p);

as

int (*p)[3];

[rsmith]

lib/Parse/ParseDecl.cpp

4838

This looks like it'll provide the wrong location if there were misplaced brackets.

4856

Have you considered putting the bracket parsing code way down here (and recursively calling back into this function after parsing them)?

[rtrieu]

lib/Parse/ParseDecl.cpp

4838

The recovery for brackets requires !D.mayOmitIdentifier() [line 4710] while this branch requires D.mayOmitIdentifier(). Either bracket recovery happens or this code executes, but not both.

4856

Currently, the error correction of:

int [1] a [2];

to

int a[1][2];

If this was done recursively, the bracket recovery would go to the end instead to:

int a[2][1];

[rtrieu]

Switching to a recursive solution for misplaced brackets. Upon seeing a bracket before the identifier, parse the brackets and store the info. Call ParseDirectDeclarator again to continue parsing. After that returns, attach the bracket info the Declarator and emit the misplaced bracket error with fix-it hint to move the brackets after the identifier. Additionally, store the location of the beginning of the brackets so that other error messages may point to a better location.

[rsmith]

+static SourceLocation getDiagLoc(Declarator &D, SourceLocation Loc) {

This is a very generic name for quite a specific behavior.
'getMissingDeclaratorIdLoc' or similar?

+ if (Tok.is(tok::l_square)) {

Maybe factor this out into a separate function.
'ParseMisplacedBracketDeclarator' or similar?

+ SavedBracketInfo SavedInfo;
+ SourceLocation Loc = Tok.getLocation();
+
+ while (Tok.is(tok::l_square)) {
+ ParseBracketDeclarator(D, &SavedInfo);

Is it possible to handle this by parsing into a new Declarator here (and
copying its chunks into D once we're done), instead of introducing a
separate SavedBracketInfo code path into ParseBracketDeclarator?

+ Something went wrong parsing the parens, in which case,
[...]
+ Adding back the paren info to the end of the Declarator.

Please say "brackets" rather than "parens"; we're pretty consistent about
using "parens" to mean () and not [].

+ SavedInfo->push_back(
+ {DeclaratorChunk::getArray(0, false, false, 0,
T.getOpenLocation(),

MSVC doesn't support this syntax.

[rtrieu]

Moved the misplaced bracket logic from ParseDirectDeclarator into its own method.
Use a temporary Declarator to store the bracket location instead of using a custom storage object.
Renamed getDiagLoc to getMissingDeclaratorIdLoc.
Fixed up paren/bracket mistakes.

[rsmith]

Looks great, ship it!

Wait... one more thing:

int [3] *x;

We should suggest adding parentheses here, because

int *x[3];

means the wrong thing.

[rtrieu]

Add code to handle:

int [] *a;

This is handled by inserting parens in the token stream, then backtracking and have ParseDirectDeclarator handle the parens as normal through ParseParenDeclarator.

[rtrieu]

Clean up brackets.cpp test case
Add new tests for reference variables
Use ParseDeclaratorInternal instead of ParseDirectDeclarator
Remove use of token stream mangling and backtracking
Skip the error message when the closing paren comes before the opening paren in the fix-it hint

The main difference is restarting the parse earlier with ParseDeclaratorInternal, which allows for parsing the leading '*' and '&' tokens in "int [1] *foo" and "int [2] &bar"

[rsmith]

+ if (Tok.is(tok::l_square)) {
+ if (ParseMisplacedBracketDeclarator(D)) {
+ ParseMisplacedBracketDeclarator(D);
+ goto PastIdentifier;
+ return;

Remove comments here; maybe just

if (Tok.is(tok::l_square))

return ParseMisplacedBracketDeclarator(D);

+ Sometimes, parentheses are needed. Determine if they are needed by
+ looking at the current token. If they are needed, store the location
+ of the left parentheses in SuggestParenLoc.
+ SourceLocation SuggestParenLoc;
+ if (Tok.isNot(tok::identifier) && Tok.isNot(tok::l_paren) &&
+ Tok.isNot(tok::semi)) {
+ SuggestParenLoc = Tok.getLocation();
+ D.getName().EndLocation = SourceLocation();
+ }
+
+ Now that the brackets are removed, try parsing the declarator again.
+ ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator);

This logic for determining whether parens are needed doesn't seem to cover
all cases. In particular:

int [3] T::*p;

needs parens but starts with an identifier. Conversely,

int [3] ::n;

doesn't need parens, but gets them. Instead, how about something like:

SourceLocation LocBeforeDeclarator = Tok.getLocation();
ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator);

bool NeedParens = false;
if (D.getNumTypeObjects()) {

switch (D.getTypeObject(D.getNumTypeObjects())) {
  case DeclaratorChunk::Pointer:
  case DeclaratorChunk::Reference:
  case DeclaratorChunk::BlockPointer:
  case DeclaratorChunk::MemberPointer:
    NeedParens = true;
    break;
  case /* all the rest */
    break;
}

}

// ...

+ // Adding back the bracket info to the end of the Declarator.

If you've detected that you need parens, it'd be nice to fabricate a
DeclaratorChunk for the parens, just in case something downstream is making
assumptions about what DeclaratorChunks can exist based on how types can be
written.

+ The missing identifier would have been diagnosed in
ParseDirectDeclarator.
+ If parentheses are required, always suggest them.
+ if (!D.getIdentifier() && !SuggestParenLoc.isValid())
+ return;

It would be nice to assert !D.mayOmitIdentifier() somewhere in this
function, maybe right next to this check, since you're relying on that
here. (I could certainly imagine this code getting reused for parsing
abstract declarators at some point.)

+ SourceLocation EndBracketLoc =
+ TempDeclarator.getTypeObject(TempDeclarator.getNumTypeObjects() - 1)
+ .EndLoc;

Is this just 'TempDeclarator.getLocEnd()'?

+ When suggesting parentheses, the closing paren should not be before
the
+ opening paren.
+ if (SuggestParenLoc.isValid() && EndLoc < SuggestParenLoc)

This kind of SourceLocation comparison won't work in general (for instance,
it will do weird things if the two locations have different FileIDs). Does
this error case really happen?

Also, you still need to produce some kind of diagnostic on this code path.

[rtrieu]

In D2712#21, @rsmith wrote:

+ if (Tok.is(tok::l_square)) {
+ if (ParseMisplacedBracketDeclarator(D)) {
+ ParseMisplacedBracketDeclarator(D);
+ goto PastIdentifier;
+ return;

Remove comments here; maybe just

Removed comments.

if (Tok.is(tok::l_square))
  return ParseMisplacedBracketDeclarator(D);

+ Sometimes, parentheses are needed. Determine if they are needed by
+ looking at the current token. If they are needed, store the location
+ of the left parentheses in SuggestParenLoc.
+ SourceLocation SuggestParenLoc;
+ if (Tok.isNot(tok::identifier) && Tok.isNot(tok::l_paren) &&
+ Tok.isNot(tok::semi)) {
+ SuggestParenLoc = Tok.getLocation();
+ D.getName().EndLocation = SourceLocation();
+ }
+
+ Now that the brackets are removed, try parsing the declarator again.
+ ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator);

This logic for determining whether parens are needed doesn't seem to cover
all cases. In particular:

int [3] T::*p;

needs parens but starts with an identifier. Conversely,

int [3] ::n;

doesn't need parens, but gets them. Instead, how about something like:

SourceLocation LocBeforeDeclarator = Tok.getLocation();
ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator);

bool NeedParens = false;
if (D.getNumTypeObjects()) {
  switch (D.getTypeObject(D.getNumTypeObjects())) {
    case DeclaratorChunk::Pointer:
    case DeclaratorChunk::Reference:
    case DeclaratorChunk::BlockPointer:
    case DeclaratorChunk::MemberPointer:
      NeedParens = true;
      break;
    case /* all the rest */
      break;
  }
}

Paren adding updated.

+ // Adding back the bracket info to the end of the Declarator.

If you've detected that you need parens, it'd be nice to fabricate a
DeclaratorChunk for the parens, just in case something downstream is making
assumptions about what DeclaratorChunks can exist based on how types can be
written.

Done. New paren DeclaratorChuck added right after figuring out they are needed.

+ The missing identifier would have been diagnosed in
ParseDirectDeclarator.
+ If parentheses are required, always suggest them.
+ if (!D.getIdentifier() && !SuggestParenLoc.isValid())
+ return;

It would be nice to assert !D.mayOmitIdentifier() somewhere in this
function, maybe right next to this check, since you're relying on that
here. (I could certainly imagine this code getting reused for parsing
abstract declarators at some point.)

!D.mayOmitIdentifier() asserted at the beginning of the function since this function assumed the existence of an identifier.

+ SourceLocation EndBracketLoc =
+ TempDeclarator.getTypeObject(TempDeclarator.getNumTypeObjects() - 1)
+ .EndLoc;

Is this just 'TempDeclarator.getLocEnd()'?

Yes. Changed.

+ When suggesting parentheses, the closing paren should not be before the
+ opening paren.
+ if (SuggestParenLoc.isValid() && EndLoc < SuggestParenLoc)

This kind of SourceLocation comparison won't work in general (for instance,
it will do weird things if the two locations have different FileIDs). Does
this error case really happen?

Changed it to detect that the Declarator end location changes after the call to ParseDeclaratorInternal. Something like "int [];" will trigger this case.

Also, you still need to produce some kind of diagnostic on this code path.

The ParseDeclaratorInternal should provide the error here.

[rsmith]

Looks great, with one tweak.

lib/Parse/ParseDecl.cpp

5646–5649

This looks unreachable. If nothing after the brackets has been parsed, then there can't be an identifier, so we should have bailed out already. Remove?

[rtrieu]

Committed at r211641.

lib/Parse/ParseDecl.cpp

5646–5649

Removed.

include/clang/Basic/DiagnosticParseKinds.td

def err_declaration_does_not_declare_param : Error<
   "declaration does not declare a parameter">;
 def err_no_matching_param : Error<"parameter named %0 is missing">;
 
 /// C++ parser diagnostics
 def err_invalid_operator_on_type : Error<
   "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">;
 def err_expected_lparen_after_type : Error<
   "expected '(' for function-style cast or type construction">;
 def err_expected_init_in_condition : Error<
   "variable declaration in condition must have an initializer">;
 def err_expected_init_in_condition_lparen : Error<

include/clang/Parse/Parser.h

void ParseFunctionDeclaratorIdentifierList(
          Declarator &D,
          SmallVectorImpl<DeclaratorChunk::ParamInfo> &ParamInfo);
   void ParseParameterDeclarationClause(
          Declarator &D,
          ParsedAttributes &attrs,
          SmallVectorImpl<DeclaratorChunk::ParamInfo> &ParamInfo,
          SourceLocation &EllipsisLoc);
   void ParseBracketDeclarator(Declarator &D);
+  void ParseMisplacedBracketDeclarator(Declarator &D);
 
   //===--------------------------------------------------------------------===//
   // C++ 7: Declarations [dcl.dcl]
 
   /// The kind of attribute specifier we have found.
   enum CXX11AttributeKind {
     /// This is not an attribute specifier.
     CAK_NotAttributeSpecifier,

lib/Parse/ParseDecl.cpp

if (!D.getEllipsisLoc().isValid()) {
       Insertion = FixItHint::CreateInsertion(D.getIdentifierLoc(), "...");
       D.setEllipsisLoc(EllipsisLoc);
     }
     P.Diag(EllipsisLoc, diag::err_misplaced_ellipsis_in_declaration)
       << FixItHint::CreateRemoval(EllipsisLoc) << Insertion << !D.hasName();
   }
 }
 
+// When correcting from misplaced brackets before the identifier, the location
+// is saved inside the declarator so that other diagnostic messages can use
+// them.  This extracts and returns that location, or returns the provided
+// location if a stored location does not exist.
+static SourceLocation getMissingDeclaratorIdLoc(Declarator &D,
+                                                SourceLocation Loc) {
+  if (D.getName().StartLocation.isInvalid() &&
+      D.getName().EndLocation.isValid())
+    return D.getName().EndLocation;
+
+  return Loc;
+}
+
 /// ParseDirectDeclarator
 ///       direct-declarator: [C99 6.7.5]
 /// [C99]   identifier
 ///         '(' declarator ')'
 /// [GNU]   '(' attributes declarator ')'
 /// [C90]   direct-declarator '[' constant-expression[opt] ']'
 /// [C99]   direct-declarator '[' type-qual-list[opt] assignment-expr[opt] ']'
 /// [C99]   direct-declarator '[' 'static' type-qual-list[opt] assign-expr ']'

 ///         template-id
 ///
 /// Note, any additional constructs added here may need corresponding changes
 /// in isConstructorDeclarator.
 void Parser::ParseDirectDeclarator(Declarator &D) {
   DeclaratorScopeObj DeclScopeObj(*this, D.getCXXScopeSpec());
 
   if (getLangOpts().CPlusPlus && D.mayHaveIdentifier()) {
     // ParseDeclaratorInternal might already have parsed the scope.
     if (D.getCXXScopeSpec().isEmpty()) {
       bool EnteringContext = D.getContext() == Declarator::FileContext ||
                              D.getContext() == Declarator::MemberContext;
       ParseOptionalCXXScopeSpecifier(D.getCXXScopeSpec(), ParsedType(),
                                      EnteringContext);
     }
 
     if (D.getCXXScopeSpec().isValid()) {
       if (Actions.ShouldEnterDeclaratorScope(getCurScope(), D.getCXXScopeSpec()))
         // Change the declaration context for name lookup, until this function
         // is exited (and the declarator has been parsed).
         DeclScopeObj.EnterDeclaratorScope();
     }
 
     // C++0x [dcl.fct]p14:
     //   There is a syntactic ambiguity when an ellipsis occurs at the end
     //   of a parameter-declaration-clause without a preceding comma. In
     //   this case, the ellipsis is parsed as part of the
     //   abstract-declarator if the type of the parameter names a template
     //   parameter pack that has not been expanded; otherwise, it is parsed
if (D.getCXXScopeSpec().isSet()) {
           Actions.ShouldEnterDeclaratorScope(getCurScope(), D.getCXXScopeSpec()))
         // Change the declaration context for name lookup, until this function
         // is exited (and the declarator has been parsed).
         DeclScopeObj.EnterDeclaratorScope();
     }
   } else if (D.mayOmitIdentifier()) {
     // This could be something simple like "int" (in which case the declarator
     // portion is empty), if an abstract-declarator is allowed.
     D.SetIdentifier(0, Tok.getLocation());
 
     // The grammar for abstract-pack-declarator does not allow grouping parens.
     // FIXME: Revisit this once core issue 1488 is resolved.
     if (D.hasEllipsis() && D.hasGroupingParens())
       Diag(PP.getLocForEndOfToken(D.getEllipsisLoc()),
            diag::ext_abstract_pack_declarator_parens);
   } 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 (Tok.is(tok::l_square))
+      return ParseMisplacedBracketDeclarator(D);
+    if (D.getContext() == Declarator::MemberContext) {
+      Diag(getMissingDeclaratorIdLoc(D, Tok.getLocation()),
+           diag::err_expected_member_name_or_semi)
           << (D.getDeclSpec().isEmpty() ? SourceRange()
                                         : D.getDeclSpec().getSourceRange());
-    else if (getLangOpts().CPlusPlus) {
+    } else if (getLangOpts().CPlusPlus) {
       if (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)
+          Diag(getMissingDeclaratorIdLoc(D, Tok.getLocation()),
+               diag::err_expected_unqualified_id)
               << getLangOpts().CPlusPlus;
       }
-    } else
-      Diag(Tok, diag::err_expected_either) << tok::identifier << tok::l_paren;
+    } else {
+      Diag(getMissingDeclaratorIdLoc(D, Tok.getLocation()),
+           diag::err_expected_either)
+          << tok::identifier << tok::l_paren;
+    }
     D.SetIdentifier(0, Tok.getLocation());
     D.setInvalidType(true);
   }
 
  PastIdentifier:
   assert(D.isPastIdentifier() &&
          "Haven't past the location of the identifier yet?");
 
void Parser::ParseBracketDeclarator(Declarator &D) {
   D.AddTypeInfo(DeclaratorChunk::getArray(DS.getTypeQualifiers(),
                                           StaticLoc.isValid(), isStar,
                                           NumElements.release(),
                                           T.getOpenLocation(),
                                           T.getCloseLocation()),
                 attrs, T.getCloseLocation());
 }
 
+/// Diagnose brackets before an identifier.
+void Parser::ParseMisplacedBracketDeclarator(Declarator &D) {
+  assert(Tok.is(tok::l_square) && "Missing opening bracket");
+  assert(!D.mayOmitIdentifier() && "Declarator cannot omit identifier");
+
+  SourceLocation StartBracketLoc = Tok.getLocation();
+  Declarator TempDeclarator(D.getDeclSpec(), D.getContext());
+
+  while (Tok.is(tok::l_square)) {
+    ParseBracketDeclarator(TempDeclarator);
+  }
+
+  // Stuff the location of the start of the brackets into the Declarator.
+  // The diagnostics from ParseDirectDeclarator will make more sense if
+  // they use this location instead.
+  if (Tok.is(tok::semi))
+    D.getName().EndLocation = StartBracketLoc;
+
+  SourceLocation SuggestParenLoc = Tok.getLocation();
+  SourceLocation OldEndLoc = D.getLocEnd();
+
+  // Now that the brackets are removed, try parsing the declarator again.
+  ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator);
+
+  // Something went wrong parsing the brackets, in which case,
+  // ParseBracketDeclarator has emitted an error, and we don't need to emit
+  // one here.
+  if (TempDeclarator.getNumTypeObjects() == 0)
+    return;
+
+  // Determine if parens will need to be suggested in the diagnostic.
+  bool NeedParens = false;
+  if (D.getNumTypeObjects() != 0) {
+    switch (D.getTypeObject(D.getNumTypeObjects() - 1).Kind) {
+    case DeclaratorChunk::Pointer:
+    case DeclaratorChunk::Reference:
+    case DeclaratorChunk::BlockPointer:
+    case DeclaratorChunk::MemberPointer:
+      NeedParens = true;
+      break;
+    case DeclaratorChunk::Array:
+    case DeclaratorChunk::Function:
+    case DeclaratorChunk::Paren:
+      break;
+    }
+  }
+
+  if (NeedParens) {
+    // Create a DeclaratorChunk for the inserted parens.
+    ParsedAttributes attrs(AttrFactory);
+    SourceLocation EndLoc = PP.getLocForEndOfToken(D.getLocEnd());
+    D.AddTypeInfo(DeclaratorChunk::getParen(SuggestParenLoc, EndLoc), attrs,
+                  SourceLocation());
+  }
+
+  // Adding back the bracket info to the end of the Declarator.
+  for (unsigned i = 0, e = TempDeclarator.getNumTypeObjects(); i < e; ++i) {
+    const DeclaratorChunk &Chunk = TempDeclarator.getTypeObject(i);
+    ParsedAttributes attrs(AttrFactory);
+    attrs.set(Chunk.Common.AttrList);
+    D.AddTypeInfo(Chunk, attrs, SourceLocation());
+  }
+
+  // The missing identifier would have been diagnosed in ParseDirectDeclarator.
+  // If parentheses are required, always suggest them.
+  if (!D.getIdentifier() && !NeedParens)
+    return;
+
+  // Nothing after the brackets has been parsed as part of the Declarator, so
+  // no correction is needed.
+  if (OldEndLoc == D.getLocEnd())
+    return;
+
+  SourceLocation EndBracketLoc = TempDeclarator.getLocEnd();
+
+  // Generate the move bracket error message.
+  SourceRange BracketRange(StartBracketLoc, EndBracketLoc);
+  SourceLocation EndLoc = PP.getLocForEndOfToken(D.getLocEnd());
+
+  if (NeedParens) {
+    Diag(EndLoc, diag::err_brackets_go_after_unqualified_id)
+        << getLangOpts().CPlusPlus
+        << FixItHint::CreateInsertion(SuggestParenLoc, "(")
+        << FixItHint::CreateInsertion(EndLoc, ")")
+        << FixItHint::CreateInsertionFromRange(
+               EndLoc, CharSourceRange(BracketRange, true))
+        << FixItHint::CreateRemoval(BracketRange);
+  } else {
+    Diag(EndLoc, diag::err_brackets_go_after_unqualified_id)
+        << getLangOpts().CPlusPlus
+        << FixItHint::CreateInsertionFromRange(
+               EndLoc, CharSourceRange(BracketRange, true))
+        << FixItHint::CreateRemoval(BracketRange);
+  }
+}
+
 /// [GNU]   typeof-specifier:
 ///           typeof ( expressions )
 ///           typeof ( type-name )
 /// [GNU/C++] typeof unary-expression
 ///
 void Parser::ParseTypeofSpecifier(DeclSpec &DS) {
   assert(Tok.is(tok::kw_typeof) && "Not a typeof specifier");
   Token OpTok = Tok;

test/Parser/brackets.c

+// 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
+  };
+}
+
+void test3() {
+  int [5] *;
+  // expected-error@-1{{expected identifier or '('}}
+  // CHECK: {{^}}  int [5] *;
+  // CHECK: {{^}}           ^
+  // CHECK-NOT: fix-it
+  // expected-error@-5{{brackets go after the identifier}}
+  // CHECK: {{^}}  int [5] *;
+  // CHECK: {{^}}      ~~~~ ^
+  // CHECK: {{^}}          ()[5]
+  // CHECK: fix-it:{{.*}}:{[[@LINE-9]]:7-[[@LINE-9]]:11}:""
+  // CHECK: fix-it:{{.*}}:{[[@LINE-10]]:11-[[@LINE-10]]:11}:"("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-11]]:12-[[@LINE-11]]:12}:")[5]"
+
+  int [5] * a;
+  // expected-error@-1{{brackets go after the identifier}}
+  // CHECK: {{^}}  int [5] * a;
+  // CHECK: {{^}}      ~~~~   ^
+  // CHECK: {{^}}          (  )[5]
+  // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
+  // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:11-[[@LINE-6]]:11}:"("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-7]]:14-[[@LINE-7]]:14}:")[5]"
+
+  int *b[5] = a;  // expected-error{{}} a should not be corrected to type b
+
+  int (*c)[5] = a;  // a should be the same type as c
+}
+#endif
+
+// CHECK: 8 errors generated.

test/Parser/brackets.cpp

+// 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 unqualified-id}}
+  // 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.
+
+  int[1] g[2];
+  // expected-error@-1{{brackets go after the unqualified-id}}
+  // CHECK: {{^}}  int[1] g[2];
+  // CHECK: {{^}}     ~~~     ^
+  // CHECK: {{^}}             [1]
+  // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:6-[[@LINE-5]]:9}:""
+  // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:14-[[@LINE-6]]:14}:"[1]"
+}
+
+void test2() {
+  int [3] (*a) = 0;
+  // expected-error@-1{{brackets go after the unqualified-id}}
+  // CHECK: {{^}}  int [3] (*a) = 0;
+  // CHECK: {{^}}      ~~~~    ^
+  // CHECK: {{^}}              [3]
+  // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
+  // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:15-[[@LINE-6]]:15}:"[3]"
+
+#ifndef FIXIT
+  // Make sure a is corrected to be like type y, instead of like type z.
+  int (*b)[3] = a;
+  int (*c[3]) = a;  // expected-error{{}}
+#endif
+}
+
+struct A {
+  static int [1][1]x;
+  // expected-error@-1{{brackets go after the unqualified-id}}
+  // CHECK: {{^}}  static int [1][1]x;
+  // CHECK: {{^}}             ~~~~~~ ^
+  // CHECK: {{^}}                    [1][1]
+  // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:14-[[@LINE-5]]:20}:""
+  // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:21-[[@LINE-6]]:21}:"[1][1]"
+};
+
+int [1][1]A::x = { {42} };
+// expected-error@-1{{brackets go after the unqualified-id}}
+// CHECK: {{^}}int [1][1]A::x = { {42} };
+// CHECK: {{^}}    ~~~~~~    ^
+// CHECK: {{^}}              [1][1]
+// CHECK: fix-it:{{.*}}:{[[@LINE-5]]:5-[[@LINE-5]]:11}:""
+// CHECK: fix-it:{{.*}}:{[[@LINE-6]]:15-[[@LINE-6]]:15}:"[1][1]"
+
+struct B { static int (*x)[5]; };
+int [5] *B::x = 0;
+// expected-error@-1{{brackets go after the unqualified-id}}
+// CHECK: {{^}}int [5] *B::x = 0;
+// CHECK: {{^}}    ~~~~     ^
+// CHECK: {{^}}        (    )[5]
+// CHECK: fix-it:{{.*}}:{[[@LINE-5]]:5-[[@LINE-5]]:9}:""
+// CHECK: fix-it:{{.*}}:{[[@LINE-6]]:9-[[@LINE-6]]:9}:"("
+// CHECK: fix-it:{{.*}}:{[[@LINE-7]]:14-[[@LINE-7]]:14}:")[5]"
+
+void test3() {
+  int [3] *a;
+  // expected-error@-1{{brackets go after the unqualified-id}}
+  // CHECK: {{^}}  int [3] *a;
+  // CHECK: {{^}}      ~~~~  ^
+  // CHECK: {{^}}          ( )[3]
+  // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
+  // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:11-[[@LINE-6]]:11}:"("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-7]]:13-[[@LINE-7]]:13}:")[3]"
+
+  int (*b)[3] = a;  // no error
+}
+
+void test4() {
+  int [2] a;
+  // expected-error@-1{{brackets go after the unqualified-id}}
+  // CHECK: {{^}}  int [2] a;
+  // CHECK: {{^}}      ~~~~ ^
+  // CHECK: {{^}}           [2]
+  // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
+  // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:12-[[@LINE-6]]:12}:"[2]"
+
+  int [2] &b = a;
+  // expected-error@-1{{brackets go after the unqualified-id}}
+  // CHECK: {{^}}  int [2] &b = a;
+  // CHECK: {{^}}      ~~~~  ^
+  // CHECK: {{^}}          ( )[2]
+  // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
+  // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:11-[[@LINE-6]]:11}:"("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-7]]:13-[[@LINE-7]]:13}:")[2]"
+
+}
+
+namespace test5 {
+#ifndef FIXIT
+int [][][];
+// expected-error@-1{{expected unqualified-id}}
+// CHECK: {{^}}int [][][];
+// CHECK: {{^}}    ^
+
+struct C {
+  int [];
+  // expected-error@-1{{expected member name or ';' after declaration specifiers}}
+  // CHECK: {{^}}  int [];
+  // CHECK: {{^}}  ~~~ ^
+};
+
+#endif
+}
+
+namespace test6 {
+struct A {
+  static int arr[3];
+};
+int [3] ::test6::A::arr = {1,2,3};
+// expected-error@-1{{brackets go after the unqualified-id}}
+// CHECK: {{^}}int [3] ::test6::A::arr = {1,2,3};
+// CHECK: {{^}}    ~~~~               ^
+// CHECK: {{^}}                       [3]
+// CHECK: fix-it:{{.*}}:{[[@LINE-5]]:5-[[@LINE-5]]:9}:""
+// CHECK: fix-it:{{.*}}:{[[@LINE-6]]:24-[[@LINE-6]]:24}:"[3]"
+
+}
+
+namespace test7 {
+class A{};
+void test() {
+  int [3] A::*a;
+  // expected-error@-1{{brackets go after the unqualified-id}}
+  // CHECK: {{^}}  int [3] A::*a;
+  // CHECK: {{^}}      ~~~~     ^
+  // CHECK: {{^}}          (    )[3]
+  // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
+  // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:11-[[@LINE-6]]:11}:"("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-7]]:16-[[@LINE-7]]:16}:")[3]"
+}
+}
+// CHECK: 14 errors generated.