Based on a true story...

Clang currently gives unhelpful diagnostics for cases such as this:

      static const char *const Triples[] = {
        "powerpc-linux-gnu",
        "powerpc-unknown-linux-gnu"
      },
      CandidateLibDirs.append(LibDirs, LibDirs + llvm::array_lengthof(LibDirs));

Viz:

lib/Driver/ToolChains.cpp:1582:7: error: default initialization of an object
of const type 'const char'
     CandidateLibDirs.append(LibDirs, LibDirs + llvm::array_lengthof(LibDirs));
     ^
lib/Driver/ToolChains.cpp:1582:23: error: expected ';' at end of declaration
     CandidateLibDirs.append(LibDirs, LibDirs + llvm::array_lengthof(LibDirs));
                     ^
                     ;

The attached patch is a conservative fix for this issue. In cases where a
declarator group contains a comma followed by a newline followed by something
which obviously is neither a declarator nor a typo for a declarator, we give a
fixit suggesting that a ; was intended:

lib/Driver/ToolChains.cpp:1581:8: error: expected ';' at end of declaration
      },
       ^ ;

OK to commit?

Thanks,
Richard
Index: test/Parser/cxx-decl.cpp
===================================================================
--- test/Parser/cxx-decl.cpp	(revision 141193)
+++ test/Parser/cxx-decl.cpp	(working copy)
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -verify -fsyntax-only -triple i386-linux %s
 
 int x(*g); // expected-error {{use of undeclared identifier 'g'}}
 
@@ -66,6 +66,36 @@
   int z  // expected-error {{expected ';' at end of declaration list}}
 };
 
+// Make sure we know these are legitimate commas and not typos for ';'.
+namespace Commas {
+  struct S {
+    static int a;
+    int c,
+    operator()();
+  };
+
+  int global1,
+  __attribute__(()) global2,
+  (global5),
+  *global6,
+  &global7 = global1,
+  &&global8 = static_cast<int&&>(global1), // expected-warning 2{{rvalue reference}}
+  S::a,
+  global9,
+  global10 = 0,
+  global11 == 0, // expected-error {{did you mean '='}}
+  global12 __attribute__(()),
+  global13(0),
+  global14[2],
+  global15;
+
+  void g() {
+    static int a,
+    b __asm__("ebx"), // expected-error {{expected ';' at end of declaration}}
+    Statics:return;
+  }
+}
+
 // PR5825
 struct test5 {};
 ::new(static_cast<void*>(0)) test5; // expected-error {{expected unqualified-id}}
Index: test/FixIt/fixit.cpp
===================================================================
--- test/FixIt/fixit.cpp	(revision 141193)
+++ test/FixIt/fixit.cpp	(working copy)
@@ -107,4 +107,16 @@
   aPtr = br; // expected-error {{assigning to 'A *' from incompatible type 'B'; take the address with &}}
 }
 
-
+struct S { void f(int, char); };
+int itsAComma,
+itsAComma2 = 0,
+oopsAComma(42), // expected-error {{expected ';' after declaration}}
+AD oopsMoreCommas() {
+  static int n = 0,
+  static char c,
+  &d = c, // expected-error {{expected ';' after declaration}}
+  S s, // expected-error {{expected ';' after declaration}}
+  s.f(n, d);
+  AD ad, // expected-error {{expected ';' after declaration}}
+  return ad;
+}
Index: test/FixIt/fixit.c
===================================================================
--- test/FixIt/fixit.c	(revision 141193)
+++ test/FixIt/fixit.c	(working copy)
@@ -70,3 +70,10 @@
   : c++;
   c = c + 3; L4: return;
 }
+
+int oopsAComma = 0,
+void oopsMoreCommas() {
+  static int a[] = { 0, 1, 2 },
+  static int b[] = { 3, 4, 5 },
+  &a == &b ? oopsMoreCommas() : removeUnusedLabels(a[0]);
+}
Index: test/FixIt/fixit-cxx0x.cpp
===================================================================
--- test/FixIt/fixit-cxx0x.cpp	(revision 141193)
+++ test/FixIt/fixit-cxx0x.cpp	(working copy)
@@ -57,3 +57,9 @@
     // -> constexpr static char *const p = 0;
   };
 }
