From: Ted Kremenek Date: Wed, 10 Nov 2010 05:59:39 +0000 (+0000) Subject: Region-allocate all AttributeList objects from a factory object instead of manually... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8113ecfa4e41e2c888b1794389dfe3bce6386493;p=clang Region-allocate all AttributeList objects from a factory object instead of manually managing them using new/delete and OwningPtrs. After memory profiling Clang, I witnessed periodic leaks of these objects; digging deeper into the code, it was clear that our management of these objects was a mess. The ownership rules were murky at best, and not always followed. Worse, there are plenty of error paths where we could screw up. This patch introduces AttributeList::Factory, which is a factory class that creates AttributeList objects and then blows them away all at once. While conceptually simple, most of the changes in this patch just have to do with migrating over to the new interface. Most of the changes have resulted in some nice simplifications. This new strategy currently holds on to all AttributeList objects during the lifetime of the Parser object. This is easily tunable. If we desire to have more bound the lifetime of AttributeList objects more precisely, we can have the AttributeList::Factory object (in Parser) push/pop its underlying allocator as we enter/leave key methods in the Parser. This means that we get simple memory management while still having the ability to finely control memory use if necessary. Note that because AttributeList objects are now BumpPtrAllocated, we may reduce malloc() traffic in many large files with attributes. This fixes the leak reported in: git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@118675 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index f72cf63d47..f8ca18afa8 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -25,8 +25,6 @@ #include namespace clang { - class AttributeList; - struct CXX0XAttributeList; class PragmaHandler; class Scope; class DeclGroupRef; @@ -142,6 +140,9 @@ class Parser : public CodeCompletionHandler { /// The "depth" of the template parameters currently being parsed. unsigned TemplateParameterDepth; + + /// Factory object for creating AttributeList objects. + AttributeList::Factory AttrFactory; public: Parser(Preprocessor &PP, Sema &Actions); diff --git a/include/clang/Sema/AttributeList.h b/include/clang/Sema/AttributeList.h index 5cdfb9484e..1a4f38204b 100644 --- a/include/clang/Sema/AttributeList.h +++ b/include/clang/Sema/AttributeList.h @@ -15,6 +15,7 @@ #ifndef LLVM_CLANG_SEMA_ATTRLIST_H #define LLVM_CLANG_SEMA_ATTRLIST_H +#include "llvm/Support/Allocator.h" #include "clang/Sema/Ownership.h" #include "clang/Basic/SourceLocation.h" #include @@ -32,6 +33,9 @@ namespace clang { /// 4: __attribute__(( aligned(16) )). ParmName is unused, Args/Num used. /// class AttributeList { +public: + class Factory; +private: IdentifierInfo *AttrName; SourceLocation AttrLoc; IdentifierInfo *ScopeName; @@ -45,14 +49,33 @@ class AttributeList { mutable bool Invalid; /// True if already diagnosed as invalid. AttributeList(const AttributeList &); // DO NOT IMPLEMENT void operator=(const AttributeList &); // DO NOT IMPLEMENT -public: - AttributeList(IdentifierInfo *AttrName, SourceLocation AttrLoc, + void operator delete(void *); // DO NOT IMPLEMENT + ~AttributeList(); // DO NOT IMPLEMENT + AttributeList(llvm::BumpPtrAllocator &Alloc, + IdentifierInfo *AttrName, SourceLocation AttrLoc, IdentifierInfo *ScopeName, SourceLocation ScopeLoc, IdentifierInfo *ParmName, SourceLocation ParmLoc, Expr **args, unsigned numargs, - AttributeList *Next, bool declspec = false, bool cxx0x = false); - ~AttributeList(); - + AttributeList *Next, bool declspec, bool cxx0x); +public: + class Factory { + llvm::BumpPtrAllocator Alloc; + public: + Factory() {} + ~Factory() {} + AttributeList *Create(IdentifierInfo *AttrName, SourceLocation AttrLoc, + IdentifierInfo *ScopeName, SourceLocation ScopeLoc, + IdentifierInfo *ParmName, SourceLocation ParmLoc, + Expr **args, unsigned numargs, + AttributeList *Next, bool declspec = false, bool cxx0x = false) { + AttributeList *Mem = Alloc.Allocate(); + new (Mem) AttributeList(Alloc, AttrName, AttrLoc, ScopeName, ScopeLoc, + ParmName, ParmLoc, args, numargs, + Next, declspec, cxx0x); + return Mem; + } + }; + enum Kind { // Please keep this list alphabetized. AT_IBAction, // Clang-specific. AT_IBOutlet, // Clang-specific. diff --git a/include/clang/Sema/DeclSpec.h b/include/clang/Sema/DeclSpec.h index 0957e77557..5c043cd261 100644 --- a/include/clang/Sema/DeclSpec.h +++ b/include/clang/Sema/DeclSpec.h @@ -274,7 +274,6 @@ public: writtenBS() { } ~DeclSpec() { - delete AttrList; delete [] ProtocolQualifiers; delete [] ProtocolLocs; } @@ -813,7 +812,6 @@ struct DeclaratorChunk { unsigned TypeQuals : 3; AttributeList *AttrList; void destroy() { - delete AttrList; } }; @@ -824,7 +822,6 @@ struct DeclaratorChunk { bool LValueRef : 1; AttributeList *AttrList; void destroy() { - delete AttrList; } }; @@ -967,7 +964,6 @@ struct DeclaratorChunk { unsigned TypeQuals : 3; AttributeList *AttrList; void destroy() { - delete AttrList; } }; @@ -988,7 +984,6 @@ struct DeclaratorChunk { return *reinterpret_cast(ScopeMem.Mem); } void destroy() { - delete AttrList; Scope().~CXXScopeSpec(); } }; @@ -1244,7 +1239,6 @@ public: for (unsigned i = 0, e = DeclTypeInfo.size(); i != e; ++i) DeclTypeInfo[i].destroy(); DeclTypeInfo.clear(); - delete AttrList; AttrList = 0; AsmLabel = 0; InlineParamsUsed = false; diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 2ba47641a0..00788b0673 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -122,8 +122,8 @@ AttributeList *Parser::ParseGNUAttributes(SourceLocation *EndLoc) { if (Tok.is(tok::r_paren)) { // __attribute__(( mode(byte) )) ConsumeParen(); // ignore the right paren loc for now - CurrAttr = new AttributeList(AttrName, AttrNameLoc, 0, AttrNameLoc, - ParmName, ParmLoc, 0, 0, CurrAttr); + CurrAttr = AttrFactory.Create(AttrName, AttrNameLoc, 0, AttrNameLoc, + ParmName, ParmLoc, 0, 0, CurrAttr); } else if (Tok.is(tok::comma)) { ConsumeToken(); // __attribute__(( format(printf, 1, 2) )) @@ -146,10 +146,10 @@ AttributeList *Parser::ParseGNUAttributes(SourceLocation *EndLoc) { } if (ArgExprsOk && Tok.is(tok::r_paren)) { ConsumeParen(); // ignore the right paren loc for now - CurrAttr = new AttributeList(AttrName, AttrNameLoc, 0, - AttrNameLoc, ParmName, ParmLoc, - ArgExprs.take(), ArgExprs.size(), - CurrAttr); + CurrAttr = AttrFactory.Create(AttrName, AttrNameLoc, 0, + AttrNameLoc, ParmName, ParmLoc, + ArgExprs.take(), ArgExprs.size(), + CurrAttr); } } } else { // not an identifier @@ -158,8 +158,8 @@ AttributeList *Parser::ParseGNUAttributes(SourceLocation *EndLoc) { // parse a possibly empty comma separated list of expressions // __attribute__(( nonnull() )) ConsumeParen(); // ignore the right paren loc for now - CurrAttr = new AttributeList(AttrName, AttrNameLoc, 0, AttrNameLoc, - 0, SourceLocation(), 0, 0, CurrAttr); + CurrAttr = AttrFactory.Create(AttrName, AttrNameLoc, 0, AttrNameLoc, + 0, SourceLocation(), 0, 0, CurrAttr); break; case tok::kw_char: case tok::kw_wchar_t: @@ -175,8 +175,8 @@ AttributeList *Parser::ParseGNUAttributes(SourceLocation *EndLoc) { case tok::kw_double: case tok::kw_void: case tok::kw_typeof: - CurrAttr = new AttributeList(AttrName, AttrNameLoc, 0, AttrNameLoc, - 0, SourceLocation(), 0, 0, CurrAttr); + CurrAttr = AttrFactory.Create(AttrName, AttrNameLoc, 0, AttrNameLoc, + 0, SourceLocation(), 0, 0, CurrAttr); if (CurrAttr->getKind() == AttributeList::AT_IBOutletCollection) Diag(Tok, diag::err_iboutletcollection_builtintype); // If it's a builtin type name, eat it and expect a rparen @@ -207,7 +207,7 @@ AttributeList *Parser::ParseGNUAttributes(SourceLocation *EndLoc) { // Match the ')'. if (ArgExprsOk && Tok.is(tok::r_paren)) { ConsumeParen(); // ignore the right paren loc for now - CurrAttr = new AttributeList(AttrName, AttrNameLoc, 0, + CurrAttr = AttrFactory.Create(AttrName, AttrNameLoc, 0, AttrNameLoc, 0, SourceLocation(), ArgExprs.take(), ArgExprs.size(), CurrAttr); @@ -216,8 +216,8 @@ AttributeList *Parser::ParseGNUAttributes(SourceLocation *EndLoc) { } } } else { - CurrAttr = new AttributeList(AttrName, AttrNameLoc, 0, AttrNameLoc, - 0, SourceLocation(), 0, 0, CurrAttr); + CurrAttr = AttrFactory.Create(AttrName, AttrNameLoc, 0, AttrNameLoc, + 0, SourceLocation(), 0, 0, CurrAttr); } } if (ExpectAndConsume(tok::r_paren, diag::err_expected_rparen)) @@ -260,15 +260,15 @@ AttributeList* Parser::ParseMicrosoftDeclSpec(AttributeList *CurrAttr) { ExprResult ArgExpr(ParseAssignmentExpression()); if (!ArgExpr.isInvalid()) { Expr *ExprList = ArgExpr.take(); - CurrAttr = new AttributeList(AttrName, AttrNameLoc, 0, AttrNameLoc, 0, - SourceLocation(), &ExprList, 1, - CurrAttr, true); + CurrAttr = AttrFactory.Create(AttrName, AttrNameLoc, 0, AttrNameLoc, 0, + SourceLocation(), &ExprList, 1, + CurrAttr, true); } if (ExpectAndConsume(tok::r_paren, diag::err_expected_rparen)) SkipUntil(tok::r_paren, false); } else { - CurrAttr = new AttributeList(AttrName, AttrNameLoc, 0, AttrNameLoc, - 0, SourceLocation(), 0, 0, CurrAttr, true); + CurrAttr = AttrFactory.Create(AttrName, AttrNameLoc, 0, AttrNameLoc, + 0, SourceLocation(), 0, 0, CurrAttr, true); } } if (ExpectAndConsume(tok::r_paren, diag::err_expected_rparen)) @@ -287,8 +287,8 @@ AttributeList* Parser::ParseMicrosoftTypeAttributes(AttributeList *CurrAttr) { if (Tok.is(tok::kw___ptr64) || Tok.is(tok::kw___w64)) // FIXME: Support these properly! continue; - CurrAttr = new AttributeList(AttrName, AttrNameLoc, 0, AttrNameLoc, 0, - SourceLocation(), 0, 0, CurrAttr, true); + CurrAttr = AttrFactory.Create(AttrName, AttrNameLoc, 0, AttrNameLoc, 0, + SourceLocation(), 0, 0, CurrAttr, true); } return CurrAttr; } @@ -298,8 +298,8 @@ AttributeList* Parser::ParseBorlandTypeAttributes(AttributeList *CurrAttr) { while (Tok.is(tok::kw___pascal)) { IdentifierInfo *AttrName = Tok.getIdentifierInfo(); SourceLocation AttrNameLoc = ConsumeToken(); - CurrAttr = new AttributeList(AttrName, AttrNameLoc, 0, AttrNameLoc, 0, - SourceLocation(), 0, 0, CurrAttr, true); + CurrAttr = AttrFactory.Create(AttrName, AttrNameLoc, 0, AttrNameLoc, 0, + SourceLocation(), 0, 0, CurrAttr, true); } return CurrAttr; } @@ -1899,15 +1899,15 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc, SourceLocation RBraceLoc = MatchRHSPunctuation(tok::r_brace, LBraceLoc); - llvm::OwningPtr AttrList; + AttributeList *AttrList = 0; // If attributes exist after struct contents, parse them. if (Tok.is(tok::kw___attribute)) - AttrList.reset(ParseGNUAttributes()); + AttrList = ParseGNUAttributes(); Actions.ActOnFields(getCurScope(), RecordLoc, TagDecl, FieldDecls.data(), FieldDecls.size(), LBraceLoc, RBraceLoc, - AttrList.get()); + AttrList); StructScope.Exit(); Actions.ActOnTagFinishDefinition(getCurScope(), TagDecl, RBraceLoc); } @@ -1950,10 +1950,10 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS, ConsumeCodeCompletionToken(); } - llvm::OwningPtr Attr; + AttributeList *Attr = 0; // If attributes exist after tag, parse them. if (Tok.is(tok::kw___attribute)) - Attr.reset(ParseGNUAttributes()); + Attr = ParseGNUAttributes(); CXXScopeSpec &SS = DS.getTypeSpecScope(); if (getLang().CPlusPlus) { @@ -2044,7 +2044,7 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS, const char *PrevSpec = 0; unsigned DiagID; Decl *TagDecl = Actions.ActOnTag(getCurScope(), DeclSpec::TST_enum, TUK, - StartLoc, SS, Name, NameLoc, Attr.get(), + StartLoc, SS, Name, NameLoc, Attr, AS, MultiTemplateParamsArg(Actions), Owned, IsDependent, IsScopedEnum, @@ -2127,9 +2127,9 @@ void Parser::ParseEnumBody(SourceLocation StartLoc, Decl *EnumDecl) { SourceLocation IdentLoc = ConsumeToken(); // If attributes exist after the enumerator, parse them. - llvm::OwningPtr Attr; + AttributeList *Attr = 0; if (Tok.is(tok::kw___attribute)) - Attr.reset(ParseGNUAttributes()); + Attr = ParseGNUAttributes(); SourceLocation EqualLoc; ExprResult AssignedVal; @@ -2144,7 +2144,7 @@ void Parser::ParseEnumBody(SourceLocation StartLoc, Decl *EnumDecl) { Decl *EnumConstDecl = Actions.ActOnEnumConstant(getCurScope(), EnumDecl, LastEnumConstDecl, IdentLoc, Ident, - Attr.get(), EqualLoc, + Attr, EqualLoc, AssignedVal.release()); EnumConstantDecls.push_back(EnumConstDecl); LastEnumConstDecl = EnumConstDecl; @@ -2171,14 +2171,14 @@ void Parser::ParseEnumBody(SourceLocation StartLoc, Decl *EnumDecl) { // Eat the }. SourceLocation RBraceLoc = MatchRHSPunctuation(tok::r_brace, LBraceLoc); - llvm::OwningPtr Attr; + AttributeList *Attr = 0; // If attributes exist after the identifier list, parse them. if (Tok.is(tok::kw___attribute)) - Attr.reset(ParseGNUAttributes()); // FIXME: where do they do? + Attr = ParseGNUAttributes(); // FIXME: where do they do? Actions.ActOnEnumBody(StartLoc, LBraceLoc, RBraceLoc, EnumDecl, EnumConstantDecls.data(), EnumConstantDecls.size(), - getCurScope(), Attr.get()); + getCurScope(), Attr); EnumScope.Exit(); Actions.ActOnTagFinishDefinition(getCurScope(), EnumDecl, RBraceLoc); @@ -2928,10 +2928,10 @@ void Parser::ParseParenDeclarator(Declarator &D) { // In either case, we need to eat any attributes to be able to determine what // sort of paren this is. // - llvm::OwningPtr AttrList; + AttributeList *AttrList = 0; bool RequiresArg = false; if (Tok.is(tok::kw___attribute)) { - AttrList.reset(ParseGNUAttributes()); + AttrList = ParseGNUAttributes(); // We require that the argument list (if this is a non-grouping paren) be // present even if the attribute list was empty. @@ -2941,12 +2941,11 @@ void Parser::ParseParenDeclarator(Declarator &D) { if (Tok.is(tok::kw___cdecl) || Tok.is(tok::kw___stdcall) || Tok.is(tok::kw___thiscall) || Tok.is(tok::kw___fastcall) || Tok.is(tok::kw___w64) || Tok.is(tok::kw___ptr64)) { - AttrList.reset(ParseMicrosoftTypeAttributes(AttrList.take())); + AttrList = ParseMicrosoftTypeAttributes(AttrList); } // Eat any Borland extensions. - if (Tok.is(tok::kw___pascal)) { - AttrList.reset(ParseBorlandTypeAttributes(AttrList.take())); - } + if (Tok.is(tok::kw___pascal)) + AttrList = ParseBorlandTypeAttributes(AttrList); // If we haven't past the identifier yet (or where the identifier would be // stored, if this is an abstract declarator), then this is probably just @@ -2976,7 +2975,7 @@ void Parser::ParseParenDeclarator(Declarator &D) { bool hadGroupingParens = D.hasGroupingParens(); D.setGroupingParens(true); if (AttrList) - D.AddAttributes(AttrList.take(), SourceLocation()); + D.AddAttributes(AttrList, SourceLocation()); ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator); // Match the ')'. @@ -2993,7 +2992,7 @@ void Parser::ParseParenDeclarator(Declarator &D) { // ParseFunctionDeclarator to handle of argument list. D.SetIdentifier(0, Tok.getLocation()); - ParseFunctionDeclarator(StartLoc, D, AttrList.take(), RequiresArg); + ParseFunctionDeclarator(StartLoc, D, AttrList, RequiresArg); } /// ParseFunctionDeclarator - We are after the identifier and have parsed the @@ -3038,10 +3037,8 @@ void Parser::ParseFunctionDeclarator(SourceLocation LParenLoc, Declarator &D, // This parameter list may be empty. if (Tok.is(tok::r_paren)) { - if (RequiresArg) { + if (RequiresArg) Diag(Tok, diag::err_argument_required_after_attribute); - delete AttrList; - } SourceLocation RParenLoc = ConsumeParen(); // Eat the closing ')'. SourceLocation EndLoc = RParenLoc; @@ -3099,10 +3096,8 @@ void Parser::ParseFunctionDeclarator(SourceLocation LParenLoc, Declarator &D, if (TryAnnotateTypeOrScopeToken() || !Tok.is(tok::annot_typename)) { // K&R identifier lists can't have typedefs as identifiers, per // C99 6.7.5.3p11. - if (RequiresArg) { + if (RequiresArg) Diag(Tok, diag::err_argument_required_after_attribute); - delete AttrList; - } // Identifier list. Note that '(' identifier-list ')' is only allowed for // normal declarators, not for abstract-declarators. Get the first diff --git a/lib/Parse/ParseDeclCXX.cpp b/lib/Parse/ParseDeclCXX.cpp index 743442a39c..140f149101 100644 --- a/lib/Parse/ParseDeclCXX.cpp +++ b/lib/Parse/ParseDeclCXX.cpp @@ -69,12 +69,12 @@ Decl *Parser::ParseNamespace(unsigned Context, } // Read label attributes, if present. - llvm::OwningPtr AttrList; + AttributeList *AttrList = 0; if (Tok.is(tok::kw___attribute)) { attrTok = Tok; // FIXME: save these somewhere. - AttrList.reset(ParseGNUAttributes()); + AttrList = ParseGNUAttributes(); } if (Tok.is(tok::equal)) { @@ -112,7 +112,7 @@ Decl *Parser::ParseNamespace(unsigned Context, Decl *NamespcDecl = Actions.ActOnStartNamespaceDef(getCurScope(), InlineLoc, IdentLoc, Ident, - LBrace, AttrList.get()); + LBrace, AttrList); PrettyDeclStackTraceEntry CrashInfo(Actions, NamespcDecl, NamespaceLoc, "parsing namespace"); @@ -391,9 +391,9 @@ Decl *Parser::ParseUsingDeclaration(unsigned Context, } // Parse (optional) attributes (most likely GNU strong-using extension). - llvm::OwningPtr AttrList; + AttributeList *AttrList = 0; if (Tok.is(tok::kw___attribute)) - AttrList.reset(ParseGNUAttributes()); + AttrList = ParseGNUAttributes(); // Eat ';'. DeclEnd = Tok.getLocation(); @@ -413,8 +413,8 @@ Decl *Parser::ParseUsingDeclaration(unsigned Context, return 0; } - return Actions.ActOnUsingDeclaration(getCurScope(), AS, true, UsingLoc, SS, Name, - AttrList.get(), IsTypeName, TypenameLoc); + return Actions.ActOnUsingDeclaration(getCurScope(), AS, true, UsingLoc, SS, + Name, AttrList, IsTypeName, TypenameLoc); } /// ParseStaticAssertDeclaration - Parse C++0x static_assert-declaratoion. @@ -1708,14 +1708,14 @@ void Parser::ParseCXXMemberSpecification(SourceLocation RecordLoc, } // If attributes exist after class contents, parse them. - llvm::OwningPtr AttrList; + AttributeList *AttrList = 0; if (Tok.is(tok::kw___attribute)) - AttrList.reset(ParseGNUAttributes()); + AttrList = ParseGNUAttributes(); if (TagDecl) Actions.ActOnFinishCXXMemberSpecification(getCurScope(), RecordLoc, TagDecl, LBraceLoc, RBraceLoc, - AttrList.get()); + AttrList); // C++ 9.2p2: Within the class member-specification, the class is regarded as // complete within function bodies, default arguments, @@ -2093,9 +2093,9 @@ CXX0XAttributeList Parser::ParseCXX0XAttributes(SourceLocation *EndLoc) { break; } - CurrAttr = new AttributeList(AttrName, AttrLoc, 0, AttrLoc, 0, - SourceLocation(), 0, 0, CurrAttr, false, - true); + CurrAttr = AttrFactory.Create(AttrName, AttrLoc, 0, AttrLoc, 0, + SourceLocation(), 0, 0, CurrAttr, false, + true); AttrParsed = true; break; } @@ -2115,9 +2115,9 @@ CXX0XAttributeList Parser::ParseCXX0XAttributes(SourceLocation *EndLoc) { ExprVector ArgExprs(Actions); ArgExprs.push_back(ArgExpr.release()); - CurrAttr = new AttributeList(AttrName, AttrLoc, 0, AttrLoc, - 0, ParamLoc, ArgExprs.take(), 1, CurrAttr, - false, true); + CurrAttr = AttrFactory.Create(AttrName, AttrLoc, 0, AttrLoc, + 0, ParamLoc, ArgExprs.take(), 1, CurrAttr, + false, true); AttrParsed = true; break; diff --git a/lib/Parse/ParseObjc.cpp b/lib/Parse/ParseObjc.cpp index 3475602c95..d18efac5e2 100644 --- a/lib/Parse/ParseObjc.cpp +++ b/lib/Parse/ParseObjc.cpp @@ -833,9 +833,9 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc, ReturnType = ParseObjCTypeName(DSRet, false); // If attributes exist before the method, parse them. - llvm::OwningPtr MethodAttrs; + AttributeList *MethodAttrs = 0; if (getLang().ObjC2 && Tok.is(tok::kw___attribute)) - MethodAttrs.reset(ParseGNUAttributes()); + MethodAttrs = ParseGNUAttributes(); if (Tok.is(tok::code_completion)) { Actions.CodeCompleteObjCMethodDecl(getCurScope(), mType == tok::minus, @@ -860,8 +860,7 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc, if (Tok.isNot(tok::colon)) { // If attributes exist after the method, parse them. if (getLang().ObjC2 && Tok.is(tok::kw___attribute)) - MethodAttrs.reset(addAttributeLists(MethodAttrs.take(), - ParseGNUAttributes())); + MethodAttrs = addAttributeLists(MethodAttrs, ParseGNUAttributes()); Selector Sel = PP.getSelectorTable().getNullarySelector(SelIdent); Decl *Result @@ -869,8 +868,7 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc, mType, IDecl, DSRet, ReturnType, Sel, 0, CParamInfo.data(), CParamInfo.size(), - MethodAttrs.get(), - MethodImplKind); + MethodAttrs, MethodImplKind); PD.complete(Result); return Result; } @@ -970,8 +968,7 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc, // FIXME: Add support for optional parameter list... // If attributes exist after the method, parse them. if (getLang().ObjC2 && Tok.is(tok::kw___attribute)) - MethodAttrs.reset(addAttributeLists(MethodAttrs.take(), - ParseGNUAttributes())); + MethodAttrs = addAttributeLists(MethodAttrs, ParseGNUAttributes()); if (KeyIdents.size() == 0) return 0; @@ -982,15 +979,9 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc, mType, IDecl, DSRet, ReturnType, Sel, &ArgInfos[0], CParamInfo.data(), CParamInfo.size(), - MethodAttrs.get(), + MethodAttrs, MethodImplKind, isVariadic); PD.complete(Result); - - // Delete referenced AttributeList objects. - for (llvm::SmallVectorImpl::iterator - I = ArgInfos.begin(), E = ArgInfos.end(); I != E; ++I) - delete I->ArgAttrs; - return Result; } diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp index 78d0d2f2d5..12444e87bb 100644 --- a/lib/Parse/ParseStmt.cpp +++ b/lib/Parse/ParseStmt.cpp @@ -84,7 +84,7 @@ Parser::ParseStatementOrDeclaration(StmtVector &Stmts, bool OnlyStatement) { CXX0XAttributeList Attr; if (getLang().CPlusPlus0x && isCXX0XAttributeSpecifier()) Attr = ParseCXX0XAttributes(); - llvm::OwningPtr AttrList(Attr.AttrList); + AttributeList *AttrList = Attr.AttrList; // Cases in this switch statement should fall through if the parser expects // the token to end in a semicolon (in which case SemiError should be set), @@ -106,16 +106,15 @@ Parser::ParseStatementOrDeclaration(StmtVector &Stmts, bool OnlyStatement) { case tok::identifier: if (NextToken().is(tok::colon)) { // C99 6.8.1: labeled-statement // identifier ':' statement - return ParseLabeledStatement(AttrList.take()); + return ParseLabeledStatement(AttrList); } // PASS THROUGH. default: { if ((getLang().CPlusPlus || !OnlyStatement) && isDeclarationStatement()) { SourceLocation DeclStart = Tok.getLocation(), DeclEnd; - AttrList.take(); //Passing 'Attr' to ParseDeclaration transfers ownership. - DeclGroupPtrTy Decl = ParseDeclaration(Stmts, Declarator::BlockContext, DeclEnd, - Attr); + DeclGroupPtrTy Decl = ParseDeclaration(Stmts, Declarator::BlockContext, + DeclEnd, Attr); return Actions.ActOnDeclStmt(Decl, DeclStart, DeclEnd); } @@ -142,43 +141,43 @@ Parser::ParseStatementOrDeclaration(StmtVector &Stmts, bool OnlyStatement) { } case tok::kw_case: // C99 6.8.1: labeled-statement - return ParseCaseStatement(AttrList.take()); + return ParseCaseStatement(AttrList); case tok::kw_default: // C99 6.8.1: labeled-statement - return ParseDefaultStatement(AttrList.take()); + return ParseDefaultStatement(AttrList); case tok::l_brace: // C99 6.8.2: compound-statement - return ParseCompoundStatement(AttrList.take()); + return ParseCompoundStatement(AttrList); case tok::semi: // C99 6.8.3p3: expression[opt] ';' return Actions.ActOnNullStmt(ConsumeToken()); case tok::kw_if: // C99 6.8.4.1: if-statement - return ParseIfStatement(AttrList.take()); + return ParseIfStatement(AttrList); case tok::kw_switch: // C99 6.8.4.2: switch-statement - return ParseSwitchStatement(AttrList.take()); + return ParseSwitchStatement(AttrList); case tok::kw_while: // C99 6.8.5.1: while-statement - return ParseWhileStatement(AttrList.take()); + return ParseWhileStatement(AttrList); case tok::kw_do: // C99 6.8.5.2: do-statement - Res = ParseDoStatement(AttrList.take()); + Res = ParseDoStatement(AttrList); SemiError = "do/while"; break; case tok::kw_for: // C99 6.8.5.3: for-statement - return ParseForStatement(AttrList.take()); + return ParseForStatement(AttrList); case tok::kw_goto: // C99 6.8.6.1: goto-statement - Res = ParseGotoStatement(AttrList.take()); + Res = ParseGotoStatement(AttrList); SemiError = "goto"; break; case tok::kw_continue: // C99 6.8.6.2: continue-statement - Res = ParseContinueStatement(AttrList.take()); + Res = ParseContinueStatement(AttrList); SemiError = "continue"; break; case tok::kw_break: // C99 6.8.6.3: break-statement - Res = ParseBreakStatement(AttrList.take()); + Res = ParseBreakStatement(AttrList); SemiError = "break"; break; case tok::kw_return: // C99 6.8.6.4: return-statement - Res = ParseReturnStatement(AttrList.take()); + Res = ParseReturnStatement(AttrList); SemiError = "return"; break; @@ -195,7 +194,7 @@ Parser::ParseStatementOrDeclaration(StmtVector &Stmts, bool OnlyStatement) { } case tok::kw_try: // C++ 15: try-block - return ParseCXXTryBlock(AttrList.take()); + return ParseCXXTryBlock(AttrList); } // If we reached this code, the statement must end in a semicolon. @@ -223,7 +222,6 @@ StmtResult Parser::ParseLabeledStatement(AttributeList *Attr) { assert(Tok.is(tok::identifier) && Tok.getIdentifierInfo() && "Not an identifier!"); - llvm::OwningPtr AttrList(Attr); Token IdentTok = Tok; // Save the whole token. ConsumeToken(); // eat the identifier. @@ -234,7 +232,7 @@ StmtResult Parser::ParseLabeledStatement(AttributeList *Attr) { // Read label attributes, if present. if (Tok.is(tok::kw___attribute)) - AttrList.reset(addAttributeLists(AttrList.take(), ParseGNUAttributes())); + Attr = addAttributeLists(Attr, ParseGNUAttributes()); StmtResult SubStmt(ParseStatement()); @@ -244,7 +242,7 @@ StmtResult Parser::ParseLabeledStatement(AttributeList *Attr) { return Actions.ActOnLabelStmt(IdentTok.getLocation(), IdentTok.getIdentifierInfo(), - ColonLoc, SubStmt.get(), AttrList.take()); + ColonLoc, SubStmt.get(), Attr); } /// ParseCaseStatement @@ -255,7 +253,6 @@ StmtResult Parser::ParseLabeledStatement(AttributeList *Attr) { StmtResult Parser::ParseCaseStatement(AttributeList *Attr) { assert(Tok.is(tok::kw_case) && "Not a case stmt!"); // FIXME: Use attributes? - delete Attr; // It is very very common for code to contain many case statements recursively // nested, as in (but usually without indentation): @@ -381,7 +378,6 @@ StmtResult Parser::ParseCaseStatement(AttributeList *Attr) { /// StmtResult Parser::ParseDefaultStatement(AttributeList *Attr) { //FIXME: Use attributes? - delete Attr; assert(Tok.is(tok::kw_default) && "Not a default stmt!"); SourceLocation DefaultLoc = ConsumeToken(); // eat the 'default'. @@ -439,7 +435,6 @@ StmtResult Parser::ParseDefaultStatement(AttributeList *Attr) { StmtResult Parser::ParseCompoundStatement(AttributeList *Attr, bool isStmtExpr) { //FIXME: Use attributes? - delete Attr; assert(Tok.is(tok::l_brace) && "Not a compount stmt!"); @@ -586,7 +581,6 @@ bool Parser::ParseParenExprOrCondition(ExprResult &ExprResult, /// StmtResult Parser::ParseIfStatement(AttributeList *Attr) { // FIXME: Use attributes? - delete Attr; assert(Tok.is(tok::kw_if) && "Not an if stmt!"); SourceLocation IfLoc = ConsumeToken(); // eat the 'if'. @@ -709,7 +703,6 @@ StmtResult Parser::ParseIfStatement(AttributeList *Attr) { /// [C++] 'switch' '(' condition ')' statement StmtResult Parser::ParseSwitchStatement(AttributeList *Attr) { // FIXME: Use attributes? - delete Attr; assert(Tok.is(tok::kw_switch) && "Not a switch stmt!"); SourceLocation SwitchLoc = ConsumeToken(); // eat the 'switch'. @@ -795,7 +788,6 @@ StmtResult Parser::ParseSwitchStatement(AttributeList *Attr) { /// [C++] 'while' '(' condition ')' statement StmtResult Parser::ParseWhileStatement(AttributeList *Attr) { // FIXME: Use attributes? - delete Attr; assert(Tok.is(tok::kw_while) && "Not a while stmt!"); SourceLocation WhileLoc = Tok.getLocation(); @@ -870,7 +862,6 @@ StmtResult Parser::ParseWhileStatement(AttributeList *Attr) { /// Note: this lets the caller parse the end ';'. StmtResult Parser::ParseDoStatement(AttributeList *Attr) { // FIXME: Use attributes? - delete Attr; assert(Tok.is(tok::kw_do) && "Not a do stmt!"); SourceLocation DoLoc = ConsumeToken(); // eat the 'do'. @@ -947,7 +938,6 @@ StmtResult Parser::ParseDoStatement(AttributeList *Attr) { /// StmtResult Parser::ParseForStatement(AttributeList *Attr) { // FIXME: Use attributes? - delete Attr; assert(Tok.is(tok::kw_for) && "Not a for stmt!"); SourceLocation ForLoc = ConsumeToken(); // eat the 'for'. @@ -1141,7 +1131,6 @@ StmtResult Parser::ParseForStatement(AttributeList *Attr) { /// StmtResult Parser::ParseGotoStatement(AttributeList *Attr) { // FIXME: Use attributes? - delete Attr; assert(Tok.is(tok::kw_goto) && "Not a goto stmt!"); SourceLocation GotoLoc = ConsumeToken(); // eat the 'goto'. @@ -1177,7 +1166,6 @@ StmtResult Parser::ParseGotoStatement(AttributeList *Attr) { /// StmtResult Parser::ParseContinueStatement(AttributeList *Attr) { // FIXME: Use attributes? - delete Attr; SourceLocation ContinueLoc = ConsumeToken(); // eat the 'continue'. return Actions.ActOnContinueStmt(ContinueLoc, getCurScope()); @@ -1191,7 +1179,6 @@ StmtResult Parser::ParseContinueStatement(AttributeList *Attr) { /// StmtResult Parser::ParseBreakStatement(AttributeList *Attr) { // FIXME: Use attributes? - delete Attr; SourceLocation BreakLoc = ConsumeToken(); // eat the 'break'. return Actions.ActOnBreakStmt(BreakLoc, getCurScope()); @@ -1202,7 +1189,6 @@ StmtResult Parser::ParseBreakStatement(AttributeList *Attr) { /// 'return' expression[opt] ';' StmtResult Parser::ParseReturnStatement(AttributeList *Attr) { // FIXME: Use attributes? - delete Attr; assert(Tok.is(tok::kw_return) && "Not a return stmt!"); SourceLocation ReturnLoc = ConsumeToken(); // eat the 'return'. @@ -1522,7 +1508,6 @@ Decl *Parser::ParseFunctionTryBlock(Decl *Decl) { /// StmtResult Parser::ParseCXXTryBlock(AttributeList* Attr) { // FIXME: Add attributes? - delete Attr; assert(Tok.is(tok::kw_try) && "Expected 'try'"); diff --git a/lib/Sema/AttributeList.cpp b/lib/Sema/AttributeList.cpp index 89f59f9195..06bde633d9 100644 --- a/lib/Sema/AttributeList.cpp +++ b/lib/Sema/AttributeList.cpp @@ -16,37 +16,26 @@ #include "llvm/ADT/StringSwitch.h" using namespace clang; -AttributeList::AttributeList(IdentifierInfo *aName, SourceLocation aLoc, +AttributeList::AttributeList(llvm::BumpPtrAllocator &Alloc, + IdentifierInfo *aName, SourceLocation aLoc, IdentifierInfo *sName, SourceLocation sLoc, IdentifierInfo *pName, SourceLocation pLoc, Expr **ExprList, unsigned numArgs, AttributeList *n, bool declspec, bool cxx0x) - : AttrName(aName), AttrLoc(aLoc), ScopeName(sName), ScopeLoc(sLoc), + : AttrName(aName), AttrLoc(aLoc), ScopeName(sName), + ScopeLoc(sLoc), ParmName(pName), ParmLoc(pLoc), NumArgs(numArgs), Next(n), DeclspecAttribute(declspec), CXX0XAttribute(cxx0x), Invalid(false) { if (numArgs == 0) Args = 0; else { - Args = new Expr*[numArgs]; + // Allocate the Args array using the BumpPtrAllocator. + Args = Alloc.Allocate(numArgs); memcpy(Args, ExprList, numArgs*sizeof(Args[0])); } } -AttributeList::~AttributeList() { - if (Args) { - // FIXME: before we delete the vector, we need to make sure the Expr's - // have been deleted. Since ActionBase::ExprTy is "void", we are dependent - // on the actions module for actually freeing the memory. The specific - // hooks are ActOnDeclarator, ActOnTypeName, ActOnParamDeclaratorType, - // ParseField, ParseTag. Once these routines have freed the expression, - // they should zero out the Args slot (to indicate the memory has been - // freed). If any element of the vector is non-null, we should assert. - delete [] Args; - } - delete Next; -} - AttributeList::Kind AttributeList::getKind(const IdentifierInfo *Name) { llvm::StringRef AttrName = Name->getName(); diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 33b3124090..f7638f80d9 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -3483,7 +3483,6 @@ Decl *Sema::ActOnUsingDirective(Scope *S, } // FIXME: We ignore attributes for now. - delete AttrList; return UDir; } @@ -3810,7 +3809,6 @@ NamedDecl *Sema::BuildUsingDeclaration(Scope *S, AccessSpecifier AS, assert(IdentLoc.isValid() && "Invalid TargetName location."); // FIXME: We ignore attributes for now. - delete AttrList; if (SS.isEmpty()) { Diag(IdentLoc, diag::err_using_requires_qualname); diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index 4c586e5e02..a4f1d34aec 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/lib/Sema/SemaStmt.cpp @@ -238,13 +238,12 @@ Sema::ActOnLabelStmt(SourceLocation IdentLoc, IdentifierInfo *II, // According to GCC docs, "the only attribute that makes sense after a label // is 'unused'". bool HasUnusedAttr = false; - llvm::OwningPtr AttrList(Attr); - for (const AttributeList* a = AttrList.get(); a; a = a->getNext()) { - if (a->getKind() == AttributeList::AT_unused) { + for ( ; Attr; Attr = Attr->getNext()) { + if (Attr->getKind() == AttributeList::AT_unused) { HasUnusedAttr = true; } else { - Diag(a->getLoc(), diag::warn_label_attribute_not_unused); - a->setInvalid(true); + Diag(Attr->getLoc(), diag::warn_label_attribute_not_unused); + Attr->setInvalid(true); } }