From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com [IPv6:2a00:1450:4864:20::52c]) by sourceware.org (Postfix) with ESMTPS id CBA313858433; Fri, 17 Sep 2021 13:03:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CBA313858433 Received: by mail-ed1-x52c.google.com with SMTP id z94so29611847ede.8; Fri, 17 Sep 2021 06:03:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ADZcAzgs1iwOI6gXsAhJY8lYXougMDgERdJPpCEqnDs=; b=7l7nJ+1YnW8qpyC7N/P1XnDLv8iFy3+rJhpOFjHby0qRo/r1TI52kbKgJNCVJqy0ID z/dNXvPU5xaNlRBCN08pVxSl98P+AtEPeWB7vK6vp+Q32xJ6kUOmT5vWSzwAdQB1WIim IoAQrGqcuD5zbgz8ATEJz2dxMZ0vtnLhEC3B/B0znjMfEeEjei8NBNIIGgYsrKqiDIFc 067LU2IlNxu/9yqiH8/3DLjQQOKnKPZXmdVjUQTPqqE1IhhHk7/WXX8TFNmDJXvzYLtJ aHYvmgtZMw5CwZ87BOkWuvyMc4toiu3GwxCrj5hgoCKqB6sHSreNEe9m8mF6/ZEdQOAl Aufg== X-Gm-Message-State: AOAM5337KxCghEk1Mc2ClK8J6IH/3d+bse4OzesriuORCr4rv05kdXuI m9CFeuoILc5JjtdxHGz7KkE5gVEEaDTvLhY45aU= X-Google-Smtp-Source: ABdhPJx8SwUJCoOi4+cxho1qGL2140MhRiG++Z2TRp4ouegXGuRIfc4jjVFpZVz8li+iBirC+ZKRor7NQ5UaclcRrQ4= X-Received: by 2002:a50:cfc1:: with SMTP id i1mr5713494edk.251.1631883809421; Fri, 17 Sep 2021 06:03:29 -0700 (PDT) MIME-Version: 1.0 References: <87r1f6qzmx.fsf@euler.schwinge.homeip.net> <87czqd7e4k.fsf@euler.schwinge.homeip.net> <55126f14-dda1-2040-0976-70b89582a60c@gmail.com> <87h7f7mcqf.fsf@euler.schwinge.homeip.net> <60f2a6ac-764f-470e-2807-acc19c5038a8@gmail.com> <87mtokuag6.fsf@euler.schwinge.homeip.net> <87v92za1t2.fsf@euler.schwinge.homeip.net> In-Reply-To: From: Richard Biener Date: Fri, 17 Sep 2021 15:03:18 +0200 Message-ID: Subject: Re: [PING^2] Re: Fix 'hash_table::expand' to destruct stale Value objects To: Jonathan Wakely Cc: Thomas Schwinge , GCC Development , GCC Patches , Martin Sebor Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Sep 2021 13:03:32 -0000 On Fri, Sep 17, 2021 at 2:39 PM Jonathan Wakely wrote: > > On Fri, 17 Sept 2021 at 13:08, Richard Biener > wrote: > > > > On Fri, Sep 17, 2021 at 1:17 PM Thomas Schwinge wrote: > > > > > > Hi! > > > > > > On 2021-09-10T10:00:25+0200, I wrote: > > > > On 2021-09-01T19:31:19-0600, Martin Sebor via Gcc-patches wrote: > > > >> On 8/30/21 4:46 AM, Thomas Schwinge wrote: > > > >>> Ping -- we still need to plug the memory leak; see patch attached, and/or > > > >>> long discussion here: > > > >> > > > >> Thanks for answering my questions. I have no concerns with going > > > >> forward with the patch as is. > > > > > > > > Thanks, Martin. Ping for formal approval (and review for using proper > > > > C++ terminology in the 'gcc/hash-table.h:hash_table::expand' source code > > > > comment that I'm adding). Patch again attached, for easy reference. > > > > > > Ping, once again. > > > > I'm happy when a C++ literate approves the main change which I quote as > > > > new ((void*) q) value_type (std::move (x)); > > + > > + /* Manually invoke destructor of original object, to counterbalance > > + object constructed via placement new. */ > > + x.~value_type (); > > > > but I had the impression that std::move already "moves away" from the source? > > It just casts the argument to an rvalue reference, which allows the > value_type constructor to steal its guts. > > > That said, the dance above looks iffy, there must be a nicer way to "move" > > an object in C++? > > The code above is doing two things: transfer the resources from x to a > new object at location *q, and then destroy x. > > The first part (moving its resources) has nothing to do with > destruction. An object still needs to be destroyed, even if its guts > have been moved to another object. > > The second part is destroying the object, to end its lifetime. You > wouldn't usually call a destructor explicitly, because it would be > done automatically at the end of scope for objects on the stack, or > done by delete when you free obejcts on the heap. This is a special > case where the object's lifetime is manually managed in storage that > is manually managed. > > > > > What happens if the dtor is deleted btw? > > If the destructor is deleted you have created an unusable type that > cannot be stored in containers. It can only be created using new, and > then never destroyed. If you play stupid games, you win stupid prizes. > Don't do that. > > > Shouldn't you use sth > > like a placement 'delete' instead of invoking a DTOR? > > No, there is no placement delete. This is exactly the right way to > destroy an object in-place. > > I haven't read the rest of the patch, but the snippet above looks fine. OK, thanks for clarifying. The patch is OK then. Thanks, Richard.