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

[Chris Lattner]

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,

That's no problem, a virtual dtor would be enough to handle that fine.
>
> 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.

Ok, ok, you're right :)

-Chris