This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 1/2] destroy values as well as keys when removing them from hash maps
- From: Richard Biener <rguenther at suse dot de>
- To: Trevor Saunders <tbsaunde at tbsaunde dot org>
- Cc: tbsaunde+gcc at tbsaunde dot org, gcc-patches at gcc dot gnu dot org, rdsandiford at googlemail dot com
- Date: Wed, 2 Dec 2015 09:57:47 +0100 (CET)
- Subject: Re: [PATCH 1/2] destroy values as well as keys when removing them from hash maps
- Authentication-results: sourceware.org; auth=none
- References: <1448318933-23235-1-git-send-email-tbsaunde+gcc at tbsaunde dot org> <87wpsy3qfs dot fsf at googlemail dot com> <20151202050346 dot GA29495 at tsaunders-iceball dot corp dot tor1 dot mozilla dot com>
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. 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.
> Trev
>
> >
> > Thanks,
> > Richard
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)