Tony Cook writes:
Approximately 54 tickets were reviewed, and 12 patches were applied.
[perl #126593] illustrates how some of perl's internal tools need to be careful of which parts of the language they use.
The tr/// operator can do its job in one of two ways, if all the code points are between 0 and 255 with a 256 entry table of shorts, otherwise using a swash, which is created by SWASHNEW in lib/utf8_heavy.pl.
tr/// uses the UTF-8 flag on the search and replacement strings to decide whether to use the look-up table or the swash, so it's possible for the swash to be used even when the search and replacement strings are representable as bytes.
aa8f6cef changed a s/// operator in lib/utf8_heavy.pl to a tr/// operator. All of the characters in the search and replacement strings can be represented as bytes - they're all ASCII range, so at first sight the implementation should be using the lookup table rather than the swash.
The problem is code that uses the deprecated ${^ENCODING} variable, in this case the encoding::warnings module. encoding::warnings sets ${^ENCODING} to a filter that warns (or croaks) when a non-UTF-8 marked PV with non-ASCII is used with UTF-8 marked PVs.1
When parsing string literals, including tr/// operators, S_scan_str() in toke.c always returns UTF-8 marked strings when ${^ENCODING} is true
The module that started [perl #125693] loads encoding::warnings, so ${^ENCODING} is now set, then Fatal, which loads Carp which includes the line:
$VERSION =~ tr/_//d;
so both the search and replacement strings are passed to S_pmtrans() (op.c) as UTF-8 marked strings.
S_pmtrans() attempts to create a swash, which starts to load utf8_heavy.pl, until we get to the line:
(my $loose = $_[0]) =~ tr/-_ \t//d;
where things break.
[1] the current recommended practice is that the UTF-8 flag controls internal representation only and combining two such strings isn't an issue. Don't use encoding::warnings.
Hours | Activity |
0.17 | #122251 review upstream tickets and resolve |
0.45 | #123710 testing, close |
2.55 | #123737 review discussion, testing |
#123737 debugging, produce a patch and comment | |
1.62 | #123991 produce a patch and comment |
2.51 | #124068 testing, fix issues, more testing |
#124068 another fix, testing, push to blead | |
1.15 | #124080 review, testing |
#124080 review test results, push to blead | |
2.32 | #124097 review code, try a fix |
#124097 review more code, comment with patch | |
#124097 testing, comment | |
0.83 | #124349 produce a patch |
0.75 | #125619 review discussion and comment |
0.40 | #125830 review cflags.SH changes, testing, apply to blead |
2.14 | #126042 research |
#126042 more research, comment with a simple patch | |
2.86 | #126045 review discussion, research, work on test patch |
#126045 more work on patch, comment with patch (and some | |
smartmatch irc discussion) | |
1.94 | #126193 review code, work on a patch, testing, comment with patch |
#126193 re-check patch, testing, apply to blead | |
1.27 | #126240 (camel issues), testing and comment |
0.52 | #126325 review patch, testing and apply to blead |
0.50 | #126368 review, test all three modules with blead and close |
0.53 | #126403 review and comment |
0.23 | #126437 research, comment |
2.32 | #126443 (sec) comment |
#126443 create a test, testing, comment with new patch | |
#126443 testing, apply to blead | |
0.57 | #126469 review, re-test and apply to blead |
1.37 | #126474 comment |
#126474 research, comment | |
1.55 | #126480 produce a patch and comment |
#126480 minor corrections to patch, push a smoke-me | |
#126480 testing, apply to blead | |
0.88 | #126502 testing |
#126502 more testing, start bisect | |
#126502 comment | |
1.08 | #126533 review and comment |
#126533 test and apply to blead | |
0.60 | #126534 review, test and apply |
1.03 | #126544 research, comment |
0.40 | #126546 fuzzer tickets meta-ticket |
0.60 | #126552 testing, debugging |
5.59 | #126593 debugging, research, longer comment |
#126593 review discussion, try some fixes | |
#126593 comment with patch | |
2.03 | #126602 review, produce a patch and comment |
#126602 re-check patch, testing, apply to blead | |
0.50 | #126608 review and comment |
0.43 | #126609 review, test and apply to blead |
0.76 | #126611 review and comment |
#126611 review latest patch, testing, apply to blead | |
3.13 | #126621 review, code, prep for testing, reproduce, review |
failing tests, work on simple reproducer | |
#126621 comment, try to work out a fix | |
0.15 | #126632 review, requires CPAN updates, so leave |
2.17 | #126633 review failing code, review patch, make a simple reproducer |
#126633 try to make a simpler reproducer, try a simple | |
patch and comment | |
1.93 | #126635 reproduce, review code, produce a patch and comment |
#126635 produce alternate patch and comment | |
#126635 testing, apply to blead | |
0.98 | #126707 review change, research, comment |
1.25 | make_ext.pl silent handling, notice unneeded rebuilds and |
open #126710 | |
1.67 | #126719 debugging, open Encode ticket, comments |
0.32 | #126731 review code, testing, find dup and comment, merge tickets |
3.64 | #126755 (sec) try to map MapPathW crash, working up test code |
#126755 (sec) more test code, testing, re-working supplied fix | |
0.35 | 5.24.0 blocker updates |
7.01 | #57512 review discussion, look over code |
#57512 more look over code, testing | |
#57512 research, code on in-place edit close failures | |
#57512 more code, testing, comment with patches | |
2.78 | cygwin build warnings - testing, seem to be 32-bit |
generic, testing, fixes | |
0.32 | cygwin failures – re-test original fix and apply to blead |
0.48 | jhi's scandir thread – research and comment |
0.63 | khw's cygwin issues |
0.28 | os390 dynaloader issue: fix an else nesting bug |
0.65 | rjbs's redefinition warning strangeness: irc and reply to list |
0.93 | smartmatch review overloading code |
2.47 | smartmatch testing, work on overload.t failures |
2.08 | smartmatch: fix overload.t tests, more testing, push to branch |
2.72 | smartmatch: rebase, work on overloading |
78.39 Hours Total
Leave a comment