PR16480: Reimplement token-caching for constructor initializer lists. This

previously didn't work if a mem-initializer-id had a template argument which
contained parentheses or braces.

We now implement a simple rule: just look for a ') {' or '} {' that is not
nested. The '{' is assumed to start the function-body. There are still two
cases which we misparse, where the ') {' comes from a compound literal or
from a lambda. The former case is not valid C++, and the latter will probably
not be valid C++ once DR1607 is resolved, so these seem to be of low value,
and we do not regress on them with this change. EDG and g++ also misparse
both of these cases.

llvm-svn: 185598

clang/include/clang/Basic/DiagnosticParseKinds.td

@@ -152,7 +152,7 @@ def err_expected_ident_lparen : Error<"expected identifier or '('">;
 def err_expected_ident_lbrace : Error<"expected identifier or '{'">;
 def err_expected_lbrace : Error<"expected '{'">;
 def err_expected_lparen : Error<"expected '('">;
-def err_expected_lparen_or_lbrace : Error<"expected '('or '{'">;
+def err_expected_lparen_or_lbrace : Error<"expected '(' or '{'">;
 def err_expected_rparen : Error<"expected ')'">;
 def err_expected_lsquare : Error<"expected '['">;
 def err_expected_rsquare : Error<"expected ']'">;

clang/lib/Parse/ParseCXXInlineMethods.cpp

