Igor, Sure you can proceed with the push. Sergos. > On 22 Jul 2021, at 14:48, Igor Munkin wrote: > > Sergos, > > Thanks for your review! TL;DR: I would rather leave the patch with no > changes, but please consider my comments regarding your alternative > approach for iterating through the guest stack. > > On 22.07.21, Sergey Ostanevich wrote: >> Hi! >> >> Thanks for the patch! >> >> I would rather remove all mentions of ‘bottom loop’ as we named it at >> Intel. I'm not a big pro in python, still the ‘for’ loop works fine >> for me: >> >> If we can unwind to the next frame - dump it (and further) >> >>>>> s=[1] >>>>> for i in s: >> ... if (i==1): >> ... s.append(2) >> ... continue >> ... print(i) >> ... >> 2 >> >> If it is the only one (dummy): >> >>>>> s=[3] >>>>> for i in s: >> ... if (i==1): >> ... s.append(2) >> ... continue >> ... print(i) >> ... >> 3 >> >> Does it make sense in this case? > > Let's see what would we get for our case with guest stack unwinding. The > algorithm is described by Sergey in the comment near the loop. But > imagine, we are iterating (backwards) through [L->stack, L-top] segment > via for loop (as you suggest above). There is one condition to check > whether we have reached the bottom of the frame (dump framelink slot and > continue the same way you showed above). However, we need one more > condition to skip the second slot for the framelink in LJ_FR2 mode. As > a result we have a loop that iterates through the stack with at least > two conditions to skip some slots or dump them in other way. > Furthermore, your proposal leads to much more complex change in the > extension, comparing to those made by Sergey. > > As I've already said, postcondition loop is the classic way for this > case. Moreover, I googled "how to emulate a do-while loop in Python?" > and the second option is unrolling the first iteration[1]. If one > wonders what is the first option -- use "while-true" loop with the > conditional break at the end. I equally hate both, so I'm OK with the > one chosen by Sergey. If you prefer "while-true", I believe Sergey can > rewrite this part, but considering your LGTM below, I guess you're OK > also with the current solution. So, I'll push the patch in a jiffy. > >> >> Anyways, it’s too much of nitpicking - LGTM in its current version. >> >> Regards, >> Sergos >> >> > > > >> > > [1]: https://stackoverflow.com/a/743186 > > -- > Best regards, > IM