RE: [PATCH 5/6] gdb/continuations: use lambdas instead of function pointers

["Aktemur, Tankut Baris via Gdb-patches" <gdb-patches@sourceware.org>]

On Wednesday, April 21, 2021 9:44 PM, Tom Tromey wrote:
> >>>>> ">" == Tankut Baris Aktemur via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> >> Use lambdas and std::list to track inferior continuations.  This is a
> >> refactoring.
> 
> Thanks for the patch.
> 
> >> @@ -122,7 +39,12 @@ void
> >>  do_all_inferior_continuations ()
> >>  {
> >>    struct inferior *inf = current_inferior ();
> >> -  do_my_continuations (&inf->continuations);
> >> +  while (!inf->continuations.empty ())
> >> +    {
> >> +      auto &cont = inf->continuations.front ();
> >> +      inf->continuations.pop_front ();
> >> +      cont ();
> 
> It seems like this sequence can lead to 'cont' referencing an invalid
> function, because 'cont' is just a reference, but pop_front will cause
> it to be destroyed.

Ah, thanks for catching that.

> Maybe "auto cont = std::move (...)" would work ok.
> Or, delaying the pop until after the callback is called seems fine as
> well.

The continuations at the moment are rather plain, but just in case a
continuation in the future adds more continuations, delaying the pop
could cause a wrong element to be popped, I'm afraid.  How about the
following? (The same is to be used in Patch 6/6, too).

@@ -122,7 +39,12 @@ void
 do_all_inferior_continuations ()
 {
   struct inferior *inf = current_inferior ();
-  do_my_continuations (&inf->continuations);
+  while (!inf->continuations.empty ())
+    {
+      auto iter = inf->continuations.begin ();
+      (*iter) ();
+      inf->continuations.erase (iter);
+    }
 }


> >> +   A continuation is a closure (i.e.  a lambda) to be called to finish
> 
> I think it would be fine to say "A continuation is a std::function"
> here, because that encompasses lambdas but also other valid forms, like
> ordinary functions.

Out of curiosity, how do you pronounce 'std'?  I was just thinking if it
should be "a std::function" or "an std::function".  I pronounce std as
"ess-tee-dee" and "an" sounded more natural to me.

> >> +  std::list<std::function<void ()> > continuations;
> 
> gdb style usually uses the ">>" token instead of adding the space here.
> 
> thanks,
> Tom

Fixed.

Thanks
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928