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.

diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 81ed4ab..61e5974 100644 (file)
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1562,6 +1562,12 @@ public:
   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) {

diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 71d216b..3f73a1b 100644 (file)
--- a/clang/include/clang/Parse/Parser.h
+++ b/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 @@ private:
   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);

diff --git a/clang/include/clang/Parse/RAIIObjectsForParser.h b/clang/include/clang/Parse/RAIIObjectsForParser.h
index fb092c0..40351bf 100644 (file)
--- a/clang/include/clang/Parse/RAIIObjectsForParser.h
+++ b/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

diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 7106323..cbff35a 100644 (file)
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/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 {

diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 492db4d..57f09e9 100644 (file)
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/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");

diff --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp
index 5025ea5..2cc0419 100644 (file)
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/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.

diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index e9dbd50..5fa23f2 100644 (file)
--- a/clang/lib/Parse/Parser.cpp
+++ b/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()) {

diff --git a/clang/test/Parser/cxx-template-decl.cpp b/clang/test/Parser/cxx-template-decl.cpp
index 14b97f1..64e7ca9 100644 (file)
--- a/clang/test/Parser/cxx-template-decl.cpp
+++ b/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 +{{}}
+}