+
+namespace SemiCommaTypo {
+  int m {},
+  n [[]], // expected-error {{expected ';' at end of declaration}}
+  int o;
+}
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h	(revision 141193)
+++ include/clang/Parse/Parser.h	(working copy)
@@ -1469,6 +1469,7 @@
                                         ParsedAttributes &attrs,
                                         bool RequireSemi,
                                         ForRangeInit *FRI = 0);
+  bool MightBeDeclarator(unsigned Context);
   DeclGroupPtrTy ParseDeclGroup(ParsingDeclSpec &DS, unsigned Context,
                                 bool AllowFunctionDefinitions,
                                 SourceLocation *DeclEnd = 0,
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp	(revision 141193)
+++ lib/Parse/ParseDecl.cpp	(working copy)
@@ -984,6 +984,54 @@
   return ParseDeclGroup(DS, Context, /*FunctionDefs=*/ false, &DeclEnd, FRI);  
 }
 
+/// Returns true if this might be the start of a declarator.
+bool Parser::MightBeDeclarator(unsigned Context) {
+  switch (Tok.getKind()) {
+  case tok::annot_cxxscope:
+  case tok::annot_template_id:
+  case tok::caret:
+  case tok::colon: // Might be a typo for '::'.
+  case tok::coloncolon:
+  case tok::kw___attribute:
+  case tok::kw_alignas:
+  case tok::kw_operator:
+  case tok::l_paren:
+  case tok::star:
+    return true;
+
+  case tok::amp:
+  case tok::ampamp:
+    return getLang().CPlusPlus;
+
+  case tok::identifier:
+    switch (NextToken().getKind()) {
+    case tok::coloncolon:
+    case tok::comma:
+    case tok::equal:
+    case tok::equalequal: // Might be a typo for '='.
+    case tok::kw_asm:
+    case tok::kw___attribute:
+    case tok::l_brace:
+    case tok::l_paren:
+    case tok::l_square:
+    case tok::less:
+    case tok::semi:
+      return true;
+
+    case tok::colon:
+      // At namespace scope, 'identifier:' is probably a typo for 'identifier::'
+      // and in block scope it's probably a label.
+      return Context == Declarator::FileContext;
+
+    default:
+      return false;
+    }
+
+  default:
+    return false;
+  }
+}
+
 /// ParseDeclGroup - Having concluded that this is either a function
 /// definition or a group of object declarations, actually parse the
 /// result.
@@ -1061,12 +1109,23 @@
   if (FirstDecl)
     DeclsInGroup.push_back(FirstDecl);
 
+  bool ExpectSemi = Context != Declarator::ForContext;
+
   // If we don't have a comma, it is either the end of the list (a ';') or an
   // error, bail out.
   while (Tok.is(tok::comma)) {
-    // Consume the comma.
-    ConsumeToken();
+    SourceLocation CommaLoc = ConsumeToken();
 
+    if (Tok.isAtStartOfLine() && ExpectSemi && !MightBeDeclarator(Context)) {
+      // This comma was followed by a line-break and something which can't be
+      // the start of a declarator. The comma was probably a typo for a
+      // semicolon.
+      Diag(CommaLoc, diag::err_expected_semi_declaration)
+        << FixItHint::CreateReplacement(CommaLoc, ";");
+      ExpectSemi = false;
+      break;
+    }
+
     // Parse the next declarator.
     D.clear();
 
@@ -1090,7 +1149,7 @@
   if (DeclEnd)
     *DeclEnd = Tok.getLocation();
 
-  if (Context != Declarator::ForContext &&
+  if (ExpectSemi &&
       ExpectAndConsume(tok::semi,
                        Context == Declarator::FileContext
                          ? diag::err_invalid_token_after_toplevel_declarator
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to