[PATCH 1/2] destroy values as well as keys when removing them from hash maps

Richard Sandiford richard.sandiford@arm.com
Wed Dec 2 09:35:00 GMT 2015


Richard Biener <rguenther@suse.de> writes:
> On Wed, 2 Dec 2015, Trevor Saunders wrote:
>> On Tue, Dec 01, 2015 at 07:43:35PM +0000, Richard Sandiford wrote:
>> > tbsaunde+gcc@tbsaunde.org writes:
>> > > -template <typename H>
>> > > +template <typename H, typename Value>
>> > >  template <typename T>
>> > >  inline void
>> > > -simple_hashmap_traits <H>::remove (T &entry)
>> > > +simple_hashmap_traits <H, Value>::remove (T &entry)
>> > >  {
>> > >    H::remove (entry.m_key);
>> > > +  entry.m_value.~Value ();
>> > >  }
>> > 
>> > This is just repeating my IRC comment really, but doesn't this mean that
>> > we're calling the destructor on an object that was never constructed?
>> > I.e. nothing ever calls placement new on the entry, the m_key, or the
>> > m_value.
>> 
>> I believe you are correct that placement new is not called.  I'd say its
>> a bug waiting to happen given that the usage of auto_vec seems to
>> demonstrate that people expect objects to be initialized and destroyed.
>> However for now all values are either POD, or auto_vec and in either
>> case the current 0 initialization has the same effect as the
>> constructor.  So There may be a theoretical problem with how we
>> initialize values that will become real when somebody adds a constructor
>> that doesn't just 0 initialize.  So it should probably be improved at
>> some point, but it doesn't seem necessary to mess with it at this point
>> instead of next stage 1.
>
> Agreed.

OK.  I was just worried that (IIRC) we had cases where for:

     a.~foo ()
     a.x = ...;

the assignment to a.x was removed as dead since the object had been
destroyed.  Maybe that could happen again if we don't have an explicit
constructor to create a new object.

Thanks,
Richard

> You'll also need a more elaborate allocator/constructor
> scheme for this considering the case where no default constructor
> is available.  See how alloc-pool.h tries to dance around this
> using a "raw" allocate and a operator new...
>
> Richard.



More information about the Gcc-patches mailing list