improve diagnostics for missing 'template' keyword

[rsmith]

Summary

We track when we see a name-shaped expression followed by a < token that we parse as a comparison. Then:

• if we see a token sequence that cannot possibly be an expression but can be a template argument (in particular, a type-id) that follows either a , or the <, diagnose that the < was supposed to start a template argument list
• if we see <>, diagnose that the <> was supposed to be a template argument list
• if we see >(), diagnose that the <[...]> was supposed to be a template argument list

This only changes the diagnostic for error cases, and in practice appears to catch the most common cases where a missing template keyword leads to parse errors within a template.

[kparzysz]

This needs to be restored to the original form. A mistake in a commit message closed this review by accident.

[kparzysz]

Restored the original patch.

[kparzysz]

Please take over, I commandeered to restore the patch.

[dblaikie]

Looks pretty good to me - nice work!

include/clang/Basic/BitmaskEnum.h

+//===--- BitmaskEnum.h - wrapper of LLVM's bitmask enum facility-*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+/// \file
+/// Provides LLVM's BitmaskEnum facility to enumeration types declared in
+/// namespace clang.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_BASIC_BITMASKENUM_H
+#define LLVM_CLANG_BASIC_BITMASKENUM_H
+
+#include "llvm/ADT/BitmaskEnum.h"
+
+namespace clang {
+  LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+}
+
+#endif

include/clang/Parse/Parser.h

 //  This file defines the Parser interface.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_PARSE_PARSER_H
 #define LLVM_CLANG_PARSE_PARSER_H
 
 #include "clang/AST/Availability.h"
+#include "clang/Basic/BitmaskEnum.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Basic/OperatorPrecedence.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Lex/CodeCompletionHandler.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/LoopHint.h"
 #include "clang/Sema/Sema.h"
class Parser : public CodeCompletionHandler {
 
   /// Gathers and cleans up TemplateIdAnnotations when parsing of a
   /// top-level declaration is finished.
   SmallVector<TemplateIdAnnotation *, 16> TemplateIds;
 
   /// Identifiers which have been declared within a tentative parse.
   SmallVector<IdentifierInfo *, 8> TentativelyDeclaredIdentifiers;
 
+  /// Tracker for '<' tokens that might have been intended to be treated as an
+  /// angle bracket instead of a less-than comparison.
+  ///
+  /// This happens when the user intends to form a template-id, but typoes the
+  /// template-name or forgets a 'template' keyword for a dependent template
+  /// name.
+  ///
+  /// We track these locations from the point where we see a '<' with a
+  /// name-like expression on its left until we see a '>' or '>>' that might
+  /// match it.
+  struct AngleBracketTracker {
+    /// Flags used to rank candidate template names when there is more than one
+    /// '<' in a scope.
+    enum Priority : unsigned short {
+      /// A non-dependent name that is a potential typo for a template name.
+      PotentialTypo = 0x0,
+      /// A dependent name that might instantiate to a template-name.
+      DependentName = 0x2,
+
+      /// A space appears before the '<' token.
+      SpaceBeforeLess = 0x0,
+      /// No space before the '<' token
+      NoSpaceBeforeLess = 0x1,
+
+      LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue*/ DependentName)
+    };
+
+    struct Loc {
+      Expr *TemplateName;
+      SourceLocation LessLoc;
+      AngleBracketTracker::Priority Priority;
+      unsigned short ParenCount, BracketCount, BraceCount;
+
+      bool isActive(Parser &P) const {
+        return P.ParenCount == ParenCount && P.BracketCount == BracketCount &&
+               P.BraceCount == BraceCount;
+      }
+
+      bool isActiveOrNested(Parser &P) const {
+        return isActive(P) || P.ParenCount > ParenCount ||
+               P.BracketCount > BracketCount || P.BraceCount > BraceCount;
+      }
+    };
+
+    SmallVector<Loc, 8> Locs;
+
+    /// Add an expression that might have been intended to be a template name.
+    /// In the case of ambiguity, we arbitrarily select the innermost such
+    /// expression, for example in 'foo < bar < baz', 'bar' is the current
+    /// candidate. No attempt is made to track that 'foo' is also a candidate
+    /// for the case where we see a second suspicious '>' token.
+    void add(Parser &P, Expr *TemplateName, SourceLocation LessLoc,
+             Priority Prio) {
+      if (!Locs.empty() && Locs.back().isActive(P)) {
+        if (Locs.back().Priority <= Prio) {
+          Locs.back().TemplateName = TemplateName;
+          Locs.back().LessLoc = LessLoc;
+          Locs.back().Priority = Prio;
+        }
+      } else {
+        Locs.push_back({TemplateName, LessLoc, Prio,
+                        P.ParenCount, P.BracketCount, P.BraceCount});
+      }
+    }
+
+    /// Mark the current potential missing template location as having been
+    /// handled (this happens if we pass a "corresponding" '>' or '>>' token
+    /// or leave a bracket scope).
+    void clear(Parser &P) {
+      while (!Locs.empty() && Locs.back().isActiveOrNested(P))
+        Locs.pop_back();
+    }
+
+    /// Get the current enclosing expression that might hve been intended to be
+    /// a template name.
+    Loc *getCurrent(Parser &P) {
+      if (!Locs.empty() && Locs.back().isActive(P))
+        return &Locs.back();
+      return nullptr;
+    }
+  };
+
+  AngleBracketTracker AngleBrackets;
+
   IdentifierInfo *getSEHExceptKeyword();
 
   /// True if we are within an Objective-C container while parsing C-like decls.
   ///
   /// This is necessary because Sema thinks we have left the container
   /// to parse the C-like decls, meaning Actions.getObjCDeclContext() will
   /// be NULL.
   bool ParsingInObjCContainer;
