From 6163aa96799cbad7f2f58e02c5bebee9647056a5 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Sun, 5 Apr 2020 23:10:08 -0700 Subject: [PATCH] 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 | 6 ++++ clang/include/clang/Parse/Parser.h | 17 ++++++++++- .../clang/Parse/RAIIObjectsForParser.h | 20 ------------- clang/lib/Parse/ParseDeclCXX.cpp | 1 + clang/lib/Parse/ParseStmt.cpp | 1 + clang/lib/Parse/ParseTemplate.cpp | 3 ++ clang/lib/Parse/Parser.cpp | 28 ++++++------------- clang/test/Parser/cxx-template-decl.cpp | 7 +++++ 8 files changed, 42 insertions(+), 41 deletions(-) diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h index 81ed4ab57b4119..61e5974c1f0d90 100644 --- a/clang/include/clang/Lex/Preprocessor.h +++ b/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) { diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index 71d216bfb260a0..3f73a1b90268ce 100644 --- 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 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 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); diff --git a/clang/include/clang/Parse/RAIIObjectsForParser.h b/clang/include/clang/Parse/RAIIObjectsForParser.h index fb092c0507838b..40351bf71d9f14 100644 --- 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 &Container; - - public: - DestroyTemplateIdAnnotationsRAIIObj( - SmallVectorImpl &Container) - : Container(Container) {} - - ~DestroyTemplateIdAnnotationsRAIIObj() { - for (SmallVectorImpl::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 710632379ace9f..cbff35ace337b6 100644 --- 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(TagType), TagDecl); + MaybeDestroyTemplateIds(); } T.consumeClose(); } else { diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp index 492db4d6390c05..57f09e93ebf71c 100644 --- 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 5025ea5c85febe..2cc0419582daf1 100644 --- 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 e9dbd50c3ac102..5fa23f2cdc13d6 100644 --- 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 14b97f12e3572c..64e7ca921f575b 100644 --- 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::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 b; + template auto f() -> b<0>; // expected-error +{{}} +}