[cfe-commits] r96642 - in /cfe/trunk: include/clang/AST/Expr.h lib/AST/Expr.cpp lib/Frontend/PCHReaderStmt.cpp lib/Frontend/RewriteObjC.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaInit.cpp
Ted Kremenek
kremenek at apple.com
Thu Feb 18 16:42:34 PST 2010
Author: kremenek
Date: Thu Feb 18 18:42:33 2010
New Revision: 96642
URL: http://llvm.org/viewvc/llvm-project?rev=96642&view=rev
Log:
Change InitListExpr to allocate the array for holding references
to initializer expressions in an array allocated using ASTContext.
This plugs a memory leak when ASTContext uses a BumpPtrAllocator to
allocate memory for AST nodes.
In my mind this isn't an ideal solution; it would be nice to have
a general "vector"-like class that allocates memory using ASTContext,
but whose guts could be separated from the methods of InitListExpr
itself. I haven't gone and taken this approach yet because it isn't
clear yet if we'll eventually want an alternate solution for recylcing
memory using by InitListExprs as we are constructing the ASTs.
Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/Frontend/PCHReaderStmt.cpp
cfe/trunk/lib/Frontend/RewriteObjC.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
Modified: cfe/trunk/include/clang/AST/Expr.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=96642&r1=96641&r2=96642&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Feb 18 18:42:33 2010
@@ -2419,8 +2419,15 @@
/// return NULL, indicating that the current initializer list also
/// serves as its syntactic form.
class InitListExpr : public Expr {
- // FIXME: Eliminate this vector in favor of ASTContext allocation
- std::vector<Stmt *> InitExprs;
+ /// The initializers.
+ // FIXME: Instead of directly maintaining the initializer array,
+ // we may wish to use a specialized vector class that uses ASTContext
+ // to allocate memory. This will keep the implementation simpler, and
+ // we can possibly use ASTContext to recylce memory even when using
+ // a BumpPtrAllocator.
+ Stmt **InitExprs;
+ unsigned NumInits, Capacity;
+
SourceLocation LBraceLoc, RBraceLoc;
/// Contains the initializer list that describes the syntactic form
@@ -2436,13 +2443,15 @@
bool HadArrayRangeDesignator;
public:
- InitListExpr(SourceLocation lbraceloc, Expr **initexprs, unsigned numinits,
- SourceLocation rbraceloc);
+ InitListExpr(ASTContext &C, SourceLocation lbraceloc, Expr **initexprs,
+ unsigned numinits, SourceLocation rbraceloc);
+
+ virtual void DoDestroy(ASTContext &C);
/// \brief Build an empty initializer list.
explicit InitListExpr(EmptyShell Empty) : Expr(InitListExprClass, Empty) { }
- unsigned getNumInits() const { return InitExprs.size(); }
+ unsigned getNumInits() const { return NumInits; }
const Expr* getInit(unsigned Init) const {
assert(Init < getNumInits() && "Initializer access out of range!");
@@ -2460,7 +2469,7 @@
}
/// \brief Reserve space for some number of initializers.
- void reserveInits(unsigned NumInits);
+ void reserveInits(ASTContext &Context, unsigned NumInits);
/// @brief Specify the number of initializers
///
@@ -2477,7 +2486,7 @@
/// When @p Init is out of range for this initializer list, the
/// initializer list will be extended with NULL expressions to
/// accomodate the new entry.
- Expr *updateInit(unsigned Init, Expr *expr);
+ Expr *updateInit(ASTContext &Context, unsigned Init, Expr *expr);
/// \brief If this initializes a union, specifies which field in the
/// union to initialize.
@@ -2523,13 +2532,13 @@
virtual child_iterator child_begin();
virtual child_iterator child_end();
- typedef std::vector<Stmt *>::iterator iterator;
- typedef std::vector<Stmt *>::reverse_iterator reverse_iterator;
+ typedef Stmt** iterator;
+ typedef std::reverse_iterator<iterator> reverse_iterator;
- iterator begin() { return InitExprs.begin(); }
- iterator end() { return InitExprs.end(); }
- reverse_iterator rbegin() { return InitExprs.rbegin(); }
- reverse_iterator rend() { return InitExprs.rend(); }
+ iterator begin() { return InitExprs ? InitExprs : 0; }
+ iterator end() { return InitExprs ? InitExprs + NumInits : 0; }
+ reverse_iterator rbegin() { return reverse_iterator(end()); }
+ reverse_iterator rend() { return reverse_iterator(begin()); }
};
/// @brief Represents a C99 designated initializer expression.
Modified: cfe/trunk/lib/AST/Expr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=96642&r1=96641&r2=96642&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Thu Feb 18 18:42:33 2010
@@ -713,40 +713,75 @@
return OverOps[Opc];
}
-InitListExpr::InitListExpr(SourceLocation lbraceloc,
+InitListExpr::InitListExpr(ASTContext &C, SourceLocation lbraceloc,
Expr **initExprs, unsigned numInits,
SourceLocation rbraceloc)
: Expr(InitListExprClass, QualType(), false, false),
+ InitExprs(0), NumInits(numInits), Capacity(numInits),
LBraceLoc(lbraceloc), RBraceLoc(rbraceloc), SyntacticForm(0),
- UnionFieldInit(0), HadArrayRangeDesignator(false)
-{
- for (unsigned I = 0; I != numInits; ++I) {
- if (initExprs[I]->isTypeDependent())
+ UnionFieldInit(0), HadArrayRangeDesignator(false)
+{
+ if (NumInits == 0)
+ return;
+
+ InitExprs = new (C) Stmt*[Capacity];
+
+ for (unsigned I = 0; I != NumInits; ++I) {
+ Expr *Ex = initExprs[I];
+ if (Ex->isTypeDependent())
TypeDependent = true;
- if (initExprs[I]->isValueDependent())
+ if (Ex->isValueDependent())
ValueDependent = true;
+ InitExprs[I] = Ex;
}
-
- InitExprs.insert(InitExprs.end(), initExprs, initExprs+numInits);
}
-void InitListExpr::reserveInits(unsigned NumInits) {
- if (NumInits > InitExprs.size())
- InitExprs.reserve(NumInits);
+void InitListExpr::DoDestroy(ASTContext &C) {
+ DestroyChildren(C);
+ if (InitExprs)
+ C.Deallocate(InitExprs);
+ this->~InitListExpr();
+ C.Deallocate((void*) this);
}
-void InitListExpr::resizeInits(ASTContext &Context, unsigned NumInits) {
- for (unsigned Idx = NumInits, LastIdx = InitExprs.size();
- Idx < LastIdx; ++Idx)
- InitExprs[Idx]->Destroy(Context);
- InitExprs.resize(NumInits, 0);
+void InitListExpr::reserveInits(ASTContext &C, unsigned newCapacity) {
+ if (newCapacity > Capacity) {
+ if (!Capacity)
+ Capacity = newCapacity;
+ else if ((Capacity *= 2) < newCapacity)
+ Capacity = newCapacity;
+
+ Stmt **newInits = new (C) Stmt*[Capacity];
+ if (InitExprs) {
+ memcpy(newInits, InitExprs, NumInits * sizeof(*InitExprs));
+ C.Deallocate(InitExprs);
+ }
+ InitExprs = newInits;
+ }
}
-Expr *InitListExpr::updateInit(unsigned Init, Expr *expr) {
- if (Init >= InitExprs.size()) {
- InitExprs.insert(InitExprs.end(), Init - InitExprs.size() + 1, 0);
- InitExprs.back() = expr;
- return 0;
+void InitListExpr::resizeInits(ASTContext &C, unsigned N) {
+ // If the new number of expressions is less than the old one, destroy
+ // the expressions that are beyond the new size.
+ for (unsigned i = N, LastIdx = NumInits; i < LastIdx; ++i)
+ InitExprs[i]->Destroy(C);
+
+ // If we are expanding the number of expressions, reserve space.
+ reserveInits(C, N);
+
+ // If we are expanding the number of expressions, zero out beyond our
+ // current capacity.
+ for (unsigned i = NumInits; i < N; ++i)
+ InitExprs[i] = 0;
+
+ NumInits = N;
+}
+
+Expr *InitListExpr::updateInit(ASTContext &C, unsigned Init, Expr *expr) {
+ if (Init >= NumInits) {
+ // Resize the number of initializers. This will adjust the amount
+ // of memory allocated as well as zero-pad the initializers.
+ resizeInits(C, Init+1);
}
Expr *Result = cast_or_null<Expr>(InitExprs[Init]);
@@ -2586,12 +2621,8 @@
Stmt::child_iterator VAArgExpr::child_end() { return &Val+1; }
// InitListExpr
-Stmt::child_iterator InitListExpr::child_begin() {
- return InitExprs.size() ? &InitExprs[0] : 0;
-}
-Stmt::child_iterator InitListExpr::child_end() {
- return InitExprs.size() ? &InitExprs[0] + InitExprs.size() : 0;
-}
+Stmt::child_iterator InitListExpr::child_begin() { return begin(); }
+Stmt::child_iterator InitListExpr::child_end() { return end(); }
// DesignatedInitExpr
Stmt::child_iterator DesignatedInitExpr::child_begin() {
Modified: cfe/trunk/lib/Frontend/PCHReaderStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/PCHReaderStmt.cpp?rev=96642&r1=96641&r2=96642&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/PCHReaderStmt.cpp (original)
+++ cfe/trunk/lib/Frontend/PCHReaderStmt.cpp Thu Feb 18 18:42:33 2010
@@ -554,9 +554,9 @@
unsigned PCHStmtReader::VisitInitListExpr(InitListExpr *E) {
VisitExpr(E);
unsigned NumInits = Record[Idx++];
- E->reserveInits(NumInits);
+ E->reserveInits(*Reader.getContext(), NumInits);
for (unsigned I = 0; I != NumInits; ++I)
- E->updateInit(I,
+ E->updateInit(*Reader.getContext(), I,
cast<Expr>(StmtStack[StmtStack.size() - NumInits - 1 + I]));
E->setSyntacticForm(cast_or_null<InitListExpr>(StmtStack.back()));
E->setLBraceLoc(SourceLocation::getFromRawEncoding(Record[Idx++]));
Modified: cfe/trunk/lib/Frontend/RewriteObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/RewriteObjC.cpp?rev=96642&r1=96641&r2=96642&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/RewriteObjC.cpp (original)
+++ cfe/trunk/lib/Frontend/RewriteObjC.cpp Thu Feb 18 18:42:33 2010
@@ -2613,9 +2613,11 @@
CastExpr::CK_Unknown, SuperRep);
} else {
// (struct objc_super) { <exprs from above> }
- InitListExpr *ILE = new (Context) InitListExpr(SourceLocation(),
- &InitExprs[0], InitExprs.size(),
- SourceLocation());
+ InitListExpr *ILE = new (Context) InitListExpr(*Context,
+ SourceLocation(),
+ &InitExprs[0],
+ InitExprs.size(),
+ SourceLocation());
TypeSourceInfo *superTInfo
= Context->getTrivialTypeSourceInfo(superType);
SuperRep = new (Context) CompoundLiteralExpr(SourceLocation(), superTInfo,
@@ -2698,9 +2700,11 @@
CastExpr::CK_Unknown, SuperRep);
} else {
// (struct objc_super) { <exprs from above> }
- InitListExpr *ILE = new (Context) InitListExpr(SourceLocation(),
- &InitExprs[0], InitExprs.size(),
- SourceLocation());
+ InitListExpr *ILE = new (Context) InitListExpr(*Context,
+ SourceLocation(),
+ &InitExprs[0],
+ InitExprs.size(),
+ SourceLocation());
TypeSourceInfo *superTInfo
= Context->getTrivialTypeSourceInfo(superType);
SuperRep = new (Context) CompoundLiteralExpr(SourceLocation(), superTInfo,
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=96642&r1=96641&r2=96642&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Feb 18 18:42:33 2010
@@ -3763,8 +3763,8 @@
// Semantic analysis for initializers is done by ActOnDeclarator() and
// CheckInitializer() - it requires knowledge of the object being intialized.
- InitListExpr *E = new (Context) InitListExpr(LBraceLoc, InitList, NumInit,
- RBraceLoc);
+ InitListExpr *E = new (Context) InitListExpr(Context, LBraceLoc, InitList,
+ NumInit, RBraceLoc);
E->setType(Context.VoidTy); // FIXME: just a place holder for now.
return Owned(E);
}
@@ -4038,7 +4038,8 @@
// FIXME: This means that pretty-printing the final AST will produce curly
// braces instead of the original commas.
Op.release();
- InitListExpr *E = new (Context) InitListExpr(LParenLoc, &initExprs[0],
+ InitListExpr *E = new (Context) InitListExpr(Context, LParenLoc,
+ &initExprs[0],
initExprs.size(), RParenLoc);
E->setType(Ty);
return BuildCompoundLiteralExpr(LParenLoc, TInfo, RParenLoc, Owned(E));
@@ -4752,8 +4753,7 @@
QualType UnionType, FieldDecl *Field) {
// Build an initializer list that designates the appropriate member
// of the transparent union.
- InitListExpr *Initializer = new (C) InitListExpr(SourceLocation(),
- &E, 1,
+ InitListExpr *Initializer = new (C) InitListExpr(C, SourceLocation(), &E, 1,
SourceLocation());
Initializer->setType(UnionType);
Initializer->setInitializedFieldInUnion(Field);
Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=96642&r1=96641&r2=96642&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Thu Feb 18 18:42:33 2010
@@ -280,7 +280,7 @@
// extend the initializer list to include the constructor
// call and make a note that we'll need to take another pass
// through the initializer list.
- ILE->updateInit(Init, MemberInit.takeAs<Expr>());
+ ILE->updateInit(SemaRef.Context, Init, MemberInit.takeAs<Expr>());
RequiresSecondPass = true;
}
} else if (InitListExpr *InnerILE
@@ -390,7 +390,7 @@
// extend the initializer list to include the constructor
// call and make a note that we'll need to take another pass
// through the initializer list.
- ILE->updateInit(Init, ElementInit.takeAs<Expr>());
+ ILE->updateInit(SemaRef.Context, Init, ElementInit.takeAs<Expr>());
RequiresSecondPass = true;
}
} else if (InitListExpr *InnerILE
@@ -1669,8 +1669,8 @@
}
InitListExpr *Result
- = new (SemaRef.Context) InitListExpr(InitRange.getBegin(), 0, 0,
- InitRange.getEnd());
+ = new (SemaRef.Context) InitListExpr(SemaRef.Context, InitRange.getBegin(),
+ 0, 0, InitRange.getEnd());
Result->setType(CurrentObjectType.getNonReferenceType());
@@ -1707,12 +1707,12 @@
if (NumElements < NumInits)
NumElements = IList->getNumInits();
- Result->reserveInits(NumElements);
+ Result->reserveInits(SemaRef.Context, NumElements);
// Link this new initializer list into the structured initializer
// lists.
if (StructuredList)
- StructuredList->updateInit(StructuredIndex, Result);
+ StructuredList->updateInit(SemaRef.Context, StructuredIndex, Result);
else {
Result->setSyntacticForm(IList);
SyntacticToSemantic[IList] = Result;
@@ -1730,7 +1730,8 @@
if (!StructuredList)
return;
- if (Expr *PrevInit = StructuredList->updateInit(StructuredIndex, expr)) {
+ if (Expr *PrevInit = StructuredList->updateInit(SemaRef.Context,
+ StructuredIndex, expr)) {
// This initializer overwrites a previous initializer. Warn.
SemaRef.Diag(expr->getSourceRange().getBegin(),
diag::warn_initializer_overrides)
More information about the cfe-commits
mailing list