[clang] 6163aa9 - PR45239: Don't deallocate TemplateIdAnnotations if they might still be
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 5 23:23:33 PDT 2020
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 +{{}}
+}
More information about the cfe-commits
mailing list