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

[Trevor Saunders <tbsaunde at tbsaunde dot org>]

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.

Trev

> 
> Thanks,
> Richard