private:
   }
 
   /// ConsumeParen - This consume method keeps the paren count up-to-date.
   ///
   SourceLocation ConsumeParen() {
     assert(isTokenParen() && "wrong consume method");
     if (Tok.getKind() == tok::l_paren)
       ++ParenCount;
-    else if (ParenCount)
+    else if (ParenCount) {
+      AngleBrackets.clear(*this);
       --ParenCount;       // Don't let unbalanced )'s drive the count negative.
+    }
     PrevTokLocation = Tok.getLocation();
     PP.Lex(Tok);
     return PrevTokLocation;
   }
 
   /// ConsumeBracket - This consume method keeps the bracket count up-to-date.
   ///
   SourceLocation ConsumeBracket() {
     assert(isTokenBracket() && "wrong consume method");
     if (Tok.getKind() == tok::l_square)
       ++BracketCount;
-    else if (BracketCount)
+    else if (BracketCount) {
+      AngleBrackets.clear(*this);
       --BracketCount;     // Don't let unbalanced ]'s drive the count negative.
+    }
 
     PrevTokLocation = Tok.getLocation();
     PP.Lex(Tok);
     return PrevTokLocation;
   }
 
   /// ConsumeBrace - This consume method keeps the brace count up-to-date.
   ///
   SourceLocation ConsumeBrace() {
     assert(isTokenBrace() && "wrong consume method");
     if (Tok.getKind() == tok::l_brace)
       ++BraceCount;
-    else if (BraceCount)
+    else if (BraceCount) {
+      AngleBrackets.clear(*this);
       --BraceCount;     // Don't let unbalanced }'s drive the count negative.
+    }
 
     PrevTokLocation = Tok.getLocation();
     PP.Lex(Tok);
     return PrevTokLocation;
   }
 
   /// ConsumeStringToken - Consume the current 'peek token', lexing a new one
   /// and returning the token kind.  This method is specific to strings, as it

include/clang/Parse/RAIIObjectsForParser.h

class ParenBraceBracketBalancer {
     Parser &P;
     unsigned short ParenCount, BracketCount, BraceCount;
   public:
     ParenBraceBracketBalancer(Parser &p)
       : P(p), ParenCount(p.ParenCount), BracketCount(p.BracketCount),
         BraceCount(p.BraceCount) { }
     
