dtarditi created this revision.
dtarditi added a subscriber: cfe-commits.
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.
https://reviews.llvm.org/D26435
Files:
include/clang/Parse/Parser.h
include/clang/Sema/DeclSpec.h
lib/Parse/ParseCXXInlineMethods.cpp
lib/Parse/ParseDecl.cpp
lib/Parse/ParseDeclCXX.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaDeclCXX.cpp
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -395,17 +395,15 @@
++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(),
Toks->back().getLocation());
else
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();
Index: lib/Sema/DeclSpec.cpp
===================================================================
--- lib/Sema/DeclSpec.cpp
+++ lib/Sema/DeclSpec.cpp
@@ -229,7 +229,8 @@
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.
Index: lib/Parse/ParseDeclCXX.cpp
===================================================================
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -2039,7 +2039,7 @@
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)));
}
}
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -6021,7 +6021,7 @@
// 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.
@@ -6057,13 +6057,11 @@
// 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.release();
Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
} else {
Actions.ActOnParamUnparsedDefaultArgument(Param, EqualLoc,
@@ -6099,7 +6097,7 @@
ParamInfo.push_back(DeclaratorChunk::ParamInfo(ParmII,
ParmDeclarator.getIdentifierLoc(),
- Param, DefArgToks));
+ Param, std::move(DefArgToks)));
}
if (TryConsumeToken(tok::ellipsis, EllipsisLoc)) {
Index: lib/Parse/ParseCXXInlineMethods.cpp
===================================================================
--- lib/Parse/ParseCXXInlineMethods.cpp
+++ lib/Parse/ParseCXXInlineMethods.cpp
@@ -319,7 +319,8 @@
// 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 @@
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();
Index: include/clang/Sema/DeclSpec.h
===================================================================
--- include/clang/Sema/DeclSpec.h
+++ include/clang/Sema/DeclSpec.h
@@ -1188,14 +1188,14 @@
/// 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 {
@@ -1311,8 +1311,7 @@
/// 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;
+ Params[I].DefaultArgTokens.release();
}
if (DeleteParams) {
delete[] Params;
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -1017,17 +1017,17 @@
/// (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;
/// Toks - The sequence of tokens that comprises the default
/// 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
