[cfe-commits] r118675 - in /cfe/trunk: include/clang/Parse/Parser.h include/clang/Sema/AttributeList.h include/clang/Sema/DeclSpec.h lib/Parse/ParseDecl.cpp lib/Parse/ParseDeclCXX.cpp lib/Parse/ParseObjc.cpp lib/Parse/ParseStmt.cpp lib/Sema/AttributeList.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaStmt.cpp

Ted Kremenek kremenek at apple.com
Tue Nov 9 21:59:39 PST 2010


Author: kremenek
Date: Tue Nov  9 23:59:39 2010
New Revision: 118675

URL: http://llvm.org/viewvc/llvm-project?rev=118675&view=rev
Log:
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>

Modified:
    cfe/trunk/include/clang/Parse/Parser.h
    cfe/trunk/include/clang/Sema/AttributeList.h
    cfe/trunk/include/clang/Sema/DeclSpec.h
    cfe/trunk/lib/Parse/ParseDecl.cpp
    cfe/trunk/lib/Parse/ParseDeclCXX.cpp
    cfe/trunk/lib/Parse/ParseObjc.cpp
    cfe/trunk/lib/Parse/ParseStmt.cpp
    cfe/trunk/lib/Sema/AttributeList.cpp
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
    cfe/trunk/lib/Sema/SemaStmt.cpp

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=118675&r1=118674&r2=118675&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Tue Nov  9 23:59:39 2010
@@ -25,8 +25,6 @@
 #include <list>
 
 namespace clang {
-  class AttributeList;
-  struct CXX0XAttributeList;
   class PragmaHandler;
   class Scope;
   class DeclGroupRef;
@@ -142,6 +140,9 @@
   
   /// 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);

Modified: cfe/trunk/include/clang/Sema/AttributeList.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=118675&r1=118674&r2=118675&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/AttributeList.h (original)
+++ cfe/trunk/include/clang/Sema/AttributeList.h Tue Nov  9 23:59:39 2010
@@ -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 <cassert>
@@ -32,6 +33,9 @@
 /// 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 @@
   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<AttributeList>();
+        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.

Modified: cfe/trunk/include/clang/Sema/DeclSpec.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/DeclSpec.h?rev=118675&r1=118674&r2=118675&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/DeclSpec.h (original)
+++ cfe/trunk/include/clang/Sema/DeclSpec.h Tue Nov  9 23:59:39 2010
@@ -274,7 +274,6 @@
       writtenBS() {
   }
   ~DeclSpec() {
-    delete AttrList;
     delete [] ProtocolQualifiers;
     delete [] ProtocolLocs;
   }
@@ -813,7 +812,6 @@
     unsigned TypeQuals : 3;
     AttributeList *AttrList;
     void destroy() {
-      delete AttrList;
     }
   };
 
@@ -824,7 +822,6 @@
     bool LValueRef : 1;
     AttributeList *AttrList;
     void destroy() {
-      delete AttrList;
     }
   };
 
@@ -967,7 +964,6 @@
     unsigned TypeQuals : 3;
     AttributeList *AttrList;
     void destroy() {
-      delete AttrList;
     }
   };
 
@@ -988,7 +984,6 @@
       return *reinterpret_cast<const CXXScopeSpec*>(ScopeMem.Mem);
     }
     void destroy() {
-      delete AttrList;
       Scope().~CXXScopeSpec();
     }
   };
@@ -1244,7 +1239,6 @@
     for (unsigned i = 0, e = DeclTypeInfo.size(); i != e; ++i)
       DeclTypeInfo[i].destroy();
     DeclTypeInfo.clear();
-    delete AttrList;
     AttrList = 0;
     AsmLabel = 0;
     InlineParamsUsed = false;

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=118675&r1=118674&r2=118675&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Tue Nov  9 23:59:39 2010
@@ -122,8 +122,8 @@
           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 @@
             }
             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 @@
           // 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 @@
           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 @@
             // 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 @@
           }
         }
       } 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 @@
       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 @@
     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 @@
   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 @@
 
   SourceLocation RBraceLoc = MatchRHSPunctuation(tok::r_brace, LBraceLoc);
 
-  llvm::OwningPtr<AttributeList> 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 @@
     ConsumeCodeCompletionToken();
   }
   
-  llvm::OwningPtr<AttributeList> 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 @@
   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 @@
     SourceLocation IdentLoc = ConsumeToken();
 
     // If attributes exist after the enumerator, parse them.
-    llvm::OwningPtr<AttributeList> Attr;
+    AttributeList *Attr = 0;
     if (Tok.is(tok::kw___attribute))
-      Attr.reset(ParseGNUAttributes());
+      Attr = ParseGNUAttributes();
 
     SourceLocation EqualLoc;
     ExprResult AssignedVal;
@@ -2144,7 +2144,7 @@
     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 @@
   // Eat the }.
   SourceLocation RBraceLoc = MatchRHSPunctuation(tok::r_brace, LBraceLoc);
 
