hamzasood created this revision.
hamzasood added reviewers: erichkeane, v.g.vassilev, malcolm.parsons, rsmith.
Herald added a reviewer: javed.absar.
Herald added subscribers: cfe-commits, kristof.beyls, eraman.
This patch extracts the eof/stop token pattern used in late-parsing to a
re-usable RAII class. This simplifies a lot of the late-parsing code.
A few related bugs were fixed in the process:
- Late-parsing a method with default arguments but without an exception
specification would result in an invalid union access. It was mostly harmless
but it's technically undefined behaviour.
- When an inherited late-parsed parameter is encountered, the associated
declaration is force-casted to FunctionDecl. This caused a crash when the
declaration is a template.
Repository:
rC Clang
https://reviews.llvm.org/D50763
Files:
include/clang/Parse/Parser.h
include/clang/Sema/Sema.h
lib/Parse/ParseCXXInlineMethods.cpp
lib/Parse/ParseDeclCXX.cpp
lib/Sema/SemaExprCXX.cpp
test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p4.cpp
Index: test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p4.cpp
===================================================================
--- test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p4.cpp
+++ test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p4.cpp
@@ -61,15 +61,24 @@
struct A {
struct B {
static void Foo (int = 0);
+
+ template<typename T>
+ static void TFoo (T = 0);
};
// should not hide default args
friend void B::Foo (int);
+
+ // Same as above, but with templates!
+ // Should not crash the compiler.
+ template<typename T>
+ friend void B::TFoo (T);
};
void Test ()
{
A::B::Foo ();
+ A::B::TFoo<int> ();
}
} // namespace
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -1116,10 +1116,10 @@
this->Enabled = true;
}
-
-Sema::CXXThisScopeRAII::~CXXThisScopeRAII() {
+void Sema::CXXThisScopeRAII::Exit() {
if (Enabled) {
S.CXXThisTypeOverride = OldCXXThisTypeOverride;
+ Enabled = false;
}
}
Index: lib/Parse/ParseDeclCXX.cpp
===================================================================
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -2097,43 +2097,42 @@
/// delayed, e.g., default arguments or an exception-specification, create a
/// late-parsed method declaration record to handle the parsing at the end of
/// the class definition.
-void Parser::HandleMemberFunctionDeclDelays(Declarator& DeclaratorInfo,
+void Parser::HandleMemberFunctionDeclDelays(Declarator &DeclaratorInfo,
Decl *ThisDecl) {
- DeclaratorChunk::FunctionTypeInfo &FTI
- = DeclaratorInfo.getFunctionTypeInfo();
- // If there was a late-parsed exception-specification, we'll need a
- // late parse
- bool NeedLateParse = FTI.getExceptionSpecType() == EST_Unparsed;
-
- if (!NeedLateParse) {
- // Look ahead to see if there are any default args
- for (unsigned ParamIdx = 0; ParamIdx < FTI.NumParams; ++ParamIdx) {
- auto Param = cast<ParmVarDecl>(FTI.Params[ParamIdx].Param);
- if (Param->hasUnparsedDefaultArg()) {
- NeedLateParse = true;
- break;
- }
- }
- }
+ DeclaratorChunk::FunctionTypeInfo &FTI = DeclaratorInfo.getFunctionTypeInfo();
+ auto ParamInfoRange = llvm::make_range(FTI.Params,
+ FTI.Params + FTI.NumParams);
- if (NeedLateParse) {
- // Push this method onto the stack of late-parsed method
- // declarations.
- auto LateMethod = new LateParsedMethodDeclaration(this, ThisDecl);
- getCurrentClass().LateParsedDeclarations.push_back(LateMethod);
- LateMethod->TemplateScope = getCurScope()->isTemplateParamScope();
+ const bool LateParseDefaultArgs =
+ llvm::any_of(ParamInfoRange, [](const DeclaratorChunk::ParamInfo &PI)
+ { return cast<ParmVarDecl>(PI.Param)->hasUnparsedDefaultArg(); });
- // Stash the exception-specification tokens in the late-pased method.
- LateMethod->ExceptionSpecTokens = FTI.ExceptionSpecTokens;
- FTI.ExceptionSpecTokens = nullptr;
+ const bool LateParseExceptionSpec =
+ FTI.getExceptionSpecType() == EST_Unparsed;
+
+ // If we didn't delay parsing for any parts of this function, then we're done.
+ if (!LateParseDefaultArgs && !LateParseExceptionSpec)
+ return;
- // Push tokens for each parameter. Those that do not have
- // defaults will be NULL.
- LateMethod->DefaultArgs.reserve(FTI.NumParams);
- for (unsigned ParamIdx = 0; ParamIdx < FTI.NumParams; ++ParamIdx)
- LateMethod->DefaultArgs.push_back(LateParsedDefaultArgument(
- FTI.Params[ParamIdx].Param,
- std::move(FTI.Params[ParamIdx].DefaultArgTokens)));
+ // Push this method onto the stack of late-parsed method declarations.
+ auto LateMethod = new LateParsedMethodDeclaration(this, ThisDecl);
+ getCurrentClass().LateParsedDeclarations.push_back(LateMethod);
+ LateMethod->TemplateScope = getCurScope()->isTemplateParamScope();
+
+ // Stash the '= initializer-clause' tokens for each parameter. We need to do
+ // this for every parameter, even those without a default initializer, so that
+ // the late-parsed method has a reference to the parameter declarations.
+ LateMethod->DefaultArgs.reserve(FTI.NumParams);
+ llvm::transform(ParamInfoRange, std::back_inserter(LateMethod->DefaultArgs),
+ [](DeclaratorChunk::ParamInfo &PI) {
+ return LateParsedDefaultArgument(
+ PI.Param, std::move(PI.DefaultArgTokens));
+ });
+
+ if (LateParseExceptionSpec) {
+ // Stash the exception-specification tokens.
+ LateMethod->ExceptionSpecTokens.reset(FTI.ExceptionSpecTokens);
+ FTI.ExceptionSpecTokens = nullptr;
}
}
Index: lib/Parse/ParseCXXInlineMethods.cpp
===================================================================
--- lib/Parse/ParseCXXInlineMethods.cpp
+++ lib/Parse/ParseCXXInlineMethods.cpp
@@ -209,15 +209,6 @@
// Consume everything up to (but excluding) the comma or semicolon.
ConsumeAndStoreInitializer(Toks, CIK_DefaultInitializer);
}
-
- // Store an artificial EOF token to ensure that we don't run off the end of
- // the initializer when we come to parse it.
- Token Eof;
- Eof.startToken();
- Eof.setKind(tok::eof);
- Eof.setLocation(Tok.getLocation());
- Eof.setEofData(VarD);
- Toks.push_back(Eof);
}
Parser::LateParsedDeclaration::~LateParsedDeclaration() {}
@@ -256,6 +247,57 @@
Self->ParseLexedMemberInitializer(*this);
}
+namespace {
+
+/// Object that temporarily points the parser to a list of cached tokens.
+/// On construction, the current token will be the first token in the cache.
+/// On destruction, the current token will be as it was before construction.
+///
+/// \warning
+/// The input cache will be modified to allow for some extra bookkeeing.
+/// The cache shouldn't be considered usable after parsing its tokens.
+class UseCachedTokensRAII {
+ Parser &Self;
+
+ /// Some sort of pointer that uniquely identifies the contents of the cache.
+ const void *Data;
+
+public:
+ UseCachedTokensRAII(Parser &Self, CachedTokens &Cache, const void *Data)
+ : Self(Self), Data(Data) {
+ // Mark the end of the token stream so that we know when to stop.
+ Token EndTok;
+ EndTok.startToken();
+ EndTok.setKind(tok::eof);
+ EndTok.setLocation(Cache.back().getEndLoc());
+ EndTok.setEofData(Data);
+ Cache.push_back(EndTok);
+
+ // Enter the token stream.
+ Cache.push_back(Self.getCurToken()); // Save the current token.
+ Self.getPreprocessor().EnterTokenStream(Cache, true);
+ Self.ConsumeAnyToken(true); // Consume the saved token.
+ }
+
+ bool isAtEnd() const {
+ const Token Tok = Self.getCurToken();
+ return Tok.is(tok::eof) && Tok.getEofData() == Data;
+ }
+
+ ~UseCachedTokensRAII() {
+ // There could be leftover tokens (e.g. because of an error).
+ // Skip through until we reach the original token position.
+ while (Self.getCurToken().isNot(tok::eof))
+ Self.ConsumeAnyToken();
+
+ // Clean up the remaining stop token.
+ if (isAtEnd())
+ Self.ConsumeAnyToken();
+ }
+};
+
+} // end unnamed namespace
+
/// ParseLexedMethodDeclarations - We finished parsing the member
/// specification of a top (non-nested) C++ class. Now go over the
/// stack of method declarations with some parts for which parsing was
@@ -296,128 +338,100 @@
Actions.ActOnReenterTemplateScope(getCurScope(), LM.Method);
++CurTemplateDepthTracker;
}
+
// Start the delayed C++ method declaration
Actions.ActOnStartDelayedCXXMethodDeclaration(getCurScope(), LM.Method);
- // Introduce the parameters into scope and parse their default
- // arguments.
ParseScope PrototypeScope(this, Scope::FunctionPrototypeScope |
- Scope::FunctionDeclarationScope | Scope::DeclScope);
+ Scope::FunctionDeclarationScope |
+ Scope::DeclScope);
+
+ // Get the underlying function declaration, looking past templating.
+ auto *Func = cast<FunctionDecl>(
+ isa<FunctionTemplateDecl>(LM.Method)
+ ? cast<FunctionTemplateDecl>(LM.Method)->getTemplatedDecl()
+ : LM.Method);
+
+ // Introduce the parameters into scope and parse their default arguments.
for (unsigned I = 0, N = LM.DefaultArgs.size(); I != N; ++I) {
- auto Param = cast<ParmVarDecl>(LM.DefaultArgs[I].Param);
- // Introduce the parameter into scope.
- bool HasUnparsed = Param->hasUnparsedDefaultArg();
- Actions.ActOnDelayedCXXMethodParameter(getCurScope(), Param);
- std::unique_ptr<CachedTokens> Toks = std::move(LM.DefaultArgs[I].Toks);
- if (Toks) {
- ParenBraceBracketBalancer BalancerRAIIObj(*this);
+ LateParsedDefaultArgument &DefaultArg = LM.DefaultArgs[I];
- // Mark the end of the default argument so that we know when to stop when
- // we parse it later on.
- Token LastDefaultArgToken = Toks->back();
- Token DefArgEnd;
- DefArgEnd.startToken();
- DefArgEnd.setKind(tok::eof);
- DefArgEnd.setLocation(LastDefaultArgToken.getEndLoc());
- DefArgEnd.setEofData(Param);
- Toks->push_back(DefArgEnd);
+ auto *Param = cast<ParmVarDecl>(DefaultArg.Param);
+ const bool HasUnparsed = Param->hasUnparsedDefaultArg();
- // Parse the default argument from its saved token stream.
- Toks->push_back(Tok); // So that the current token doesn't get lost
- PP.EnterTokenStream(*Toks, true);
+ // Introduce the parameter into scope.
+ Actions.ActOnDelayedCXXMethodParameter(getCurScope(), Param);
- // Consume the previously-pushed token.
- ConsumeAnyToken();
+ // If it has a default argument, parse it now.
+ if (std::unique_ptr<CachedTokens> Toks = std::move(DefaultArg.Toks)) {
+ ParenBraceBracketBalancer BalancerRAIIObj(*this);
+ UseCachedTokensRAII CachedTokStream(*this, *Toks, Param);
// Consume the '='.
assert(Tok.is(tok::equal) && "Default argument not starting with '='");
SourceLocation EqualLoc = ConsumeToken();
- // The argument isn't actually potentially evaluated unless it is
- // used.
+ // The argument isn't actually potentially evaluated unless it's used.
EnterExpressionEvaluationContext Eval(
Actions,
Sema::ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed, Param);
+ // Parse the initializer-clause.
ExprResult DefArgResult;
if (getLangOpts().CPlusPlus11 && Tok.is(tok::l_brace)) {
Diag(Tok, diag::warn_cxx98_compat_generalized_initializer_lists);
DefArgResult = ParseBraceInitializer();
- } else
+ } else {
DefArgResult = ParseAssignmentExpression();
+ }
+
DefArgResult = Actions.CorrectDelayedTyposInExpr(DefArgResult);
if (DefArgResult.isInvalid()) {
Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
} else {
- if (Tok.isNot(tok::eof) || Tok.getEofData() != Param) {
- // The last two tokens are the terminator and the saved value of
- // Tok; the last token in the default argument is the one before
- // those.
- assert(Toks->size() >= 3 && "expected a token in default arg");
- Diag(Tok.getLocation(), diag::err_default_arg_unparsed)
- << SourceRange(Tok.getLocation(),
- (*Toks)[Toks->size() - 3].getLocation());
- }
+ if (!CachedTokStream.isAtEnd())
+ Diag(Tok.getLocation(), diag::err_default_arg_unparsed);
+
Actions.ActOnParamDefaultArgument(Param, EqualLoc,
DefArgResult.get());
}
+ } else if (HasUnparsed) {
+ assert(Param->hasInheritedDefaultArg() &&
+ "We should've cached tokens for this default arg");
- // There could be leftover tokens (e.g. because of an error).
- // Skip through until we reach the 'end of default argument' token.
- while (Tok.isNot(tok::eof))
- ConsumeAnyToken();
+ ParmVarDecl *OldParam = Func->getPreviousDecl()->getParamDecl(I);
+ assert (!OldParam->hasUnparsedDefaultArg() &&
+ "Previous declaration should be parsed by now");
- if (Tok.is(tok::eof) && Tok.getEofData() == Param)
- ConsumeAnyToken();
- } else if (HasUnparsed) {
- assert(Param->hasInheritedDefaultArg());
- FunctionDecl *Old = cast<FunctionDecl>(LM.Method)->getPreviousDecl();
- ParmVarDecl *OldParam = Old->getParamDecl(I);
- assert (!OldParam->hasUnparsedDefaultArg());
if (OldParam->hasUninstantiatedDefaultArg())
Param->setUninstantiatedDefaultArg(
OldParam->getUninstantiatedDefaultArg());
else
Param->setDefaultArg(OldParam->getInit());
}
}
+ // C++11 [expr.prim.general]p3:
+ // If a declaration declares a member function or member function
+ // template of a class X, the expression this is a prvalue of type
+ // "pointer to cv-qualifier-seq X" between the optional cv-qualifer-seq
+ // and the end of the function-definition, member-declarator, or
+ // declarator.
+ Sema::CXXThisScopeRAII ThisScope = [&] {
+ // We only need to enter a 'this' scope if we're parsing a member function.
+ if (auto *Method = dyn_cast<CXXMethodDecl>(Func)) {
+ return Sema::CXXThisScopeRAII(Actions, Method->getParent(),
+ Method->getTypeQualifiers(),
+ getLangOpts().CPlusPlus11);
+ }
+
+ return Sema::CXXThisScopeRAII(Actions, nullptr, 0, /* Enabled */false);
+ }();
+
// Parse a delayed exception-specification, if there is one.
- if (CachedTokens *Toks = LM.ExceptionSpecTokens) {
+ if (std::unique_ptr<CachedTokens> Toks = std::move(LM.ExceptionSpecTokens)) {
ParenBraceBracketBalancer BalancerRAIIObj(*this);
-
- // Add the 'stop' token.
- Token LastExceptionSpecToken = Toks->back();
- Token ExceptionSpecEnd;
- ExceptionSpecEnd.startToken();
- ExceptionSpecEnd.setKind(tok::eof);
- ExceptionSpecEnd.setLocation(LastExceptionSpecToken.getEndLoc());
- ExceptionSpecEnd.setEofData(LM.Method);
- Toks->push_back(ExceptionSpecEnd);
-
- // Parse the default argument from its saved token stream.
- Toks->push_back(Tok); // So that the current token doesn't get lost
- PP.EnterTokenStream(*Toks, true);
-
- // Consume the previously-pushed token.
- ConsumeAnyToken();
-
- // C++11 [expr.prim.general]p3:
- // If a declaration declares a member function or member function
- // template of a class X, the expression this is a prvalue of type
- // "pointer to cv-qualifier-seq X" between the optional cv-qualifer-seq
- // and the end of the function-definition, member-declarator, or
- // declarator.
- CXXMethodDecl *Method;
- if (FunctionTemplateDecl *FunTmpl
- = dyn_cast<FunctionTemplateDecl>(LM.Method))
- Method = cast<CXXMethodDecl>(FunTmpl->getTemplatedDecl());
- else
- Method = cast<CXXMethodDecl>(LM.Method);
-
- Sema::CXXThisScopeRAII ThisScope(Actions, Method->getParent(),
- Method->getTypeQualifiers(),
- getLangOpts().CPlusPlus11);
+ UseCachedTokensRAII CachedTokStream(*this, *Toks, LM.Method);
// Parse the exception-specification.
SourceRange SpecificationRange;
@@ -432,7 +446,7 @@
DynamicExceptionRanges, NoexceptExpr,
ExceptionSpecTokens);
- if (Tok.isNot(tok::eof) || Tok.getEofData() != LM.Method)
+ if (!CachedTokStream.isAtEnd())
Diag(Tok.getLocation(), diag::err_except_spec_unparsed);
// Attach the exception-specification to the method.
@@ -442,20 +456,9 @@
DynamicExceptionRanges,
NoexceptExpr.isUsable()?
NoexceptExpr.get() : nullptr);
-
- // There could be leftover tokens (e.g. because of an error).
- // Skip through until we reach the original token position.
- while (Tok.isNot(tok::eof))
- ConsumeAnyToken();
-
- // Clean up the remaining EOF token.
- if (Tok.is(tok::eof) && Tok.getEofData() == LM.Method)
- ConsumeAnyToken();
-
- delete Toks;
- LM.ExceptionSpecTokens = nullptr;
}
+ ThisScope.Exit();
PrototypeScope.Exit();
// Finish the delayed C++ method declaration.
@@ -492,22 +495,8 @@
}
ParenBraceBracketBalancer BalancerRAIIObj(*this);
+ UseCachedTokensRAII CachedTokStream(*this, LM.Toks, LM.D);
- assert(!LM.Toks.empty() && "Empty body!");
- Token LastBodyToken = LM.Toks.back();
- Token BodyEnd;
- BodyEnd.startToken();
- BodyEnd.setKind(tok::eof);
- BodyEnd.setLocation(LastBodyToken.getEndLoc());
- BodyEnd.setEofData(LM.D);
- LM.Toks.push_back(BodyEnd);
- // Append the current token at the end of the new token stream so that it
- // doesn't get lost.
- LM.Toks.push_back(Tok);
- PP.EnterTokenStream(LM.Toks, true);
-
- // Consume the previously pushed token.
- ConsumeAnyToken(/*ConsumeCodeCompletionTok=*/true);
assert(Tok.isOneOf(tok::l_brace, tok::colon, tok::kw_try)
&& "Inline method not starting with '{', ':' or 'try'");
@@ -519,12 +508,6 @@
if (Tok.is(tok::kw_try)) {
ParseFunctionTryBlock(LM.D, FnScope);
-
- while (Tok.isNot(tok::eof))
- ConsumeAnyToken();
-
- if (Tok.is(tok::eof) && Tok.getEofData() == LM.D)
- ConsumeAnyToken();
return;
}
if (Tok.is(tok::colon)) {
@@ -534,12 +517,6 @@
if (!Tok.is(tok::l_brace)) {
FnScope.Exit();
Actions.ActOnFinishFunctionBody(LM.D, nullptr);
-
- while (Tok.isNot(tok::eof))
- ConsumeAnyToken();
-
- if (Tok.is(tok::eof) && Tok.getEofData() == LM.D)
- ConsumeAnyToken();
return;
}
} else
@@ -554,12 +531,6 @@
ParseFunctionStatementBody(LM.D, FnScope);
- while (Tok.isNot(tok::eof))
- ConsumeAnyToken();
-
- if (Tok.is(tok::eof) && Tok.getEofData() == LM.D)
- ConsumeAnyToken();
-
if (auto *FD = dyn_cast_or_null<FunctionDecl>(LM.D))
if (isa<CXXMethodDecl>(FD) ||
FD->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend))
@@ -614,14 +585,7 @@
return;
ParenBraceBracketBalancer BalancerRAIIObj(*this);
-
- // Append the current token at the end of the new token stream so that it
- // doesn't get lost.
- MI.Toks.push_back(Tok);
- PP.EnterTokenStream(MI.Toks, true);
-
- // Consume the previously pushed token.
- ConsumeAnyToken(/*ConsumeCodeCompletionTok=*/true);
+ UseCachedTokensRAII CachedTokStream(*this, MI.Toks, MI.Field);
SourceLocation EqualLoc;
@@ -633,23 +597,14 @@
Actions.ActOnFinishCXXInClassMemberInitializer(MI.Field, EqualLoc,
Init.get());
- // The next token should be our artificial terminating EOF token.
- if (Tok.isNot(tok::eof)) {
- if (!Init.isInvalid()) {
- SourceLocation EndLoc = PP.getLocForEndOfToken(PrevTokLocation);
- if (!EndLoc.isValid())
- EndLoc = Tok.getLocation();
- // No fixit; we can't recover as if there were a semicolon here.
- Diag(EndLoc, diag::err_expected_semi_decl_list);
- }
+ if (!CachedTokStream.isAtEnd() && !Init.isInvalid()) {
+ SourceLocation EndLoc = PP.getLocForEndOfToken(PrevTokLocation);
+ if (!EndLoc.isValid())
+ EndLoc = Tok.getLocation();
- // Consume tokens until we hit the artificial EOF.
- while (Tok.isNot(tok::eof))
- ConsumeAnyToken();
+ // No fixit; we can't recover as if there were a semicolon here.
+ Diag(EndLoc, diag::err_expected_semi_decl_list);
}
- // Make sure this is *our* artificial EOF token.
- if (Tok.getEofData() == MI.Field)
- ConsumeAnyToken();
}
/// ConsumeAndStoreUntil - Consume and store the token at the passed token
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -5103,7 +5103,17 @@
CXXThisScopeRAII(Sema &S, Decl *ContextDecl, unsigned CXXThisTypeQuals,
bool Enabled = true);
- ~CXXThisScopeRAII();
+ CXXThisScopeRAII(CXXThisScopeRAII &&Other)
+ : S(Other.S), OldCXXThisTypeOverride(Other.OldCXXThisTypeOverride),
+ Enabled(Other.Enabled) {
+ Other.Enabled = false;
+ }
+ void operator=(CXXThisScopeRAII &&) = delete;
+
+ /// Exit the scope early before the destructor is called.
+ void Exit();
+
+ ~CXXThisScopeRAII() { Exit(); }
};
/// Make sure the value of 'this' is actually available in the current
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -1181,8 +1181,7 @@
/// argument (C++ [class.mem]p2).
struct LateParsedMethodDeclaration : public LateParsedDeclaration {
explicit LateParsedMethodDeclaration(Parser *P, Decl *M)
- : Self(P), Method(M), TemplateScope(false),
- ExceptionSpecTokens(nullptr) {}
+ : Self(P), Method(M), TemplateScope(false) {}
void ParseLexedMethodDeclarations() override;
@@ -1205,7 +1204,7 @@
/// The set of tokens that make up an exception-specification that
/// has not yet been parsed.
- CachedTokens *ExceptionSpecTokens;
+ std::unique_ptr<CachedTokens> ExceptionSpecTokens;
};
/// LateParsedMemberInitializer - An initializer for a non-static class data
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