@@ -151,9 +151,9 @@ NamedDecl *Parser::ParseCXXInlineMethodDef(AccessSpecifier AS,
     // We didn't find the left-brace we expected after the
     // constructor initializer; we already printed an error, and it's likely
     // impossible to recover, so don't try to parse this method later.
-    // If we stopped at a semicolon, consume it to avoid an extra warning.
-     if (Tok.is(tok::semi))
-      ConsumeToken();
+    // Skip over the rest of the decl and back to somewhere that looks
+    // reasonable.
+    SkipMalformedDecl();
     delete getCurrentClass().LateParsedDeclarations.back();
     getCurrentClass().LateParsedDeclarations.pop_back();
     return FnD;
@@ -658,26 +658,111 @@ bool Parser::ConsumeAndStoreFunctionPrologue(CachedTokens &Toks) {
     Toks.push_back(Tok);
     ConsumeToken();
   }
-  bool ReadInitializer = false;
-  if (Tok.is(tok::colon)) {
-    // Initializers can contain braces too.
+
+  if (Tok.isNot(tok::colon)) {
+    // Easy case, just a function body.
+
+    // Grab any remaining garbage to be diagnosed later. We stop when we reach a
+    // brace: an opening one is the function body, while a closing one probably
+    // means we've reached the end of the class.
+    ConsumeAndStoreUntil(tok::l_brace, tok::r_brace, Toks,
+                         /*StopAtSemi=*/true,
+                         /*ConsumeFinalToken=*/false);
+    if (Tok.isNot(tok::l_brace))
+      return Diag(Tok.getLocation(), diag::err_expected_lbrace);
+
+    Toks.push_back(Tok);
+    ConsumeBrace();
+    return false;
+  }
+
   Toks.push_back(Tok);
   ConsumeToken();
 
-    while (Tok.is(tok::identifier) || Tok.is(tok::coloncolon)) {
-      if (Tok.is(tok::eof) || Tok.is(tok::semi))
-        return Diag(Tok.getLocation(), diag::err_expected_lbrace);
+  // We can't reliably skip over a mem-initializer-id, because it could be
+  // a template-id involving not-yet-declared names. Given:
+  //
+  //   S ( ) : a < b < c > ( e )
+  //
+  // 'e' might be an initializer or part of a template argument, depending
+  // on whether 'b' is a template.
 
-      // Grab the identifier.
+  // Track whether we might be inside a template argument. We can give
+  // significantly better diagnostics if we know that we're not.
+  bool MightBeTemplateArgument = false;
+
+  while (true) {
+    // Skip over the mem-initializer-id, if possible.
+    if (Tok.is(tok::kw_decltype)) {
+      Toks.push_back(Tok);
+      SourceLocation OpenLoc = ConsumeToken();
+      if (Tok.isNot(tok::l_paren))
+        return Diag(Tok.getLocation(), diag::err_expected_lparen_after)
+                 << "decltype";
+      Toks.push_back(Tok);
+      ConsumeParen();
+      if (!ConsumeAndStoreUntil(tok::r_paren, Toks, /*StopAtSemi=*/true)) {
+        Diag(Tok.getLocation(), diag::err_expected_rparen);
+        Diag(OpenLoc, diag::note_matching) << "(";
+        return true;
+      }
+    }
+    do {
+      // Walk over a component of a nested-name-specifier.
+      if (Tok.is(tok::coloncolon)) {
+        Toks.push_back(Tok);
+        ConsumeToken();
+
+        if (Tok.is(tok::kw_template)) {
+          Toks.push_back(Tok);
+          ConsumeToken();
+        }
+      }
+
+      if (Tok.is(tok::identifier) || Tok.is(tok::kw_template)) {
+        Toks.push_back(Tok);
+        ConsumeToken();
+      } else if (Tok.is(tok::code_completion)) {
+        Toks.push_back(Tok);
+        ConsumeCodeCompletionToken();
+        // Consume the rest of the initializers permissively.
+        // FIXME: We should be able to perform code-completion here even if
+        //        there isn't a subsequent '{' token.
+        MightBeTemplateArgument = true;
+        break;
+      } else {
+        break;
+      }
+    } while (Tok.is(tok::coloncolon));
+
+    if (Tok.is(tok::less))
+      MightBeTemplateArgument = true;
+
+    if (MightBeTemplateArgument) {
+      // We may be inside a template argument list. Grab up to the start of the
+      // next parenthesized initializer or braced-init-list. This *might* be the
+      // initializer, or it might be a subexpression in the template argument
+      // list.
+      // FIXME: Count angle brackets, and clear MightBeTemplateArgument
+      //        if all angles are closed.
       if (!ConsumeAndStoreUntil(tok::l_paren, tok::l_brace, Toks,
                                 /*StopAtSemi=*/true,
-                                /*ConsumeFinalToken=*/false))
-        return Diag(Tok.getLocation(), diag::err_expected_lparen);
+                                /*ConsumeFinalToken=*/false)) {
+        // We're not just missing the initializer, we're also missing the
+        // function body!
+        return Diag(Tok.getLocation(), diag::err_expected_lbrace);
+      }
+    } else if (Tok.isNot(tok::l_paren) && Tok.isNot(tok::l_brace)) {
+      // We found something weird in a mem-initializer-id.
+      return Diag(Tok.getLocation(), getLangOpts().CPlusPlus11
+                                         ? diag::err_expected_lparen_or_lbrace
+                                         : diag::err_expected_lparen);
+    }
 
     tok::TokenKind kind = Tok.getKind();
     Toks.push_back(Tok);
     bool IsLParen = (kind == tok::l_paren);
-      SourceLocation LOpen = Tok.getLocation();
+    SourceLocation OpenLoc = Tok.getLocation();
 
     if (IsLParen) {
       ConsumeParen();
@@ -690,45 +775,48 @@ bool Parser::ConsumeAndStoreFunctionPrologue(CachedTokens &Toks) {
         return false;
     }
 
-      // Grab the initializer
+    // Grab the initializer (or the subexpression of the template argument).
+    // FIXME: If we support lambdas here, we'll need to set StopAtSemi to false
+    //        if we might be inside the braces of a lambda-expression.
     if (!ConsumeAndStoreUntil(IsLParen ? tok::r_paren : tok::r_brace,
                               Toks, /*StopAtSemi=*/true)) {
       Diag(Tok, IsLParen ? diag::err_expected_rparen :
                            diag::err_expected_rbrace);
-        Diag(LOpen, diag::note_matching) << (IsLParen ? "(" : "{");
+      Diag(OpenLoc, diag::note_matching) << (IsLParen ? "(" : "{");
       return true;
     }
 
-      // Grab pack ellipsis, if present
+    // Grab pack ellipsis, if present.
     if (Tok.is(tok::ellipsis)) {
       Toks.push_back(Tok);
       ConsumeToken();
     }
 
-      // Grab the separating comma, if any.
+    // If we know we just consumed a mem-initializer, we must have ',' or '{'
+    // next.
     if (Tok.is(tok::comma)) {
       Toks.push_back(Tok);
       ConsumeToken();
-      } else if (Tok.isNot(tok::l_brace)) {
-        ReadInitializer = true;
-        break;
-      }
-    }
-  }
-
-  // Grab any remaining garbage to be diagnosed later. We stop when we reach a
-  // brace: an opening one is the function body, while a closing one probably
-  // means we've reached the end of the class.
-  ConsumeAndStoreUntil(tok::l_brace, tok::r_brace, Toks,
-                       /*StopAtSemi=*/true,
-                       /*ConsumeFinalToken=*/false);
-  if (Tok.isNot(tok::l_brace)) {
-    if (ReadInitializer)
-      return Diag(Tok.getLocation(), diag::err_expected_lbrace_or_comma);
-    return Diag(Tok.getLocation(), diag::err_expected_lbrace);
-  }
-
+    } else if (Tok.is(tok::l_brace)) {
+      // This is the function body if the ')' or '}' is immediately followed by
+      // a '{'. That cannot happen within a template argument, apart from the
+      // case where a template argument contains a compound literal:
+      //
+      //   S ( ) : a < b < c > ( d ) { }
+      //   // End of declaration, or still inside the template argument?
+      //
+      // ... and the case where the template argument contains a lambda:
+      //
+      //   S ( ) : a < 0 && b < c > ( d ) + [ ] ( ) { return 0; }
+      //     ( ) > ( ) { }
+      //
+      // FIXME: Disambiguate these cases. Note that the latter case is probably
+      //        going to be made ill-formed by core issue 1607.
       Toks.push_back(Tok);
       ConsumeBrace();
       return false;
+    } else if (!MightBeTemplateArgument) {
+      return Diag(Tok.getLocation(), diag::err_expected_lbrace_or_comma);
+    }
+  }
 }

