From: Malcolm Parsons Date: Thu, 17 Nov 2016 17:52:58 +0000 (+0000) Subject: Use unique_ptr for cached tokens for default arguments in C++. X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=ff0382c16190cd2b1bc47b27c5db0570b90f1d39;p=platform%2Fupstream%2Fllvm.git 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 llvm-svn: 287241 --- diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d6710f9..7976e84 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -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 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 Toks; }; /// LateParsedMethodDeclaration - A method declaration inside a class that diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h index 9c0b9ec..75f18b8 100644 --- a/clang/include/clang/Sema/DeclSpec.h +++ b/clang/include/clang/Sema/DeclSpec.h @@ -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 DefaultArgTokens; ParamInfo() = default; ParamInfo(IdentifierInfo *ident, SourceLocation iloc, Decl *param, - CachedTokens *DefArgTokens = nullptr) + std::unique_ptr 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; diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp index 30fe374..c52b61e 100644 --- a/clang/lib/Parse/ParseCXXInlineMethods.cpp +++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp @@ -319,7 +319,8 @@ void Parser::ParseLexedMethodDeclaration(LateParsedMethodDeclaration &LM) { // Introduce the parameter into scope. bool HasUnparsed = Param->hasUnparsedDefaultArg(); Actions.ActOnDelayedCXXMethodParameter(getCurScope(), Param); - if (CachedTokens *Toks = LM.DefaultArgs[I].Toks) { + std::unique_ptr 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(LateParsedMethodDeclaration &LM) { 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(LM.Method)->getPreviousDecl(); diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index d155432..9184f01 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -6022,7 +6022,7 @@ void Parser::ParseParameterDeclarationClause( // DefArgToks is used when the parsing of default arguments needs // to be delayed. - CachedTokens *DefArgToks = nullptr; + std::unique_ptr 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::ParseParameterDeclarationClause( // 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::ParseParameterDeclarationClause( ParamInfo.push_back(DeclaratorChunk::ParamInfo(ParmII, ParmDeclarator.getIdentifierLoc(), - Param, DefArgToks)); + Param, std::move(DefArgToks))); } if (TryConsumeToken(tok::ellipsis, EllipsisLoc)) { diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index f8e8e4b..7a932ab 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -2039,7 +2039,7 @@ void Parser::HandleMemberFunctionDeclDelays(Declarator& DeclaratorInfo, 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))); } } diff --git a/clang/lib/Sema/DeclSpec.cpp b/clang/lib/Sema/DeclSpec.cpp index d2bc7e6..0f88ed3 100644 --- a/clang/lib/Sema/DeclSpec.cpp +++ b/clang/lib/Sema/DeclSpec.cpp @@ -223,13 +223,19 @@ DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto, 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. diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index e96099d..06c6af1 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -395,7 +395,7 @@ void Sema::CheckExtraCXXDefaultArguments(Declarator &D) { ++argIdx) { ParmVarDecl *Param = cast(chunk.Fun.Params[argIdx].Param); if (Param->hasUnparsedDefaultArg()) { - CachedTokens *Toks = chunk.Fun.Params[argIdx].DefaultArgTokens; + std::unique_ptr 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(Declarator &D) { 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();