[PATCH] ADT: Add SmallPtrSet::reset()

Duncan P. N. Exon Smith dexonsmith at apple.com
Thu Mar 19 17:50:33 PDT 2015


> On 2015-Mar-19, at 16:52, David Blaikie <dblaikie at gmail.com> wrote:
> 
> Totally plausible. Optional/debatable comments:
> 
> 1) is it worth supporting the more idiomatic clear+shrink_to_fit to be clear of the semantics? ('reset' seems vague) It looks like it's sort of halfway there already, with shrink_and_clear. Could just be generalized a bit more to be part of the solution to this use case

`reset()` is used with smart pointers to mean "like it's default
constructed", so that's where I was going with it.  I realize this
isn't related to pointers though :).

I hadn't considered `shrink_to_fit()`, and at first blush I liked it
better than what I chose.

However, unlike `std::vector<>` where `clear()` is "free",
`SmallPtrSet<>::clear()` is going to `malloc()` a new array.  I guess
another option is to change `clear()` to shrink down to small storage
(a bigger change, but it saves a malloc).  Thoughts on that approach?

If I decide to stick with the current model, do you have any better
names than `reset()`?

> 2) I would perhaps consider testing via looking at the address of an element added after calling reset, rather than using isSmall, but I realize that's totally subjective (& reactive - perhaps if I wrote it, I would've written it as you have)

Right, right.  I was being lazy here...

> 
> On Thu, Mar 19, 2015 at 4:43 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 0001 adds `SmallPtrSet::reset()`, which returns `SmallPtrSet` to small
> mode after completely freeing its heap memory.  To facilitate testing,
> I changed `isSmall()` to a public accessor.
> 
> Thoughts?
> 
> (0002 is the reason I was looking at this.)
> 
> 
> 





More information about the llvm-commits mailing list