Improve -Dy debugging #23591
Improve -Dy debugging #23591
Conversation
Could you provide a before/example of the effect this p.r. is intended to have? |
khwilliamson
force-pushed
the
tr_debugging
branch
from
806b51b to
9e2dd67
Compare
| DEBUG_y(PerlIO_printf(Perl_debug_log, | ||
| "%s: %d: Compiling tr/*t/*r/; /c=%d; /d=%d; /s=%d\n" | ||
| "*t is '%s'\n*r is '%s'\n", | ||
| __FILE__, __LINE__, complement, del, squash, |
There was a problem hiding this comment.
FILE is a CPP foldable "" literal on all OSes, skip the "%s" unless the C string literal in this debug fmt string has reuse somewhere else in libperl the binary.
There was a problem hiding this comment.
- You're trying to micro-optimize debugging output. Don't.
- Interpolating
__FILE__in the format string is not just harder to read, it also breaks if__FILE__contains a%. (In this case it doesn't, but why make the reader worry?)
| PerlIO_printf(Perl_debug_log, "TR_SPECIAL_HANDLING\n"); | ||
| } | ||
| else { | ||
| PerlIO_printf(Perl_debug_log, |
There was a problem hiding this comment.
Dont execute function Perl_debug_log multiple times in a row.
There was a problem hiding this comment.
Do you mean PerlIO_stderr()? Why not? (If your answer is performance, you're wrong.)
op.c
Outdated
| else if ((UV) tbl->map[tbl->size] == TR_SPECIAL_HANDLING) { | ||
| if (! del) { | ||
| const int size = tbl->size; | ||
| Perl_croak(aTHX_ "panic: Unexpected value %x in [%d]", |
There was a problem hiding this comment.
Use nocontext variant that doesn't require pushing a my_perl ptr from a c auto to a volatile outgoing C argument. Less instructions on a panic branch tat will never execute.
There was a problem hiding this comment.
Now you're trying to nano-optimize debugging code. Why?
op.c
Outdated
| else if ((UV) tbl->map[tbl->size] == TR_SPECIAL_HANDLING) { | ||
| if (! del) { | ||
| const int size = tbl->size; | ||
| Perl_croak(aTHX_ "panic: Unexpected value %x in [%d]", |
There was a problem hiding this comment.
This could be
| Perl_croak(aTHX_ "panic: Unexpected value %x in [%d]", | |
| croak("panic: Unexpected value %x in [%d]", |
There was a problem hiding this comment.
Thanks; this commit predates that ability.
It is undefined behavior in C to have a file-level symbol begin with an underscore. This is a static function; there is no need for it to have a leading or trailing underscore to mark it as internal
This creates an ARGS_ASSERT for this function. Previously, the code was using the one for plain pv_display(), which is kind of ugly. Now there is a macro for each function
The same printf can handle two things.
Commit 6ceb408 added a way to cleanly output UTF-8 tr/// values. This commit uses that to improve the debug output of compiling and running tr///. For a simple tr of of transliterating Greek capital letters to lowercase, the output of 'perl -Dy' has these added lines: > op.c: 6553: Compiling tr/*t/*r/; /c=0; /d=0; /s=0 > *t is '\x{391}-\x{3a9}' > *r is '\x{3b1}-\x{3c9}' Before the aforementioned commit the minus sign indicating a range would not have rendered properly; so things like that were omitted from the debug output. The output also now includes special mention of the special casing where the input is complemented, and/or some characters not being translated or get deleted.
While debugging this code, I found that the final generated map was unneeded and too large for most instances; so change that to not be output without the verbose option. Conversely, I found that I needed to often see the state of things after the first pass, so change to make that not verbose.
khwilliamson
force-pushed
the
tr_debugging
branch
from
9e2dd67 to
bff6937
Compare
Commit 6ceb408 added a way to cleanly output UTF-8 tr/// values. This commit uses that to improve the debug output of compiling and running tr///.