From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x432.google.com (mail-wr1-x432.google.com [IPv6:2a00:1450:4864:20::432]) by sourceware.org (Postfix) with ESMTPS id DCBC0385840D; Fri, 17 Sep 2021 19:09:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DCBC0385840D Received: by mail-wr1-x432.google.com with SMTP id u18so15030185wrg.5; Fri, 17 Sep 2021 12:09:08 -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=ELtx3tkEhey77Lgma8oBbJEuvQcWsCDq59E8JUybreE=; b=eHpi84qf78eYk+0VP/R6xc3Q6gRdNSTilhLFEQ9Ta3MSKE7whcTzEwhJSsnjUGp1HR KCeFDHKGLctx42dLi0gR9iXvW9R+/cAjzNFPVpAj+h7yxRZpjvbJimMr0H3A+nOtN7vf vBoelVV/9bDBxawifzSYGMCSd76zgwd0/Kq5YVccat7b7gmJe/JVEytfhK/BT/ew0m7r /YnrigjRJNY/pjuJSVAcgtTdqbqnN6S+FBvAkxagY5sl3lsduu8xZnetwPIj6ja5UOuN H2IKUGo+RJVYO90GyEovfHBxlj8oW2Q7a/Qkb5FfE2XCSSaOEpKIxTC18xnZzW7AqFB0 u18A== X-Gm-Message-State: AOAM530udhFdVEnxUNtgK7W6cXvEwnZgNotV3tCSPYRey6K1+iAJssPo jRIUtlOgm9fmnYDSK4I0S1inW16hfUo8gWZU0p8= X-Google-Smtp-Source: ABdhPJz+iVagK3HY5FYnOYgVTdPSbMgcc+qoLoVzZSV3S1CX29EP2Jfmk4a9bkstVusE5qsAZHqzROTBTYyh/rapC/U= X-Received: by 2002:a5d:64ab:: with SMTP id m11mr11785748wrp.343.1631905747976; Fri, 17 Sep 2021 12:09:07 -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> <87sfy39p38.fsf@euler.schwinge.homeip.net> In-Reply-To: <87sfy39p38.fsf@euler.schwinge.homeip.net> From: Jonathan Wakely Date: Fri, 17 Sep 2021 20:08:52 +0100 Message-ID: Subject: Re: [PING^2] Re: Fix 'hash_table::expand' to destruct stale Value objects To: Thomas Schwinge Cc: Richard Biener , gcc-patches , "gcc@gcc.gnu.org" , Martin Sebor X-Spam-Status: No, score=-0.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, HTML_MESSAGE, 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 Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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 19:09:12 -0000 On Fri, 17 Sep 2021, 16:52 Thomas Schwinge, wrote: > Hi! > > On 2021-09-17T15:03:18+0200, Richard Biener > wrote: > > 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 < > thomas@codesourcery.com> wrote: > >> > > On 2021-09-10T10:00:25+0200, I wrote: > >> > > > On 2021-09-01T19:31:19-0600, Martin Sebor via Gcc-patches < > gcc-patches@gcc.gnu.org> wrote: > >> > > >> On 8/30/21 4:46 AM, Thomas Schwinge wrote: > >> > > >>> Ping -- we still need to plug the memory leak; see patch > attached, [...] > > >> > > > 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. > > >> > 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. > > ACK, and happy that I understood this correctly. > > And, thanks for providing some proper C++-esque wording, which I > summarized to replace my original source code comment. > > >> > 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. > > Haha! ;-) > > And, by the way, as I understood this: if the destructor is "trivial" > (which includes POD types, for example), the explicit destructor call > here is a no-op. > Right. And you can even do x.~value_type(); for things which aren't classes and don't have any destructor at all, not even a trivial one. So in a template function, if the template argument T is int or char or long*, it's ok to do t.~T(). This is called a pseudo-destructor call (because scalar types like int don't actually have a destructor). This will also be a no-op. This allows you to write the same template code for any types* and it will correctly destroy them, whether they have a non-trivial destructor that does real work, or a trivial one, or if they are not even classes and have no destructor at all. * Well, nearly any types... You can't do it if the destructor is deleted, as Richi asked about, or private, and you can't do it for non-object types (references, functions, void) but that's ok because you can't store them in a container anyway.