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.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@96642 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h
index 23076b93e13b4b50ed5ca05ae86192e4ba8be58a..ecb821eb671c0229159fb239e44c23d3bf2f7029 100644 (file)
--- a/include/clang/AST/Expr.h
+++ b/include/clang/AST/Expr.h
@@ -2419,8 +2419,15 @@ public:
 /// 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 @@ class InitListExpr : public Expr {
   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 @@ public:
   }
 
   /// \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 @@ public:
   /// 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 @@ public:
   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.

diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
index fac65064c07c586092a3716e17082a90c8e571c8..58776f47d97e4e8a26325354aab91f8f13a6a4ab 100644 (file)
--- a/lib/AST/Expr.cpp
+++ b/lib/AST/Expr.cpp
@@ -713,40 +713,75 @@ OverloadedOperatorKind BinaryOperator::getOverloadedOperator(Opcode Opc) {
   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_begin() { return &Val; }
 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() {

diff --git a/lib/Frontend/PCHReaderStmt.cpp b/lib/Frontend/PCHReaderStmt.cpp
index d123694d699d8b302ff267d0fbab058397da8f3f..c69a5dc9dc16fc87aad0f2384571d07e6ce096d4 100644 (file)
--- a/lib/Frontend/PCHReaderStmt.cpp
+++ b/lib/Frontend/PCHReaderStmt.cpp
@@ -554,9 +554,9 @@ unsigned PCHStmtReader::VisitExtVectorElementExpr(ExtVectorElementExpr *E) {
 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++]));

diff --git a/lib/Frontend/RewriteObjC.cpp b/lib/Frontend/RewriteObjC.cpp
index fff8b54ba6cb4d0269b926d07ca42a32311ea8ec..40d444d94e43a2cacbe8f43b8453280da5ab3d45 100644 (file)
--- a/lib/Frontend/RewriteObjC.cpp
+++ b/lib/Frontend/RewriteObjC.cpp
@@ -2613,9 +2613,11 @@ Stmt *RewriteObjC::SynthMessageExpr(ObjCMessageExpr *Exp) {
                                             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 @@ Stmt *RewriteObjC::SynthMessageExpr(ObjCMessageExpr *Exp) {
                                  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,

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index e950be04858759b12b83a0592ec86baa74725b8b..0d54a164127215a6e8c519f5fd1e151d29d8e2e0 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -3763,8 +3763,8 @@ Sema::ActOnInitList(SourceLocation LBraceLoc, MultiExprArg initlist,
   // 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 @@ Sema::ActOnCastOfParenListExpr(Scope *S, SourceLocation LParenLoc,
     // 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 @@ static void ConstructTransparentUnion(ASTContext &C, Expr *&E,
                                       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);

diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp
index 7b4a41777b6ac32ae1aca1290be65fd947c6ca6e..1c4e2c837aee686e545543e1ab3b378a3934cd15 100644 (file)
--- a/lib/Sema/SemaInit.cpp
+++ b/lib/Sema/SemaInit.cpp
@@ -280,7 +280,7 @@ void InitListChecker::FillInValueInitForField(unsigned Init, FieldDecl *Field,
       // 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 @@ InitListChecker::FillInValueInitializations(const InitializedEntity &Entity,
         // 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 @@ InitListChecker::getStructuredSubobjectInit(InitListExpr *IList, unsigned Index,
   }
 
   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 @@ InitListChecker::getStructuredSubobjectInit(InitListExpr *IList, unsigned Index,
   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 @@ void InitListChecker::UpdateStructuredListElement(InitListExpr *StructuredList,
   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)