PR45239: Don't deallocate TemplateIdAnnotations if they might still be

in the token stream.

Previously we deleted all template-id annotations at the end of each
top-level declaration. That doesn't work: we can do some lookahead and
form a template-id annotation, and then roll back that lookahead, parse,
and decide that we're missing a semicolon at the end of a top-level
declaration, before we reach the annotation token. In that situation,
we'd end up parsing the annotation token after deleting its associated
data, leading to various forms of badness.

We now only delete template-id annotations if the preprocessor can
assure us that there are no annotation tokens left in the token stream
(or if we're already at EOF). This lets us delete the annotation tokens
earlier in a lot of cases; we now clean them up at the end of each
statement and class member, not just after each top-level declaration.
This also permitted some simplification of the delay-parsed templates
cleanup code.

clang/include/clang/Lex/Preprocessor.h

@@ -1562,6 +1562,12 @@ class Preprocessor {
   void EnterAnnotationToken(SourceRange Range, tok::TokenKind Kind,
                             void *AnnotationVal);
 
+  /// Determine whether it's possible for a future call to Lex to produce an
+  /// annotation token created by a previous call to EnterAnnotationToken.
+  bool mightHavePendingAnnotationTokens() {
+    return CurLexerKind != CLK_Lexer;
+  }
+
   /// Update the current token to represent the provided
   /// identifier, in order to cache an action performed by typo correction.
   void TypoCorrectToken(const Token &Tok) {

clang/include/clang/Parse/Parser.h

@@ -276,6 +276,22 @@ class Parser : public CodeCompletionHandler {
   /// top-level declaration is finished.
   SmallVector<TemplateIdAnnotation *, 16> TemplateIds;
 
+  void MaybeDestroyTemplateIds() {
+    if (!TemplateIds.empty() &&
+        (Tok.is(tok::eof) || !PP.mightHavePendingAnnotationTokens()))
+      DestroyTemplateIds();
+  }
+  void DestroyTemplateIds();
+
+  /// RAII object to destroy TemplateIdAnnotations where possible, from a
+  /// likely-good position during parsing.
+  struct DestroyTemplateIdAnnotationsRAIIObj {
+    Parser &Self;
+
+    DestroyTemplateIdAnnotationsRAIIObj(Parser &Self) : Self(Self) {}
+    ~DestroyTemplateIdAnnotationsRAIIObj() { Self.MaybeDestroyTemplateIds(); }
+  };
+
   /// Identifiers which have been declared within a tentative parse.
   SmallVector<IdentifierInfo *, 8> TentativelyDeclaredIdentifiers;
 
@@ -1466,7 +1482,6 @@ class Parser : public CodeCompletionHandler {
   void ParseLateTemplatedFuncDef(LateParsedTemplate &LPT);
 
   static void LateTemplateParserCallback(void *P, LateParsedTemplate &LPT);
-  static void LateTemplateParserCleanupCallback(void *P);
 
   Sema::ParsingClassState
   PushParsingClass(Decl *TagOrTemplate, bool TopLevelClass, bool IsInterface);

clang/include/clang/Parse/RAIIObjectsForParser.h

@@ -459,26 +459,6 @@ namespace clang {
     }
     void skipToEnd();
   };
-
-  /// RAIIObject to destroy the contents of a SmallVector of
-  /// TemplateIdAnnotation pointers and clear the vector.
-  class DestroyTemplateIdAnnotationsRAIIObj {
-    SmallVectorImpl<TemplateIdAnnotation *> &Container;
-
-  public:
-    DestroyTemplateIdAnnotationsRAIIObj(
-        SmallVectorImpl<TemplateIdAnnotation *> &Container)
-        : Container(Container) {}
-
-    ~DestroyTemplateIdAnnotationsRAIIObj() {
-      for (SmallVectorImpl<TemplateIdAnnotation *>::iterator I =
-               Container.begin(),
-             E = Container.end();
-           I != E; ++I)
-        (*I)->Destroy();
-      Container.clear();
-    }
-  };
 } // end namespace clang
 
 #endif

clang/lib/Parse/ParseDeclCXX.cpp

@@ -3339,6 +3339,7 @@ void Parser::ParseCXXMemberSpecification(SourceLocation RecordLoc,
       // Each iteration of this loop reads one member-declaration.
       ParseCXXClassMemberDeclarationWithPragmas(
           CurAS, AccessAttrs, static_cast<DeclSpec::TST>(TagType), TagDecl);
+      MaybeDestroyTemplateIds();
     }
     T.consumeClose();
   } else {

clang/lib/Parse/ParseStmt.cpp

@@ -105,6 +105,7 @@ Parser::ParseStatementOrDeclaration(StmtVector &Stmts,
 
   StmtResult Res = ParseStatementOrDeclarationAfterAttributes(
       Stmts, StmtCtx, TrailingElseLoc, Attrs);
+  MaybeDestroyTemplateIds();
 
   assert((Attrs.empty() || Res.isInvalid() || Res.isUsable()) &&
          "attributes on empty statement");

clang/lib/Parse/ParseTemplate.cpp

@@ -1618,6 +1618,9 @@ void Parser::ParseLateTemplatedFuncDef(LateParsedTemplate &LPT) {
   if (!LPT.D)
      return;
 
+  // Destroy TemplateIdAnnotations when we're done, if possible.
+  DestroyTemplateIdAnnotationsRAIIObj CleanupRAII(*this);
+
   // Get the FunctionDecl.
   FunctionDecl *FunD = LPT.D->getAsFunction();
   // Track template parameter depth.

clang/lib/Parse/Parser.cpp

@@ -433,16 +433,7 @@ Parser::~Parser() {
 
   PP.clearCodeCompletionHandler();
 
-  if (getLangOpts().DelayedTemplateParsing &&
-      !PP.isIncrementalProcessingEnabled() && !TemplateIds.empty()) {
-    // If an ASTConsumer parsed delay-parsed templates in their
-    // HandleTranslationUnit() method, TemplateIds created there were not
-    // guarded by a DestroyTemplateIdAnnotationsRAIIObj object in
-    // ParseTopLevelDecl(). Destroy them here.
-    DestroyTemplateIdAnnotationsRAIIObj CleanupRAII(TemplateIds);
-  }
-
-  assert(TemplateIds.empty() && "Still alive TemplateIdAnnotations around?");
+  DestroyTemplateIds();
 }
 
 /// Initialize - Warm up the parser.
@@ -538,11 +529,10 @@ void Parser::Initialize() {
   ConsumeToken();
 }
 
-void Parser::LateTemplateParserCleanupCallback(void *P) {
-  // While this RAII helper doesn't bracket any actual work, the destructor will
-  // clean up annotations that were created during ActOnEndOfTranslationUnit
-  // when incremental processing is enabled.
-  DestroyTemplateIdAnnotationsRAIIObj CleanupRAII(((Parser *)P)->TemplateIds);
+void Parser::DestroyTemplateIds() {
+  for (TemplateIdAnnotation *Id : TemplateIds)
+    Id->Destroy();
+  TemplateIds.clear();
 }
 
 /// Parse the first top-level declaration in a translation unit.
@@ -577,7 +567,7 @@ bool Parser::ParseFirstTopLevelDecl(DeclGroupPtrTy &Result) {
 ///           declaration
 /// [C++20]   module-import-declaration
 bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result, bool IsFirstDecl) {
-  DestroyTemplateIdAnnotationsRAIIObj CleanupRAII(TemplateIds);
+  DestroyTemplateIdAnnotationsRAIIObj CleanupRAII(*this);
 
   // Skip over the EOF token, flagging end of previous input for incremental
   // processing
@@ -663,9 +653,7 @@ bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result, bool IsFirstDecl) {
 
     // Late template parsing can begin.
     if (getLangOpts().DelayedTemplateParsing)
-      Actions.SetLateTemplateParser(LateTemplateParserCallback,
-                                    PP.isIncrementalProcessingEnabled() ?
-                                    LateTemplateParserCleanupCallback : nullptr,
+      Actions.SetLateTemplateParser(LateTemplateParserCallback, nullptr,
                                     this);
     if (!PP.isIncrementalProcessingEnabled())
       Actions.ActOnEndOfTranslationUnit();
@@ -727,7 +715,7 @@ bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result, bool IsFirstDecl) {
 Parser::DeclGroupPtrTy
 Parser::ParseExternalDeclaration(ParsedAttributesWithRange &attrs,
                                  ParsingDeclSpec *DS) {
-  DestroyTemplateIdAnnotationsRAIIObj CleanupRAII(TemplateIds);
+  DestroyTemplateIdAnnotationsRAIIObj CleanupRAII(*this);
   ParenBraceBracketBalancer BalancerRAIIObj(*this);
 
   if (PP.isCodeCompletionReached()) {

clang/test/Parser/cxx-template-decl.cpp

@@ -279,3 +279,10 @@ namespace NoCrashOnEmptyNestedNameSpecifier {
             typename T = typename ABC<FnT>::template arg_t<0>> // expected-error {{no template named 'ABC'}}
   void foo(FnT) {}
 }
+
+namespace PR45239 {
+  // Ensure we don't crash here. We used to deallocate the TemplateIdAnnotation
+  // before we'd parsed it.
+  template<int> int b;
+  template<int> auto f() -> b<0>; // expected-error +{{}}
+}