r287241 - Use unique_ptr for cached tokens for default arguments in C++.
Malcolm Parsons via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 17 09:52:59 PST 2016
Author: malcolm.parsons
Date: Thu Nov 17 11:52:58 2016
New Revision: 287241
URL: http://llvm.org/viewvc/llvm-project?rev=287241&view=rev
Log:
Use unique_ptr for cached tokens for default arguments in C++.
Summary:
This changes pointers to cached tokens for default arguments in C++ from raw pointers to unique_ptrs. There was a fixme in the code where the cached tokens are created about using a smart pointer.
The change is straightforward, though I did have to track down and fix a memory corruption caused by the change. memcpy was being used to copy parameter information. This duplicated the unique_ptr, which led to the cached token buffer being deleted prematurely.
Patch by David Tarditi!
Reviewers: malcolm.parsons
Subscribers: arphaman, malcolm.parsons, cfe-commits
Differential Revision: https://reviews.llvm.org/D26435
Modified:
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/DeclSpec.h
cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Sema/DeclSpec.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=287241&r1=287240&r2=287241&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Thu Nov 17 11:52:58 2016
@@ -1017,8 +1017,8 @@ private:
/// (C++ [class.mem]p2).
struct LateParsedDefaultArgument {
explicit LateParsedDefaultArgument(Decl *P,
- CachedTokens *Toks = nullptr)
- : Param(P), Toks(Toks) { }
+ std::unique_ptr<CachedTokens> Toks = nullptr)
+ : Param(P), Toks(std::move(Toks)) { }
/// Param - The parameter declaration for this parameter.
Decl *Param;
@@ -1027,7 +1027,7 @@ private:
/// argument expression, not including the '=' or the terminating
/// ')' or ','. This will be NULL for parameters that have no
/// default argument.
- CachedTokens *Toks;
+ std::unique_ptr<CachedTokens> Toks;
};
/// LateParsedMethodDeclaration - A method declaration inside a class that
Modified: cfe/trunk/include/clang/Sema/DeclSpec.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/DeclSpec.h?rev=287241&r1=287240&r2=287241&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/DeclSpec.h (original)
+++ cfe/trunk/include/clang/Sema/DeclSpec.h Thu Nov 17 11:52:58 2016
@@ -1188,14 +1188,14 @@ struct DeclaratorChunk {
/// declaration of a member function), it will be stored here as a
/// sequence of tokens to be parsed once the class definition is
/// complete. Non-NULL indicates that there is a default argument.
- CachedTokens *DefaultArgTokens;
+ std::unique_ptr<CachedTokens> DefaultArgTokens;
ParamInfo() = default;
ParamInfo(IdentifierInfo *ident, SourceLocation iloc,
Decl *param,
- CachedTokens *DefArgTokens = nullptr)
+ std::unique_ptr<CachedTokens> DefArgTokens = nullptr)
: Ident(ident), IdentLoc(iloc), Param(param),
- DefaultArgTokens(DefArgTokens) {}
+ DefaultArgTokens(std::move(DefArgTokens)) {}
};
struct TypeAndRange {
@@ -1310,10 +1310,8 @@ struct DeclaratorChunk {
///
/// This is used in various places for error recovery.
void freeParams() {
- for (unsigned I = 0; I < NumParams; ++I) {
- delete Params[I].DefaultArgTokens;
- Params[I].DefaultArgTokens = nullptr;
- }
+ for (unsigned I = 0; I < NumParams; ++I)
+ Params[I].DefaultArgTokens.reset();
if (DeleteParams) {
delete[] Params;
DeleteParams = false;
Modified: cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp?rev=287241&r1=287240&r2=287241&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp (original)
+++ cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp Thu Nov 17 11:52:58 2016
@@ -319,7 +319,8 @@ void Parser::ParseLexedMethodDeclaration
// Introduce the parameter into scope.
bool HasUnparsed = Param->hasUnparsedDefaultArg();
Actions.ActOnDelayedCXXMethodParameter(getCurScope(), Param);
- if (CachedTokens *Toks = LM.DefaultArgs[I].Toks) {
+ std::unique_ptr<CachedTokens> Toks = std::move(LM.DefaultArgs[I].Toks);
+ if (Toks) {
// Mark the end of the default argument so that we know when to stop when
// we parse it later on.
Token LastDefaultArgToken = Toks->back();
@@ -377,9 +378,6 @@ void Parser::ParseLexedMethodDeclaration
if (Tok.is(tok::eof) && Tok.getEofData() == Param)
ConsumeAnyToken();
-
- delete Toks;
- LM.DefaultArgs[I].Toks = nullptr;
} else if (HasUnparsed) {
assert(Param->hasInheritedDefaultArg());
FunctionDecl *Old = cast<FunctionDecl>(LM.Method)->getPreviousDecl();
Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=287241&r1=287240&r2=287241&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Thu Nov 17 11:52:58 2016
@@ -6022,7 +6022,7 @@ void Parser::ParseParameterDeclarationCl
// DefArgToks is used when the parsing of default arguments needs
// to be delayed.
- CachedTokens *DefArgToks = nullptr;
+ std::unique_ptr<CachedTokens> DefArgToks;
// If no parameter was specified, verify that *something* was specified,
// otherwise we have a missing type and identifier.
@@ -6058,13 +6058,11 @@ void Parser::ParseParameterDeclarationCl
// If we're inside a class definition, cache the tokens
// corresponding to the default argument. We'll actually parse
// them when we see the end of the class definition.
- // FIXME: Can we use a smart pointer for Toks?
- DefArgToks = new CachedTokens;
+ DefArgToks.reset(new CachedTokens);
SourceLocation ArgStartLoc = NextToken().getLocation();
if (!ConsumeAndStoreInitializer(*DefArgToks, CIK_DefaultArgument)) {
- delete DefArgToks;
- DefArgToks = nullptr;
+ DefArgToks.reset();
Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
} else {
Actions.ActOnParamUnparsedDefaultArgument(Param, EqualLoc,
@@ -6100,7 +6098,7 @@ void Parser::ParseParameterDeclarationCl
ParamInfo.push_back(DeclaratorChunk::ParamInfo(ParmII,
ParmDeclarator.getIdentifierLoc(),
- Param, DefArgToks));
+ Param, std::move(DefArgToks)));
}
if (TryConsumeToken(tok::ellipsis, EllipsisLoc)) {
Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=287241&r1=287240&r2=287241&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Thu Nov 17 11:52:58 2016
@@ -2039,7 +2039,7 @@ void Parser::HandleMemberFunctionDeclDel
LateMethod->DefaultArgs.reserve(FTI.NumParams);
for (unsigned ParamIdx = 0; ParamIdx < FTI.NumParams; ++ParamIdx)
LateMethod->DefaultArgs.push_back(LateParsedDefaultArgument(
- FTI.Params[ParamIdx].Param, FTI.Params[ParamIdx].DefaultArgTokens));
+ FTI.Params[ParamIdx].Param, std::move(FTI.Params[ParamIdx].DefaultArgTokens)));
}
}
Modified: cfe/trunk/lib/Sema/DeclSpec.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/DeclSpec.cpp?rev=287241&r1=287240&r2=287241&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/DeclSpec.cpp (original)
+++ cfe/trunk/lib/Sema/DeclSpec.cpp Thu Nov 17 11:52:58 2016
@@ -223,13 +223,19 @@ DeclaratorChunk DeclaratorChunk::getFunc
if (!TheDeclarator.InlineStorageUsed &&
NumParams <= llvm::array_lengthof(TheDeclarator.InlineParams)) {
I.Fun.Params = TheDeclarator.InlineParams;
+ // Zero the memory block so that unique pointers are initialized
+ // properly.
+ memset(I.Fun.Params, 0, sizeof(Params[0]) * NumParams);
I.Fun.DeleteParams = false;
TheDeclarator.InlineStorageUsed = true;
} else {
- I.Fun.Params = new DeclaratorChunk::ParamInfo[NumParams];
+ // Call the version of new that zeroes memory so that unique pointers
+ // are initialized properly.
+ I.Fun.Params = new DeclaratorChunk::ParamInfo[NumParams]();
I.Fun.DeleteParams = true;
}
- memcpy(I.Fun.Params, Params, sizeof(Params[0]) * NumParams);
+ for (unsigned i = 0; i < NumParams; i++)
+ I.Fun.Params[i] = std::move(Params[i]);
}
// Check what exception specification information we should actually store.
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=287241&r1=287240&r2=287241&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Nov 17 11:52:58 2016
@@ -395,7 +395,7 @@ void Sema::CheckExtraCXXDefaultArguments
++argIdx) {
ParmVarDecl *Param = cast<ParmVarDecl>(chunk.Fun.Params[argIdx].Param);
if (Param->hasUnparsedDefaultArg()) {
- CachedTokens *Toks = chunk.Fun.Params[argIdx].DefaultArgTokens;
+ std::unique_ptr<CachedTokens> Toks = std::move(chunk.Fun.Params[argIdx].DefaultArgTokens);
SourceRange SR;
if (Toks->size() > 1)
SR = SourceRange((*Toks)[1].getLocation(),
@@ -404,8 +404,6 @@ void Sema::CheckExtraCXXDefaultArguments
SR = UnparsedDefaultArgLocs[Param];
Diag(Param->getLocation(), diag::err_param_default_argument_nonfunc)
<< SR;
- delete Toks;
- chunk.Fun.Params[argIdx].DefaultArgTokens = nullptr;
} else if (Param->getDefaultArg()) {
Diag(Param->getLocation(), diag::err_param_default_argument_nonfunc)
<< Param->getDefaultArg()->getSourceRange();
More information about the cfe-commits
mailing list