-  llvm::OwningPtr<AttributeList> 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 @@
   // In either case, we need to eat any attributes to be able to determine what
   // sort of paren this is.
   //
-  llvm::OwningPtr<AttributeList> 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 @@
   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 @@
     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 @@
   // 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 @@
 
   // 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 @@
     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

Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=118675&r1=118674&r2=118675&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Tue Nov  9 23:59:39 2010
@@ -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;

Modified: cfe/trunk/lib/Parse/ParseObjc.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseObjc.cpp?rev=118675&r1=118674&r2=118675&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseObjc.cpp (original)
+++ cfe/trunk/lib/Parse/ParseObjc.cpp Tue Nov  9 23:59:39 2010
@@ -833,9 +833,9 @@
     ReturnType = ParseObjCTypeName(DSRet, false);
 
   // If attributes exist before the method, parse them.
-  llvm::OwningPtr<AttributeList> 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 @@
   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 @@
                                           mType, IDecl, DSRet, ReturnType, Sel,
                                           0, 
                                           CParamInfo.data(), CParamInfo.size(),
-                                          MethodAttrs.get(),
-                                          MethodImplKind);
+                                          MethodAttrs, MethodImplKind);
     PD.complete(Result);
     return Result;
   }
@@ -970,8 +968,7 @@
   // 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 @@
                                         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<Sema::ObjCArgInfo>::iterator
-       I = ArgInfos.begin(), E = ArgInfos.end(); I != E; ++I)
-    delete I->ArgAttrs;
-  
   return Result;
 }
 

Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=118675&r1=118674&r2=118675&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Tue Nov  9 23:59:39 2010
@@ -84,7 +84,7 @@
   CXX0XAttributeList Attr;
   if (getLang().CPlusPlus0x && isCXX0XAttributeSpecifier())
     Attr = ParseCXX0XAttributes();
-  llvm::OwningPtr<AttributeList> 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 @@
   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 @@
   }
 
   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 @@
   }
 
   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 @@
   assert(Tok.is(tok::identifier) && Tok.getIdentifierInfo() &&
          "Not an identifier!");
 
-  llvm::OwningPtr<AttributeList> AttrList(Attr);
   Token IdentTok = Tok;  // Save the whole token.
   ConsumeToken();  // eat the identifier.
 
@@ -234,7 +232,7 @@
 
   // 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 @@
 
   return Actions.ActOnLabelStmt(IdentTok.getLocation(),
                                 IdentTok.getIdentifierInfo(),
-                                ColonLoc, SubStmt.get(), AttrList.take());
+                                ColonLoc, SubStmt.get(), Attr);
 }
 
 /// ParseCaseStatement
@@ -255,7 +253,6 @@
 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::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::ParseCompoundStatement(AttributeList *Attr,
                                                         bool isStmtExpr) {
   //FIXME: Use attributes?
-  delete Attr;
 
   assert(Tok.is(tok::l_brace) && "Not a compount stmt!");
 
@@ -586,7 +581,6 @@
 ///
 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 @@
 /// [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 @@
 /// [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 @@
 /// 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::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::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::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::ParseBreakStatement(AttributeList *Attr) {
   // FIXME: Use attributes?
-  delete Attr;
 
   SourceLocation BreakLoc = ConsumeToken();  // eat the 'break'.
   return Actions.ActOnBreakStmt(BreakLoc, getCurScope());
@@ -1202,7 +1189,6 @@
 ///         '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 @@
 ///
 StmtResult Parser::ParseCXXTryBlock(AttributeList* Attr) {
   // FIXME: Add attributes?
-  delete Attr;
 
   assert(Tok.is(tok::kw_try) && "Expected 'try'");
 

Modified: cfe/trunk/lib/Sema/AttributeList.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AttributeList.cpp?rev=118675&r1=118674&r2=118675&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/AttributeList.cpp (original)
+++ cfe/trunk/lib/Sema/AttributeList.cpp Tue Nov  9 23:59:39 2010
@@ -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<Expr*>(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();
 

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=118675&r1=118674&r2=118675&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Nov  9 23:59:39 2010
@@ -3483,7 +3483,6 @@
   }
 
   // FIXME: We ignore attributes for now.
-  delete AttrList;
   return UDir;
 }
 
@@ -3810,7 +3809,6 @@
   assert(IdentLoc.isValid() && "Invalid TargetName location.");
 
   // FIXME: We ignore attributes for now.
-  delete AttrList;
 
   if (SS.isEmpty()) {
     Diag(IdentLoc, diag::err_using_requires_qualname);

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=118675&r1=118674&r2=118675&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Tue Nov  9 23:59:39 2010
@@ -238,13 +238,12 @@
   // According to GCC docs, "the only attribute that makes sense after a label
   // is 'unused'".
   bool HasUnusedAttr = false;
-  llvm::OwningPtr<const AttributeList> 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);
     }
   }
 





More information about the cfe-commits mailing list