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

Aktemur, Tankut Baris via Gdb-patches
2021-04-22T07:49:37Z

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