Trevor Saunders <tbsau...@tbsaunde.org> writes:
> On Thu, Jul 02, 2015 at 09:09:31PM +0100, Richard Sandiford wrote:
>> Martin Liška <mli...@suse.cz> writes:
>> > diff --git a/gcc/asan.c b/gcc/asan.c
>> > index e89817e..dabd6f1 100644
>> > --- a/gcc/asan.c
>> > +++ b/gcc/asan.c
>> > @@ -362,20 +362,20 @@ struct asan_mem_ref
>> > /* Pool allocation new operator. */
>> > inline void *operator new (size_t)
>> > {
>> > - return pool.allocate ();
>> > + return ::new (pool.allocate ()) asan_mem_ref ();
>> > }
>> >
>> > /* Delete operator utilizing pool allocation. */
>> > inline void operator delete (void *ptr)
>> > {
>> > - pool.remove ((asan_mem_ref *) ptr);
>> > + pool.remove (ptr);
>> > }
>> >
>> > /* Memory allocation pool. */
>> > - static pool_allocator<asan_mem_ref> pool;
>> > + static pool_allocator pool;
>> > };
>>
>> I'm probably going over old ground/wounds, sorry, but what's the benefit
>> of having this sort of pattern? Why not simply have object_allocators
>> and make callers use pool.allocate () and pool.remove (x) (with pool.remove
>> calling the destructor) instead of new and delete? It feels wrong to me
>> to tie the data type to a particular allocation object like this.
>
> Well the big question is what does allocate() do about construction? if
> it seems wierd for it to not call the ctor, but I'm not sure we can do a
> good job of forwarding args to allocate() with C++98.
If you need non-default constructors then:
new (pool) type (aaa, bbb)...;
doesn't seem too bad. I agree object_allocator's allocate () should call
the constructor.
> However it seems kind of wierd the operator new here is calling the
> placement new on the object it allocates.
Yeah.
>> And using the pool allocator functions directly has the nice property
>> that you can tell when a delete/remove isn't necessary because the pool
>> itself is being cleared.
>
> Well, all these cases involve a pool with static storage lifetime right?
> so actually if you don't delete things in these pool they are
> effectively leaked.
They might have a static storage lifetime now, but it doesn't seem like
a good idea to hard-bake that into the interface (by saying that for
these types you should use new and delete, but for other pool-allocated
types you should use object_allocators). Maybe I just have bad memories
from doing the SWITCHABLE_TARGET stuff, but there I was changing a lot
of state that was "obviously" static in the old days, but that needed
to become non-static to support vaguely-efficient switching between
different subtargets. The same kind of thing is likely to happen again.
I assume things like the jit would prefer not to have new global state
with load-time construction.
Thanks,
Richard