     ~ParenBraceBracketBalancer() {
+      P.AngleBrackets.clear(P);
       P.ParenCount = ParenCount;
       P.BracketCount = BracketCount;
       P.BraceCount = BraceCount;
     }
   };
 
   class PoisonSEHIdentifiersRAIIObject {
     PoisonIdentifierRAIIObject Ident_AbnormalTermination;

include/clang/Sema/Sema.h

 #define LLVM_CLANG_SEMA_SEMA_H
 
 #include "clang/AST/Attr.h"
 #include "clang/AST/Availability.h"
 #include "clang/AST/ComparisonCategories.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExternalASTSource.h"
 #include "clang/AST/LocInfoType.h"
 #include "clang/AST/MangleNumberingContext.h"
 #include "clang/AST/NSAPI.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/TypeLoc.h"
enum class TemplateNameKindForDiagnostics {
     AliasTemplate,
     TemplateTemplateParam,
     DependentTemplate
   };
   TemplateNameKindForDiagnostics
   getTemplateNameKindForDiagnostics(TemplateName Name);
 
   /// Determine whether it's plausible that E was intended to be a
-  /// template-name.
-  bool mightBeIntendedToBeTemplateName(ExprResult E) {
-    if (!getLangOpts().CPlusPlus || E.isInvalid())
+  /// template-name. Updates E to denote the template name expression.
+  bool mightBeIntendedToBeTemplateName(ExprResult &ER, bool &Dependent) {
+    if (!getLangOpts().CPlusPlus || ER.isInvalid())
       return false;
-    if (auto *DRE = dyn_cast<DeclRefExpr>(E.get()))
+    Expr *E = ER.get()->IgnoreImplicit();
+    while (auto *BO = dyn_cast<BinaryOperator>(E))
+      E = BO->getRHS()->IgnoreImplicit();
+    ER = E;
+    Dependent = false;
+    if (auto *DRE = dyn_cast<DeclRefExpr>(E))
       return !DRE->hasExplicitTemplateArgs();
-    if (auto *ME = dyn_cast<MemberExpr>(E.get()))
+    if (auto *ME = dyn_cast<MemberExpr>(E))
       return !ME->hasExplicitTemplateArgs();
+    Dependent = true;
+    if (auto *DSDRE = dyn_cast<DependentScopeDeclRefExpr>(E))
+      return !DSDRE->hasExplicitTemplateArgs();
+    if (auto *DSME = dyn_cast<CXXDependentScopeMemberExpr>(E))
+      return !DSME->hasExplicitTemplateArgs();
     // Any additional cases recognized here should also be handled by
     // diagnoseExprIntendedAsTemplateName.
     return false;
   }
   void diagnoseExprIntendedAsTemplateName(Scope *S, ExprResult TemplateName,
                                           SourceLocation Less,
                                           SourceLocation Greater);
 

lib/CodeGen/CGOpenMPRuntime.cpp

 #include "CGCXXABI.h"
 #include "CGCleanup.h"
 #include "CGOpenMPRuntime.h"
 #include "CGRecordLayout.h"
 #include "CodeGenFunction.h"
 #include "clang/CodeGen/ConstantInitBuilder.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Basic/BitmaskEnum.h"
 #include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/BitmaskEnum.h"
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/Value.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>

lib/Parse/ParseCXXInlineMethods.cpp

ParseScope PrototypeScope(this, Scope::FunctionPrototypeScope |
                             Scope::FunctionDeclarationScope | Scope::DeclScope);
   for (unsigned I = 0, N = LM.DefaultArgs.size(); I != N; ++I) {
     auto Param = cast<ParmVarDecl>(LM.DefaultArgs[I].Param);
     // Introduce the parameter into scope.
     bool HasUnparsed = Param->hasUnparsedDefaultArg();
     Actions.ActOnDelayedCXXMethodParameter(getCurScope(), Param);
     std::unique_ptr<CachedTokens> Toks = std::move(LM.DefaultArgs[I].Toks);
     if (Toks) {
+      ParenBraceBracketBalancer BalancerRAIIObj(*this);
+
       // Mark the end of the default argument so that we know when to stop when
       // we parse it later on.
       Token LastDefaultArgToken = Toks->back();
       Token DefArgEnd;
       DefArgEnd.startToken();
       DefArgEnd.setKind(tok::eof);
       DefArgEnd.setLocation(LastDefaultArgToken.getEndLoc());
       DefArgEnd.setEofData(Param);
if (Toks) {
             OldParam->getUninstantiatedDefaultArg());
       else
         Param->setDefaultArg(OldParam->getInit());
     }
   }
 
   // Parse a delayed exception-specification, if there is one.
   if (CachedTokens *Toks = LM.ExceptionSpecTokens) {
+    ParenBraceBracketBalancer BalancerRAIIObj(*this);
+
     // Add the 'stop' token.
     Token LastExceptionSpecToken = Toks->back();
     Token ExceptionSpecEnd;
     ExceptionSpecEnd.startToken();
     ExceptionSpecEnd.setKind(tok::eof);
     ExceptionSpecEnd.setLocation(LastExceptionSpecToken.getEndLoc());
     ExceptionSpecEnd.setEofData(LM.Method);
     Toks->push_back(ExceptionSpecEnd);
void Parser::ParseLexedMethodDef(LexedMethod &LM) {
   // If this is a member template, introduce the template parameter scope.
   ParseScope TemplateScope(this, Scope::TemplateParamScope, LM.TemplateScope);
   TemplateParameterDepthRAII CurTemplateDepthTracker(TemplateParameterDepth);
   if (LM.TemplateScope) {
     Actions.ActOnReenterTemplateScope(getCurScope(), LM.D);
     ++CurTemplateDepthTracker;
   }
 
+  ParenBraceBracketBalancer BalancerRAIIObj(*this);
+
   assert(!LM.Toks.empty() && "Empty body!");
   Token LastBodyToken = LM.Toks.back();
   Token BodyEnd;
   BodyEnd.startToken();
   BodyEnd.setKind(tok::eof);
   BodyEnd.setLocation(LastBodyToken.getEndLoc());
   BodyEnd.setEofData(LM.D);
   LM.Toks.push_back(BodyEnd);
void Parser::ParseLexedMemberInitializers(ParsingClass &Class) {
 
   Actions.ActOnFinishDelayedMemberInitializers(Class.TagOrTemplate);
 }
 
 void Parser::ParseLexedMemberInitializer(LateParsedMemberInitializer &MI) {
   if (!MI.Field || MI.Field->isInvalidDecl())
     return;
 
+  ParenBraceBracketBalancer BalancerRAIIObj(*this);
+
   // Append the current token at the end of the new token stream so that it
   // doesn't get lost.
   MI.Toks.push_back(Tok);
   PP.EnterTokenStream(MI.Toks, true);
 
   // Consume the previously pushed token.
   ConsumeAnyToken(/*ConsumeCodeCompletionTok=*/true);
 

lib/Parse/ParseDeclCXX.cpp

void Parser::SkipCXXMemberSpecification(SourceLocation RecordLoc,
   ParsedAttributes Attrs(AttrFactory);
   if (Tok.is(tok::kw___attribute))
     MaybeParseGNUAttributes(Attrs);
 }
 
 Parser::DeclGroupPtrTy Parser::ParseCXXClassMemberDeclarationWithPragmas(
     AccessSpecifier &AS, ParsedAttributesWithRange &AccessAttrs,
     DeclSpec::TST TagType, Decl *TagDecl) {
+  ParenBraceBracketBalancer BalancerRAIIObj(*this);
+
   switch (Tok.getKind()) {
   case tok::kw___if_exists:
   case tok::kw___if_not_exists:
     ParseMicrosoftIfExistsClassDeclaration(TagType, AS);
     return nullptr;
 
   case tok::semi:
     // Check for extraneous top-level semicolon.

lib/Parse/ParseExpr.cpp

while (1) {
 
     // Consume the operator, saving the operator token for error reporting.
     Token OpToken = Tok;
     ConsumeToken();
 
     if (OpToken.is(tok::caretcaret)) {
       return ExprError(Diag(Tok, diag::err_opencl_logical_exclusive_or));
     }
+
+    // If we have a name-shaped expression followed by '<', track it in case we
+    // later find we're probably supposed to be in a template-id.
+    ExprResult TemplateName = LHS;
+    bool DependentTemplateName = false;
+    if (OpToken.is(tok::less) && Actions.mightBeIntendedToBeTemplateName(
+                                     TemplateName, DependentTemplateName)) {
+      AngleBracketTracker::Priority Priority =
+          (DependentTemplateName ? AngleBracketTracker::DependentName
+                                 : AngleBracketTracker::PotentialTypo) |
+          (OpToken.hasLeadingSpace() ? AngleBracketTracker::SpaceBeforeLess
+                                     : AngleBracketTracker::NoSpaceBeforeLess);
+      AngleBrackets.add(*this, TemplateName.get(), OpToken.getLocation(),
+                        Priority);
+    }
+
+    // If we're potentially in a template-id, we may now be able to determine
+    // whether we're actually in one or not.
+    if (auto *Info = AngleBrackets.getCurrent(*this)) {
+      // If an operator is followed by a type that can be a template argument
+      // and cannot be an expression, then this is ill-formed, but might be
+      // intended to be part of a template-id. Likewise if this is <>.
+      if ((OpToken.isOneOf(tok::less, tok::comma) &&
+            isKnownToBeDeclarationSpecifier()) ||
+           (OpToken.is(tok::less) &&
+            Tok.isOneOf(tok::greater, tok::greatergreater,
+                        tok::greatergreatergreater))) {
+        if (diagnoseUnknownTemplateId(Info->TemplateName, Info->LessLoc)) {
+          AngleBrackets.clear(*this);
+          return ExprError();
+        }
+      }
+
+      // If a context that looks like a template-id is followed by '()', then
+      // this is ill-formed, but might be intended to be a template-id followed
+      // by '()'.
+      if (OpToken.is(tok::greater) && Tok.is(tok::l_paren) &&
+          NextToken().is(tok::r_paren)) {
+        Actions.diagnoseExprIntendedAsTemplateName(
+            getCurScope(), Info->TemplateName, Info->LessLoc,
+            OpToken.getLocation());
+        AngleBrackets.clear(*this);
+        return ExprError();
+      }
+    }
+
+    // After a '>' (etc), we're no longer potentially in a construct that's
+    // intended to be treated as a template-id.
+    if (OpToken.is(tok::greater) ||
+        (getLangOpts().CPlusPlus11 &&
+         OpToken.isOneOf(tok::greatergreater, tok::greatergreatergreater)))
+      AngleBrackets.clear(*this);
+
     // Bail out when encountering a comma followed by a token which can't
     // possibly be the start of an expression. For instance:
     //   int f() { return 1, }
     // We can't do this before consuming the comma, because
     // isNotExpressionStart() looks at the token stream.
     if (OpToken.is(tok::comma) && isNotExpressionStart()) {
       PP.EnterToken(Tok);
       Tok = OpToken;
       return LHS;
     }
 
-    // If a '<' token is followed by a type that can be a template argument and
-    // cannot be an expression, then this is ill-formed, but might be intended
-    // to be a template-id.
-    if (OpToken.is(tok::less) && Actions.mightBeIntendedToBeTemplateName(LHS) &&
-        (isKnownToBeDeclarationSpecifier() ||
-         Tok.isOneOf(tok::greater, tok::greatergreater,
-                     tok::greatergreatergreater)) &&
-        diagnoseUnknownTemplateId(LHS, OpToken.getLocation()))
-      return ExprError();
-
     // If the next token is an ellipsis, then this is a fold-expression. Leave
     // it alone so we can handle it in the paren expression.
     if (isFoldOperator(NextTokPrec) && Tok.is(tok::ellipsis)) {
       // FIXME: We can't check this via lookahead before we consume the token
       // because that tickles a lexer bug.
       PP.EnterToken(Tok);
       Tok = OpToken;
       return LHS;

lib/Parse/ParseExprCXX.cpp

 ///
 /// \param Loc The location of the start of the statement that requires this
 /// condition, e.g., the "for" in a for loop.
 ///
 /// \returns The parsed condition.
 Sema::ConditionResult Parser::ParseCXXCondition(StmtResult *InitStmt,
                                                 SourceLocation Loc,
                                                 Sema::ConditionKind CK) {
+  ParenBraceBracketBalancer BalancerRAIIObj(*this);
+
   if (Tok.is(tok::code_completion)) {
     Actions.CodeCompleteOrdinaryName(getCurScope(), Sema::PCC_Condition);
     cutOffParsing();
     return Sema::ConditionError();
   }
 
   ParsedAttributesWithRange attrs(AttrFactory);
   MaybeParseCXX11Attributes(attrs);

lib/Parse/ParseStmt.cpp

Diag(Loc, diag::err_for_range_identifier)
       << ((getLangOpts().CPlusPlus11 && !getLangOpts().CPlusPlus17)
               ? FixItHint::CreateInsertion(Loc, "auto &&")
               : FixItHint());
 
     FirstPart = Actions.ActOnCXXForRangeIdentifier(getCurScope(), Loc, Name,
                                                    attrs, attrs.Range.getEnd());
     ForRange = true;
   } else if (isForInitDeclaration()) {  // for (int X = 4;
+    ParenBraceBracketBalancer BalancerRAIIObj(*this);
+
     // Parse declaration, which eats the ';'.
     if (!C99orCXXorObjC)   // Use of C99-style for loops in C90 mode?
       Diag(Tok, diag::ext_c99_variable_decl_in_for_loop);
 
     // In C++0x, "for (T NS:a" might not be a typo for ::
     bool MightBeForRangeStmt = getLangOpts().CPlusPlus;
     ColonProtectionRAIIObject ColonProtection(*this, MightBeForRangeStmt);
 

lib/Sema/SemaTemplate.cpp

if (TemplateName.isInvalid())
     return;
 
   DeclarationNameInfo NameInfo;
   CXXScopeSpec SS;
   LookupNameKind LookupKind;
 
   DeclContext *LookupCtx = nullptr;
   NamedDecl *Found = nullptr;
+  bool MissingTemplateKeyword = false;
 
   // Figure out what name we looked up.
-  if (auto *ME = dyn_cast<MemberExpr>(TemplateName.get())) {
+  if (auto *DRE = dyn_cast<DeclRefExpr>(TemplateName.get())) {
+    NameInfo = DRE->getNameInfo();
+    SS.Adopt(DRE->getQualifierLoc());
+    LookupKind = LookupOrdinaryName;
+    Found = DRE->getFoundDecl();
+  } else if (auto *ME = dyn_cast<MemberExpr>(TemplateName.get())) {
     NameInfo = ME->getMemberNameInfo();
     SS.Adopt(ME->getQualifierLoc());
     LookupKind = LookupMemberName;
     LookupCtx = ME->getBase()->getType()->getAsCXXRecordDecl();
     Found = ME->getMemberDecl();
-  } else {
-    auto *DRE = cast<DeclRefExpr>(TemplateName.get());
-    NameInfo = DRE->getNameInfo();
-    SS.Adopt(DRE->getQualifierLoc());
-    LookupKind = LookupOrdinaryName;
-    Found = DRE->getFoundDecl();
+  } else if (auto *DSDRE =
+                 dyn_cast<DependentScopeDeclRefExpr>(TemplateName.get())) {
+    NameInfo = DSDRE->getNameInfo();
+    SS.Adopt(DSDRE->getQualifierLoc());
+    MissingTemplateKeyword = true;
+  } else if (auto *DSME =
+                 dyn_cast<CXXDependentScopeMemberExpr>(TemplateName.get())) {
+    NameInfo = DSME->getMemberNameInfo();
+    SS.Adopt(DSME->getQualifierLoc());
+    MissingTemplateKeyword = true;
+  } else {
+    llvm_unreachable("unexpected kind of potential template name");
+  }
+
+  // If this is a dependent-scope lookup, diagnose that the 'template' keyword
+  // was missing.
+  if (MissingTemplateKeyword) {
+    Diag(NameInfo.getLocStart(), diag::err_template_kw_missing)
+        << "" << NameInfo.getName().getAsString()
+        << SourceRange(Less, Greater);
+    return;
   }
 
   // Try to correct the name by looking for templates and C++ named casts.
   struct TemplateCandidateFilter : CorrectionCandidateCallback {
     TemplateCandidateFilter() {
       WantTypeSpecifiers = false;
       WantExpressionKeywords = false;
       WantRemainingKeywords = false;

test/SemaTemplate/dependent-template-recover.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify %s
 template<typename T, typename U, int N>
 struct X {
   void f(T* t) {
     t->f0<U>(); // expected-error{{use 'template' keyword to treat 'f0' as a dependent template name}}
     t->f0<int>(); // expected-error{{use 'template' keyword to treat 'f0' as a dependent template name}}
 
-    t->operator+<U const, 1>(); // expected-error{{use 'template' keyword to treat 'operator +' as a dependent template name}}
-    t->f1<int const, 2>(); // expected-error{{use 'template' keyword to treat 'f1' as a dependent template name}}
+    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{{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}}
 
-    // FIXME: We can't recover from these yet
-    (*t).f2<N>(); // expected-error{{expected expression}}
-    (*t).f2<0>(); // expected-error{{expected expression}}
+    (*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{{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'}}
+
+    // If there are multiple potential template names, pick the one where there
+    // is no whitespace between the name and the '<'.
+    T::foo<T::bar < 1>(); // expected-error{{missing 'template' keyword prior to dependent template name 'foo'}}
+    T::foo < T::bar<1>(); // expected-error{{missing 'template' keyword prior to dependent template name 'bar'}}
+
+    // Prefer to diagonse a missing 'template' keyword rather than finding a non-template name.
+    xyz < T::foo < 1 > (); // expected-error{{missing 'template' keyword prior to dependent template name 'foo'}}
+    T::foo < xyz < 1 > (); // expected-error{{missing 'template' keyword prior to dependent template name 'foo'}}
+
+    // ... even if the whitespace suggests the other name is the template name.
+    // FIXME: Is this the right heuristic?
+    xyz<T::foo < 1>(); // expected-error{{missing 'template' keyword prior to dependent template name 'foo'}}
+    T::foo < xyz<1>(); // expected-error{{missing 'template' keyword prior to dependent template name 'foo'}}
   }
+
+  int xyz;
 };
 
+template <typename T> void not_missing_template(T t) {
+  (T::n < 0) > (
+     ) // expected-error {{expected expression}}
+    ;
+
+  int a = T::x < 3;
+  int b = T::y > (); // expected-error {{expected expression}}
+
+  void c(int = T::x < 3);
+  void d(int = T::y > ()); // expected-error {{expected expression}}
+
+  for (int x = t < 3 ? 1 : 2; t > (); ++t) { // expected-error {{expected expression}}
+  }
+
+  // FIXME: We shouldn't treat 'T::t' as a potential template-name here,
+  // because that would leave a '?' with no matching ':'.
+  // We should probably generally treat '?' ... ':' as a bracket-like
+  // construct.
+  bool k = T::t < 3 ? 1 > () : false; // expected-error {{missing 'template' keyword}} expected-error +{{}} expected-note +{{}}
+}
+
 struct MrsBadcrumble {
   friend MrsBadcrumble operator<(void (*)(int), MrsBadcrumble);
   friend void operator>(MrsBadcrumble, int);
 } mb;
 
 template<int N, typename T> void f(T t) {
   t.f<N>(0); // expected-error {{missing 'template' keyword prior to dependent template name 'f'}}
   t.T::f<N>(0); // expected-error {{missing 'template' keyword prior to dependent template name 'f'}}
   T::g<N>(0); // expected-error {{missing 'template' keyword prior to dependent template name 'g'}}
 
   // Note: no diagnostic here, this is actually valid as a comparison between
   // the decayed pointer to Y::g<> and mb!
   T::g<mb>(0);
+
+  // ... but this one must be a template-id.
+  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}}
 };
 void q() { void (*p)(int) = Y::g; }
 template void f<0>(Y); // expected-note {{in instantiation of}}