[llvm-commits] [llvm] r55210 - in /llvm/trunk: include/llvm/ADT/FoldingSet.h lib/Support/FoldingSet.cpp

[Ted Kremenek]

On Aug 22, 2008, at 11:53 PM, Ted Kremenek wrote:

>
> On Aug 22, 2008, at 8:18 PM, Chris Lattner wrote:
>
>> On Aug 22, 2008, at 7:12 PM, Dan Gohman wrote:
>>>> Hrm, I was pretty sure it did own the nodes, who else would?
>>>> However,
>>>> the dtor isn't freeing them.  Are they all getting leaked?  What
>>>> am I
>>>> forgetting here?
>>>
>>> Are you thinking of ilist? It does own its nodes, though there's a
>>> way to remove them and reclaim ownership, and SelectionDAG uses it,
>>> for example. SelectionDAG takes care of deaellocation of its nodes.
>>
>> I was pretty certain that foldingset owned all its nodes.  Should it
>> assert that it is empty on destruction?
>
> I'm not certain it makes sense to always have a FoldingSet "own" its
> contents.  First, it can contain a heterogenous group of random stuff,
> and second, there's no reason that the contents of a FoldingSet were
> created using standard new/delete.  To me it's just a container of
> pointers: std::vector<T*> doesn't invoke 'delete' on every pointer it
> contains.  The FoldingSet::iterator interface allows clients to delete
> the contents of a FoldingSet if they are so inclined.

An example of where this matters is in ImmutableMap.  The  
FoldingSetNodes are allocated using a BumpPtrAllocator.  The buckets  
of the FoldingSet are allocated with new/delete (per the internal  
implementation of FoldingSet).  The dstor of FoldingSet ensures that  
the memory used by the buckets is reclaimed; the dstor for the  
BumpPtrAllocator makes sure that the memory used by the  
FoldingSetNodes is reclaimed.  This kind of idiom appears over and  
over in the clang static analysis library as well.