[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