Make tentative parsing to detect template-argument-lists less aggressive

(and less wrong).

It's not correct to assume that X<something, Type> is always a
template-id; there are a few cases where the comma takes us into a
non-expression syntactic context in which 'Type' might be permissible.
Stop doing that.

This slightly regresses our error recovery on the cases where the
construct is intended to be a template-id. We typically do still manage
to diagnose a missing 'template' keyword, but we realize this too late
to properly recover from the error.

This fixes a regression introduced by r360308.

llvm-svn: 360827

3 files changed, 39 insertions, 30 deletions

diff --git a/clang/lib/Parse/ParseTentative.cpp b/clang/lib/Parse/ParseTentative.cpp
index 2d2705d8471..4393326f6c7 100644
--- a/clang/lib/Parse/ParseTentative.cpp
+++ b/clang/lib/Parse/ParseTentative.cpp
@@ -2065,33 +2065,31 @@ Parser::TPResult Parser::isTemplateArgumentList(unsigned TokensToSkip) {
   if (!TryConsumeToken(tok::less))
     return TPResult::False;
 
+  // We can't do much to tell an expression apart from a template-argument,
+  // but one good distinguishing factor is that a "decl-specifier" not
+  // followed by '(' or '{' can't appear in an expression.
   bool InvalidAsTemplateArgumentList = false;
-  while (true) {
-    // We can't do much to tell an expression apart from a template-argument,
-    // but one good distinguishing factor is that a "decl-specifier" not
-    // followed by '(' or '{' can't appear in an expression.
-    if (isCXXDeclarationSpecifier(
-            TPResult::False, &InvalidAsTemplateArgumentList) == TPResult::True)
-      return TPResult::True;
-
-    // That didn't help, try the next template-argument.
-    SkipUntil({tok::comma, tok::greater, tok::greatergreater,
-               tok::greatergreatergreater},
-              StopAtSemi | StopBeforeMatch);
-    switch (Tok.getKind()) {
-    case tok::comma:
-      ConsumeToken();
-      break;
+  if (isCXXDeclarationSpecifier(TPResult::False,
+                                       &InvalidAsTemplateArgumentList) ==
+             TPResult::True)
+    return TPResult::True;
+  if (InvalidAsTemplateArgumentList)
+    return TPResult::False;
 
-    case tok::greater:
-    case tok::greatergreater:
-    case tok::greatergreatergreater:
-      if (InvalidAsTemplateArgumentList)
-        return TPResult::False;
-      return TPResult::Ambiguous;
+  // FIXME: In many contexts, X<thing1, Type> can only be a
+  // template-argument-list. But that's not true in general:
+  //
+  // using b = int;
+  // void f() {
+  //   int a = A<B, b, c = C>D; // OK, declares b, not a template-id.
+  //
+  // X<Y<0, int> // ', int>' might be end of X's template argument list
+  //
+  // We might be able to disambiguate a few more cases if we're careful.
 
-    default:
-      return TPResult::False;
-    }
-  }
+  // A template-argument-list must be terminated by a '>'.
+  if (SkipUntil({tok::greater, tok::greatergreater, tok::greatergreatergreater},
+                StopAtSemi | StopBeforeMatch))
+    return TPResult::Ambiguous;
+  return TPResult::False;
 }
diff --git a/clang/test/Parser/cxx-template-argument.cpp b/clang/test/Parser/cxx-template-argument.cpp
index b3a9071785b..70945e3aa74 100644
--- a/clang/test/Parser/cxx-template-argument.cpp
+++ b/clang/test/Parser/cxx-template-argument.cpp
@@ -127,3 +127,14 @@ namespace PR18793 {
   template<typename T, T> struct S {};
   template<typename T> int g(S<T, (T())> *);
 }
+
+namespace r360308_regression {
+  template<typename> struct S1 { static int const n = 0; };
+  template<int, typename> struct S2 { typedef int t; };
+  template<typename T> struct S3 { typename S2<S1<T>::n < 0, int>::t n; };
+
+  template<typename FT> bool f(FT p) {
+    const bool a = p.first<FT(0), b = p.second>FT(0);
+    return a == b;
+  }
+}
diff --git a/clang/test/SemaTemplate/dependent-template-recover.cpp b/clang/test/SemaTemplate/dependent-template-recover.cpp
index 90ba3cd8999..c7e27e8da25 100644
--- a/clang/test/SemaTemplate/dependent-template-recover.cpp
+++ b/clang/test/SemaTemplate/dependent-template-recover.cpp
@@ -7,7 +7,7 @@ struct X {
 
     t->operator+<U const, 1>(1); // expected-error{{use 'template' keyword to treat 'operator +' as a dependent template name}}
     t->f1<int const, 2>(1); // expected-error{{use 'template' keyword to treat 'f1' as a dependent template name}}
-    t->f1<3, int const>(1); // expected-error{{use 'template' keyword to treat 'f1' as a dependent template name}}
+    t->f1<3, int const>(1); // expected-error{{missing 'template' keyword prior to dependent template name 'f1'}}
 
     T::getAs<U>(); // expected-error{{use 'template' keyword to treat 'getAs' as a dependent template name}}
     t->T::getAs<U>(); // expected-error{{use 'template' keyword to treat 'getAs' as a dependent template name}}
@@ -15,7 +15,7 @@ struct X {
     (*t).f2<N>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f2'}}
     (*t).f2<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f2'}}
     T::f2<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f2'}}
-    T::f2<0, int>(0); // expected-error{{use 'template' keyword to treat 'f2' as a dependent template name}}
+    T::f2<0, int>(0); // expected-error{{missing 'template' keyword prior to dependent template name 'f2'}}
 
     T::foo<N < 2 || N >= 4>(); // expected-error{{missing 'template' keyword prior to dependent template name 'foo'}}
 
@@ -83,12 +83,12 @@ template<int N, typename T> void f(T t) {
   T::g<mb>(0);
 
   // ... but this one must be a template-id.
-  T::g<mb, int>(0); // expected-error {{use 'template' keyword to treat 'g' as a dependent template name}} expected-error {{no matching function}}
+  T::g<mb, int>(0); // expected-error {{missing 'template' keyword prior to dependent template name 'g'}}
 }
 
 struct Y {
   template <int> void f(int);
-  template <int = 0> static void g(int); // expected-warning 0-1{{extension}} expected-note {{candidate}}
+  template <int = 0> static void g(int); // expected-warning 0-1{{extension}}
 };
 void q() { void (*p)(int) = Y::g; }
 template void f<0>(Y); // expected-note {{in instantiation of}}