[cfe-commits] r101194 - /cfe/trunk/include/clang/AST/ASTVector.h

[Ted Kremenek]

I agree that a 'vector' shouldn't be used at all with InitListExpr.  My immediate interest was plugging the memory leak caused by using 'std::vector' without waiting for such a rewrite.  If we reach a point where vectors are not present in our AST elements, I think ASTVector should probably just get ripped out.

On Apr 14, 2010, at 7:38 AM, Douglas Gregor wrote:

> 
> On Apr 14, 2010, at 4:59 AM, Benjamin Kramer wrote:
> 
>> 
>> On 14.04.2010, at 01:39, Ted Kremenek wrote:
>> 
>>> Author: kremenek
>>> Date: Tue Apr 13 18:39:09 2010
>>> New Revision: 101194
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=101194&view=rev
>>> Log:
>>> Introduce ASTVector, which is a std::vector-like class that allocates all memory
>>> using the allocator associated with an ASTContext.  This is largely copy-and-paste
>>> from SmallVector, and should be refactored one day.
>> 
>> I was thinking about this before and came to the conclusion that a BumpPtrAllocator + vector is a
>> bad combination. Every time the vector resizes the old memory isn't freed and adds up. I guess
>> it doesn't matter in smaller cases but it could become a huge memory waste if we're not checking every
>> ASTVector's insert characteristics carefully. In my opinion a list-like data structure should be
>> preferred (where applicable).
> 
> 
> Yeah, I agree. The preferred usage for BumpPtrAllocators should either be node-base data structures (list, graph, DAG, whatever) or non-resizable chunks of memory. The best way to tackle InitListExpr, for example, would be to compute the full set of initializers within SemaInit.cpp and then allocate them in one big block via the BumpPtrAllocator. 
> 
> IMO, "resizing" should not be part of the AST.
> 
> 	- Doug