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 wrote: >> > > 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, [...] >> > > > 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. >> 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, pushed to master branch commit 89be17a1b231ade643f28fbe616d53377e069da8 "Fix 'hash_table::expand' to destruct stale Value objects". Should this be backported to release branches, after a while? Grüße Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955