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: <rdar://problem/8650003>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@118675 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Parse/ParseDeclCXX.cpp b/lib/Parse/ParseDeclCXX.cpp
index 743442a..140f149 100644
--- a/lib/Parse/ParseDeclCXX.cpp
+++ b/lib/Parse/ParseDeclCXX.cpp
@@ -69,12 +69,12 @@
}
// Read label attributes, if present.
- llvm::OwningPtr<AttributeList> 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 *NamespcDecl =
Actions.ActOnStartNamespaceDef(getCurScope(), InlineLoc, IdentLoc, Ident,
- LBrace, AttrList.get());
+ LBrace, AttrList);
PrettyDeclStackTraceEntry CrashInfo(Actions, NamespcDecl, NamespaceLoc,
"parsing namespace");
@@ -391,9 +391,9 @@
}
// Parse (optional) attributes (most likely GNU strong-using extension).
- llvm::OwningPtr<AttributeList> AttrList;
+ AttributeList *AttrList = 0;
if (Tok.is(tok::kw___attribute))
- AttrList.reset(ParseGNUAttributes());
+ AttrList = ParseGNUAttributes();
// Eat ';'.
DeclEnd = Tok.getLocation();
@@ -413,8 +413,8 @@
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 @@
}
// If attributes exist after class contents, parse them.
- llvm::OwningPtr<AttributeList> 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 @@
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 @@
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;