From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) by sourceware.org (Postfix) with ESMTPS id C5DBE3858D29; Fri, 17 Sep 2021 12:39:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C5DBE3858D29 Received: by mail-wm1-x330.google.com with SMTP id b21-20020a1c8015000000b003049690d882so9839588wmd.5; Fri, 17 Sep 2021 05:39:25 -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=DasyYFKjz30haT+Kjj/OJoXVlhGvgi4ijp2wr+Yhs+k=; b=nvm5SriY+5AF578b4ZJRbf60fW4DwldYK7FQsZKBxxnMNbod9KWiCrfcKI+wFM5eYI Emres4vMqqJ8jAcWSLqcV17rr6sjdmLcvfrN1VfX8FWvj1SVeGibuIbD+q0hl1xPxfxr wyAn8z74WFdh0sDDtr2tMYwN0gGYh8df4rr32URzqxEOO0Raq/XgdfMLUIusFIyR/34U 3UT/hV0JlRWcY7aNt+wvcd9RFGZlrUqt8Vg2YWr4QfCOS6OSM7/p8vfU9JveGhqqRhWQ 7jYfhyhektwg6faw7NU6/51sG+gwEY2sUTROQCuCOLtHmzJh9EcF6QzRuiqe4LQbSNPw li7A== X-Gm-Message-State: AOAM532cL7OBcEH4C19WiZv1Q3PU3kgd7WO9wea4GrTb0Hhmm+8lWAeA i54mFFONkbSUS0mL8FFrxON42hhI4UYvixDjGaA= X-Google-Smtp-Source: ABdhPJz64bszd7ECVH0+Wtarb4V1199h2v+1bPY0xt3x+64k9Iti+yUoquP9Z9sl78RO5nrLOrhcs/KNh8pgezH4RC4= X-Received: by 2002:a7b:c384:: with SMTP id s4mr10243218wmj.108.1631882364912; Fri, 17 Sep 2021 05:39:24 -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: Jonathan Wakely Date: Fri, 17 Sep 2021 13:39:13 +0100 Message-ID: Subject: Re: [PING^2] Re: Fix 'hash_table::expand' to destruct stale Value objects To: Richard Biener Cc: Thomas Schwinge , GCC Development , GCC Patches , Martin Sebor Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-0.7 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 12:39:27 -0000 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.