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.

http://llvm-reviews.chandlerc.com/D2712

Files:
  test/Parser/brackets.c
  test/Parser/brackets.cpp
  include/clang/Basic/DiagnosticParseKinds.td
  lib/Parse/ParseDecl.cpp
Index: test/Parser/brackets.c
===================================================================
--- test/Parser/brackets.c
+++ test/Parser/brackets.c
@@ -0,0 +1,45 @@
+// 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
+
+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: {{^}}         []
+
+  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 unqualified-id}}
+  // CHECK: {{^}}  int [1][1]x;
+  // CHECK: {{^}}      ~~~~~~ ^
+  // CHECK: {{^}}             [1][1]
+} s;
+
+#ifndef FIXIT
+void test2() {
+  int [][][];
+  // expected-error@-1{{expected identifier or '('}}
+  // CHECK: {{^}}  int [][][];
+  // CHECK: {{^}}      ^
+  struct T {
+    int [];
+    // expected-error@-1{{expected member name or ';' after declaration specifiers}}
+    // CHECK: {{^}}    int [];
+    // CHECK: {{^}}    ~~~ ^
+  };
+}
+#endif
+
+// CHECK: 4 errors generated.
Index: test/Parser/brackets.cpp
===================================================================
--- test/Parser/brackets.cpp
+++ test/Parser/brackets.cpp
@@ -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 %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]
+
+#ifndef FIXIT
+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: 6 errors generated.
Index: include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- include/clang/Basic/DiagnosticParseKinds.td
+++ include/clang/Basic/DiagnosticParseKinds.td
@@ -435,6 +435,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: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -4712,6 +4712,21 @@
 void Parser::ParseDirectDeclarator(Declarator &D) {
   DeclaratorScopeObj DeclScopeObj(*this, D.getCXXScopeSpec());
 
+  bool UnhandledError = false;
+  SourceLocation StartLoc;
+  SourceLocation EndLoc;
+
+  // 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()) {
+    UnhandledError = true;
+    StartLoc = Tok.getLocation();
+    while (Tok.is(tok::l_square)) {
+      ParseBracketDeclarator(D);
+    }
+    EndLoc = PP.getLocForEndOfToken(D.getLocEnd());
+  }
+
   if (getLangOpts().CPlusPlus && D.mayHaveIdentifier()) {
     // ParseDeclaratorInternal might already have parsed the scope.
     if (D.getCXXScopeSpec().isEmpty()) {
@@ -4843,28 +4858,60 @@
   } 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 the square brackets.
+        UnhandledError = false;
+        DiagLoc = StartLoc;
+      }
+      Diag(DiagLoc, diag::err_expected_member_name_or_semi)
         << 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())
+        if (UnhandledError) {
+          // Place the expected unqualied-id error before the square brackets.
+          UnhandledError = false;
+          Diag(StartLoc, diag::err_expected_unqualified_id)
+              << getLangOpts().CPlusPlus;
+        } else if (Tok.isAtStartOfLine() && Loc.isValid()) {
+          // Point to the end of the current line instead of the beginning of
+          // a new line to give better context for the error.
           Diag(PP.getLocForEndOfToken(Loc), diag::err_expected_unqualified_id)
               << getLangOpts().CPlusPlus;
-        else
+        } else {
           Diag(Tok, 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) {
+        // Place the expected unqualied-id error before the square brackets.
+        UnhandledError = false;
+        DiagLoc = StartLoc;
+      }
+      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.
+    UnhandledError = false;
+    CharSourceRange MoveRange(SourceRange(StartLoc, EndLoc), false);
+    SourceLocation InsertLoc = PP.getLocForEndOfToken(D.getIdentifierLoc());
+    Diag(InsertLoc, diag::err_brackets_go_after_unqualified_id)
+        << true << FixItHint::CreateInsertionFromRange(InsertLoc, MoveRange)
+        << FixItHint::CreateRemoval(MoveRange);
+  }
   assert(D.isPastIdentifier() &&
          "Haven't past the location of the identifier yet?");
 
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to