Suggest fix-it for missing '{' after base-clause

[ismailp]

Summary

Suggest fix-it for missing '{' after base-clause, and try to recover.

[rsmith]

Is this sufficiently high-confidence for a fix-it? Maybe the problem is a missing ,, or someone put virtual after the type, or similar. Maybe we failed to parse a base class template specialization and we're lost in the middle of a template-argument-list?

Let's try a bit harder to be sure here. How about checking for the next token being at the start of a line, and one of the following:

• the next token is public, protected, or private, followed by a :
• the next token is static_assert
• the next tokens are a valid simple-declaration (see Parser::isCXXSimpleDeclaration)

[rsmith]

One more obvious case: the next token is }.

[ismailp]

To increase confidence when suggesting fix-it:

• Expect certain keywords at the start of line, and maybe followed by ':'
• Expect '}'
• Expect a constructor declaration

[rsmith]

LGTM

[ismailp]

Thank you! Landed in r218468.

lib/Parse/ParseDeclCXX.cpp

if (getLangOpts().CPlusPlus && Tok.is(tok::identifier)) {
     // These attributes are not allowed to appear here,
     // and the only possible place for them to appertain
     // to the class would be between class-key and class-name.
     CheckMisplacedCXX11Attribute(Attrs, AttrFixitLoc);
   }
 
   if (Tok.is(tok::colon)) {
     ParseBaseClause(TagDecl);
-
     if (!Tok.is(tok::l_brace)) {
-      Diag(Tok, diag::err_expected_lbrace_after_base_specifiers);
-
+      bool SuggestFixIt = false;
+      SourceLocation BraceLoc = PP.getLocForEndOfToken(PrevTokLocation);
+      if (Tok.isAtStartOfLine()) {
+        switch (Tok.getKind()) {
+        case tok::kw_private:
+        case tok::kw_protected:
+        case tok::kw_public:
+          SuggestFixIt = NextToken().getKind() == tok::colon;
+          break;
+        case tok::kw_static_assert:
+        case tok::r_brace:
+        case tok::kw_using:
+        // base-clause can have simple-template-id; 'template' can't be there
+        case tok::kw_template:
+          SuggestFixIt = true;
+          break;
+        case tok::identifier:
+          SuggestFixIt = isConstructorDeclarator(true);
+          break;
+        default:
+          SuggestFixIt = isCXXSimpleDeclaration(/*AllowForRangeDecl=*/false);
+          break;
+        }
+      }
+      DiagnosticBuilder LBraceDiag =
+          Diag(BraceLoc, diag::err_expected_lbrace_after_base_specifiers);
+      if (SuggestFixIt) {
+        LBraceDiag << FixItHint::CreateInsertion(BraceLoc, " {");
+        // Try recovering from missing { after base-clause.
+        PP.EnterToken(Tok);
+        Tok.setKind(tok::l_brace);
+      } else {
         if (TagDecl)
           Actions.ActOnTagDefinitionError(getCurScope(), TagDecl);
         return;
       }
     }
+  }
 
   assert(Tok.is(tok::l_brace));
   BalancedDelimiterTracker T(*this, tok::l_brace);
   T.consumeOpen();
 
   if (TagDecl)
     Actions.ActOnStartCXXMemberDeclarations(getCurScope(), TagDecl, FinalLoc,
                                             IsFinalSpelledSealed,

test/FixIt/fixit.cpp

 // RUN: %clang_cc1 -pedantic -Wall -Wno-comment -verify -fcxx-exceptions -x c++ %s
-// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits -x c++ %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits -x c++ -std=c++11 %s 2>&1 | FileCheck %s
 // RUN: cp %s %t
 // RUN: not %clang_cc1 -pedantic -Wall -Wno-comment -fcxx-exceptions -fixit -x c++ %t
 // RUN: %clang_cc1 -fsyntax-only -pedantic -Wall -Werror -Wno-comment -fcxx-exceptions -x c++ %t
 
 /* This is a test of the various code modification hints that are
    provided as part of warning or extension diagnostics. All of the
    warnings will be fixed by -fixit, and the resulting file should
    compile cleanly with -Werror -pedantic. */
public:
     int a;
   };
 
   int f() {
     Cl0 c;
     return c->a;  // expected-error {{member reference type 'PR15045::Cl0' is not a pointer; maybe you meant to use '.'?}}
   }
 }
+
+namespace curly_after_base_clause {
+struct A { void f(); };
+struct B : A // expected-error{{expected '{' after base class list}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:13}:" {"
+  int i;
+};
+struct C : A // expected-error{{expected '{' after base class list}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:13}:" {"
+  using A::f;
+};
+struct D : A // expected-error{{expected '{' after base class list}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:13}:" {"
+    protected:
+};
+struct E : A  // expected-error{{expected '{' after base class list}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:13}:" {"
+  template<typename T> struct inner { };
+};
+struct F : A  // expected-error{{expected '{' after base class list}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:13}:" {"
+  F() { }
+};
+#if __cplusplus >= 201103L
+struct G : A  // expected-error{{expected '{' after base class list}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:13}:" {"
+  constexpr G(int) { }
+};
+struct H : A  // expected-error{{expected '{' after base class list}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:13}:" {"
+  static_assert(true, "");
+};
+#endif
+}

test/Parser/cxx-template-decl.cpp

 
 
 void Instantiate() { 
   sassert(sizeof(L<0>::O<int>::Fun(0)) == sizeof(one)); 
   sassert(sizeof(L<0>::O<char>::Fun(0)) == sizeof(one));
 }
 
 }
+
+namespace broken_baseclause {
+template<typename T>
+struct base { };
+
+struct t1 : base<int,
+  public:  // expected-error {{expected expression}}
+};  // expected-error {{expected class name}}
+// expected-error@-1 {{expected '{' after base class list}}
+struct t2 : base<int,
+  public  // expected-error {{expected expression}}
+};  // expected-error {{expected class name}}
+// expected-error@-1 {{expected '{' after base class list}}
+
+}