signatures: generate only one debug breakable line for signatures #22610
Conversation
op.h
Outdated
| @@ -171,6 +171,13 @@ Deprecated. Use C<GIMME_V> instead. | |||
| op_convert_list to set op_folded. */ | |||
| #define OPf_FOLDED (1<<16) | |||
|
|
|||
| /* force newSTATEOP() to make an OP_NEXTSTATE. | |||
There was a problem hiding this comment.
I would like to see: "... as opposed to an OP_DBSTATE"
There was a problem hiding this comment.
"make an OP_NEXTSTATE even when in debugging mode and a OP_DBSTATE would normally be produced"
There was a problem hiding this comment.
I reworded it a bit more than that.
| print $x1, $x2, $x3, $x4; | ||
| } | ||
| four(1,2,3,4); | ||
| EOS |
There was a problem hiding this comment.
If I save this heredoc as a separate program, what differences should I be able to observe in the behavior of the debugger between, say, perl-5.40.0 and blead+this patch?
There was a problem hiding this comment.
If you single-step (s) in the debugger, you should see a lot of invisible statements being executed on the sub four($x1, $x2, $x3, $x4) { line; i.e. you have to repeat s a lot to make the debugger advance into the subroutine.
I haven't tried it, but with this patch I'd assume the debugger only stops once and immediately proceeds to the print in the subroutine body when you hit s.
There was a problem hiding this comment.
If you single-step (
s) in the debugger, you should see a lot of invisible statements being executed on thesub four($x1, $x2, $x3, $x4) {line; i.e. you have to repeatsa lot to make the debugger advance into the subroutine.I haven't tried it, but with this patch I'd assume the debugger only stops once and immediately proceeds to the
s.
Thanks. In perl-5.40.0, It took me 8 s steps before the print statement executed. In the p.r., it only took 3 s steps. LGTM.
There was a problem hiding this comment.
Step through the code with "s", pre-patch this steps 6 times on the sub prolog, with patch it steps once
op.h
Outdated
| @@ -171,6 +171,13 @@ Deprecated. Use C<GIMME_V> instead. | |||
| op_convert_list to set op_folded. */ | |||
| #define OPf_FOLDED (1<<16) | |||
|
|
|||
| /* force newSTATEOP() to make an OP_NEXTSTATE. | |||
There was a problem hiding this comment.
"make an OP_NEXTSTATE even when in debugging mode and a OP_DBSTATE would normally be produced"
| @@ -913,7 +913,7 @@ sigscalarelem: | |||
| "follows optional parameter"); | |||
| } | |||
|
|
|||
| $$ = var ? newSTATEOP(0, NULL, var) : NULL; | |||
| $$ = var ? newSTATEOP(OPf_FORCE_NEXTSTATE, NULL, var) : NULL; | |||
There was a problem hiding this comment.
This means that in the debugger it now won't be single-steppable even if there is a defaulting expression. It's hard to predict what users might want here, but maybe it's best to keep the DBSTATE if there is a defop? So maybe
defop ? 0 : OPf_FORCE_NEXTSTATEfor the flags?
There was a problem hiding this comment.
That would get annoying in the same way as in #22602 if you had more than a couple of default valued arguments.
I think the ideal experience would be to make the debugger display each argument at the signature step points as you step through the signature.
Maybe these breakpoints could be optional, but it would need to be controllable at runtime (ie. need a flag on the dbstate), and I expect discoverability of such an option would be bad.
I'm not sure of a good approach for the debugger to get the variable set by the argelem.
With the debugger enabled the argcheck op and each argelem op generated to process a function signature would have an associated dbstate op, allowing the debugger to step on that op. This can be confusing and/or onerous when stepping through code, since for a signature with 7 arguments you'd have 9 steps to get past the prologue of the sub. Make most of these dbstates into nextstates to make them non-steppable. Fixes Perl#22602
tonycoz
force-pushed
the
22602-debug-signatures
branch
from
1c2da39
to
65d14c5
Compare
With the debugger enabled the argcheck op and each argelem op generated to process a function signature would have an associated dbstate op, allowing the debugger to step on that op.
This can be confusing and/or onerous when stepping through code, since for a signature with 7 arguments you'd have 9 steps to get past the prologue of the sub.
Make most of these dbstates into nextstates to make them non-steppable.
Fixes #22602