On Wed, Dec 02, 2015 at 09:35:13AM +0000, Richard Sandiford wrote:
> Richard Biener <rguent...@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+...@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.
It seems possible though it seems rather unlikely since you'd have to
insert a new element with the same hash as the one you just removed,
and the compiler would have to figure that out. On the other hand I
suppose the existing code may already be invalid since it operates on
objects without constructing them.
Trev
>
> 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.
>
