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.

http://reviews.llvm.org/D2712

Files:
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Parse/Parser.h
  lib/Parse/ParseDecl.cpp
  test/Parser/brackets.c
  test/Parser/brackets.cpp
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -4656,6 +4656,18 @@
   }
 }
 
+// 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 getDiagLoc(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
@@ -4832,24 +4844,72 @@
   } 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)) {
+      SavedBracketInfo SavedInfo;
+      SourceLocation Loc = Tok.getLocation();
+
+      while (Tok.is(tok::l_square)) {
+        ParseBracketDeclarator(D, &SavedInfo);
+      }
+
+      // 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.
+      D.getName().EndLocation = Loc;
+
+      // Now that the brackets are removed, try parsing the declarator again.
+      ParseDirectDeclarator(D);
+
+      // Something went wrong parsing the parens, in which case,
+      // ParseBracketDeclarator has emitted an error, and we don't need to emit
+      // one here.
+      if (SavedInfo.empty())
+        return;
+
+      // Adding back the paren info to the end of the Declarator.
+      for (const auto &TypeInfo : SavedInfo) {
+        ParsedAttributes attrs(AttrFactory);
+        attrs.set(TypeInfo.second);
+        D.AddTypeInfo(TypeInfo.first, attrs, SourceLocation());
+      }
+
+      // The missing identifier would have been diagnosed in
+      // ParseDirectDeclarator.
+      if (!D.getIdentifier())
+        return;
+
+      // Generate the move bracket error message.
+      SourceRange BracketRange(Loc, SavedInfo.back().first.EndLoc);
+      SourceLocation EndLoc = PP.getLocForEndOfToken(D.getLocEnd());
+      Diag(EndLoc, diag::err_brackets_go_after_unqualified_id)
+          << getLangOpts().CPlusPlus
+          << FixItHint::CreateInsertionFromRange(
+                 EndLoc, CharSourceRange(BracketRange, true))
+          << FixItHint::CreateRemoval(BracketRange);
+
+      return;
+    } else if (D.getContext() == Declarator::MemberContext) {
+      Diag(getDiagLoc(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(getDiagLoc(D, Tok.getLocation()),
+               diag::err_expected_unqualified_id)
               << getLangOpts().CPlusPlus;
       }
-    } else
-      Diag(Tok, diag::err_expected_either) << tok::identifier << tok::l_paren;
+    } else {
+      Diag(getDiagLoc(D, Tok.getLocation()), diag::err_expected_either)
+          << tok::identifier << tok::l_paren;
+    }
     D.SetIdentifier(0, Tok.getLocation());
     D.setInvalidType(true);
   }
@@ -5447,7 +5507,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 +5525,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 +5548,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 +5624,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.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.
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: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -2207,7 +2207,11 @@
          ParsedAttributes &attrs,
          SmallVectorImpl<DeclaratorChunk::ParamInfo> &ParamInfo,
          SourceLocation &EllipsisLoc);
-  void ParseBracketDeclarator(Declarator &D);
+
+  typedef SmallVector<std::pair<DeclaratorChunk, AttributeList *>, 4>
+          SavedBracketInfo;
+  void ParseBracketDeclarator(Declarator &D,
+                              SavedBracketInfo *SavedInfo = 0);
 
   //===--------------------------------------------------------------------===//
   // C++ 7: Declarations [dcl.dcl]
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">;
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to