Provide a better diagnostic when braces are put before the identifier.

When a user types:
  int [4] foo;
assume that the user means:
  int foo[4];

Update the information for 'foo' to prevent additional errors, and provide
a fix-it hint to move the brackets to the correct location.

Additionally, suggest parens for types that require it, such as:
  int [4] *foo;
to:
  int (*foo)[4];


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@211641 91177308-0d34-0410-b5e6-96231b3b80d8

5 files changed, 351 insertions, 6 deletions

diff --git a/include/clang/Basic/DiagnosticParseKinds.td b/include/clang/Basic/DiagnosticParseKinds.td
index bc097e0bf3..69e2fb3110 100644
--- a/include/clang/Basic/DiagnosticParseKinds.td
+++ b/include/clang/Basic/DiagnosticParseKinds.td
@@ -448,6 +448,8 @@ 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">;
diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
index 586545eab8..104d4b6712 100644
--- a/include/clang/Parse/Parser.h
+++ b/include/clang/Parse/Parser.h
@@ -2214,6 +2214,7 @@ private:
          SmallVectorImpl<DeclaratorChunk::ParamInfo> &ParamInfo,
          SourceLocation &EllipsisLoc);
   void ParseBracketDeclarator(Declarator &D);
+  void ParseMisplacedBracketDeclarator(Declarator &D);
 
   //===--------------------------------------------------------------------===//
   // C++ 7: Declarations [dcl.dcl]
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index cb0857bd23..09f78e5a75 100644
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -4664,6 +4664,19 @@ void Parser::ParseDeclaratorInternal(Declarator &D,
   }
 }
 
+// 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
@@ -4841,11 +4854,14 @@ void Parser::ParseDirectDeclarator(Declarator &D) {
   } 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 {
@@ -4854,11 +4870,15 @@ void Parser::ParseDirectDeclarator(Declarator &D) {
           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(nullptr, Tok.getLocation());
     D.setInvalidType(true);
   }
@@ -5564,6 +5584,96 @@ void Parser::ParseBracketDeclarator(Declarator &D) {
                 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();
+
+  // 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;
+
+  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 )
diff --git a/test/Parser/brackets.c b/test/Parser/brackets.c
new file mode 100644
index 0000000000..2750d0e42e
--- /dev/null
+++ b/test/Parser/brackets.c
@@ -0,0 +1,79 @@
+// 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.
diff --git a/test/Parser/brackets.cpp b/test/Parser/brackets.cpp
new file mode 100644
index 0000000000..f418c11d3a
--- /dev/null
+++ b/test/Parser/brackets.cpp
@@ -0,0 +1,153 @@
+// 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.