clang/test/Parser/cxx-member-initializers.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
 
 struct x {
   x() : a(4) ; // expected-error {{expected '{'}}
@@ -11,5 +11,71 @@ struct y {
 
 struct z {
   int a;
-  z() : a {} // expected-error {{expected '('}}
-};
+  z() : a {}
+}; // expected-error {{expected '{'}}
+
+namespace PR16480 {
+  template<int n> struct X {
+    X();
+    X(int);
+  };
+
+  struct A : X<0> {
+    A() : X<a<b>{0}.n>() {}
+
+    template<int> struct a {
+      int n;
+    };
+
+    static const int b = 1;
+  };
+
+  struct B : X<0> {
+    B() : X<a<b>{0} {}
+
+    static const int a = 0, b = 0;
+  };
+
+  template<int> struct a {
+    constexpr a(int) {}
+    constexpr operator int() const { return 0; }
+  };
+
+  struct C : X<0> {
+    C() : X<a<b>(0)>() {}
+
+    static const int b = 0;
+  };
+
+  struct D : X<0> {
+    D() : X<a<b>(0) {}
+
+    static const int a = 0, b = 0;
+  };
+
+  template<typename T> struct E : X<0> {
+    E(X<0>) : X<(0)>{} {}
+    E(X<1>) : X<int{}>{} {}
+    E(X<2>) : X<(0)>() {}
+    E(X<3>) : X<int{}>() {}
+  };
+
+  // FIXME: This should be valid in the union of C99 and C++11.
+  struct F : X<0> {
+    F() : X<A<T>().n + (T){}.n>{} {} // expected-error +{{}}
+
+    struct T { int n; };
+    template<typename> struct A { int n; };
+  }; // expected-error +{{}}
+
+  // FIXME: This is valid now, but may be made ill-formed by DR1607.
+  struct G : X<0> {
+    G() : X<0 && [](){return 0;}()>{} // expected-error +{{}}
+  }; // expected-error +{{}}
+
+  struct Errs : X<0> {
+    Errs(X<0>) : decltype X<0>() {} // expected-error {{expected '(' after 'decltype'}}
+    Errs(X<1>) : what is this () {} // expected-error {{expected '(' or '{'}}
+    Errs(X<2>) : decltype(X<0> // expected-note {{to match this '('}}
+  }; // expected-error {{expected ')'}}
+}