From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 59284 invoked by alias); 2 Dec 2015 09:35:22 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 59263 invoked by uid 89); 2 Dec 2015 09:35:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (207.82.80.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 02 Dec 2015 09:35:19 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-14-0P-kSpbTRN2B_oDulazQYQ-1; Wed, 02 Dec 2015 09:35:14 +0000 Received: from localhost ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 2 Dec 2015 09:35:13 +0000 From: Richard Sandiford To: Richard Biener Mail-Followup-To: Richard Biener ,Trevor Saunders , tbsaunde+gcc@tbsaunde.org, gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Cc: Trevor Saunders , tbsaunde+gcc@tbsaunde.org, gcc-patches@gcc.gnu.org Subject: Re: [PATCH 1/2] destroy values as well as keys when removing them from hash maps References: <1448318933-23235-1-git-send-email-tbsaunde+gcc@tbsaunde.org> <87wpsy3qfs.fsf@googlemail.com> <20151202050346.GA29495@tsaunders-iceball.corp.tor1.mozilla.com> Date: Wed, 02 Dec 2015 09:35:00 -0000 In-Reply-To: (Richard Biener's message of "Wed, 2 Dec 2015 09:57:47 +0100 (CET)") Message-ID: <877fkxchwu.fsf@e105548-lin.cambridge.arm.com> User-Agent: Gnus/5.130012 (Ma Gnus v0.12) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-MC-Unique: 0P-kSpbTRN2B_oDulazQYQ-1 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2015-12/txt/msg00216.txt.bz2 Richard Biener 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 >> > > +template >> > > template >> > > inline void >> > > -simple_hashmap_traits ::remove (T &entry) >> > > +simple_hashmap_traits ::remove (T &entry) >> > > { >> > > H::remove (entry.m_key); >> > > + entry.m_value.~Value (); >> > > } >> >=20 >> > This is just repeating my IRC comment really, but doesn't this mean th= at >> > 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. >>=20 >> 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 =3D ...; 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.