Author: Richard Smith Date: 2020-04-05T23:23:20-07:00 New Revision: 6163aa96799cbad7f2f58e02c5bebee9647056a5
URL: https://github.com/llvm/llvm-project/commit/6163aa96799cbad7f2f58e02c5bebee9647056a5 DIFF: https://github.com/llvm/llvm-project/commit/6163aa96799cbad7f2f58e02c5bebee9647056a5.diff LOG: 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. Added: Modified: clang/include/clang/Lex/Preprocessor.h clang/include/clang/Parse/Parser.h clang/include/clang/Parse/RAIIObjectsForParser.h clang/lib/Parse/ParseDeclCXX.cpp clang/lib/Parse/ParseStmt.cpp clang/lib/Parse/ParseTemplate.cpp clang/lib/Parse/Parser.cpp clang/test/Parser/cxx-template-decl.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h index 81ed4ab57b41..61e5974c1f0d 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 71d216bfb260..3f73a1b90268 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<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); diff --git a/clang/include/clang/Parse/RAIIObjectsForParser.h b/clang/include/clang/Parse/RAIIObjectsForParser.h index fb092c050783..40351bf71d9f 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<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 710632379ace..cbff35ace337 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<DeclSpec::TST>(TagType), TagDecl); + MaybeDestroyTemplateIds(); } T.consumeClose(); } else { diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp index 492db4d6390c..57f09e93ebf7 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 5025ea5c85fe..2cc0419582da 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 e9dbd50c3ac1..5fa23f2cdc13 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 14b97f12e357..64e7ca921f57 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<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 +{{}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
