Closed andk closed 2 years ago
Also affected: BRUMMETT/Devel-Chitin-0.18.tar.gz
Description
Commit v5.35.8-110-gbb5bc97fde/da1c (bb5bc9) stopped also GRIAN/Storable-AMF-1.23.tar.gz from succeeding with the test suite. We had this commit already in #19450 ; that issue has been closed, but the fix provided over there does not help for Storable::AMF 1.23 ; you can already find sample fail reports on http://matrix.cpantesters.org/?dist=Storable-AMF+1.23
[snip]
Failures confirmed with the following recent build on FreeBSD-12:
$ bleadperl -v | head -2 | tail -1
This is perl 5, version 35, subversion 10 (v5.35.10 (v5.35.9-7-g8432359dfe)) built for amd64-freebsd-thread-multi
$ bleadprove -vb t/64-prefer-number.t t/65-prefer-number3.t
t/64-prefer-number.t ...
1..44
ok 1 - int constant
ok 2 - double constant
[snip]
ok 12 - str var not changed
not ok 13 - Int converted is a string
ok 14 - Int 0+converted is double
ok 15 - Int 0.0+converted is double
ok 16 - Int converted again is double
not ok 17 - Int++ converted is a string
# Failed test 'Int converted is a string'
# at t/64-prefer-number.t line 37.
# Failed test 'Int++ converted is a string'
# at t/64-prefer-number.t line 47.
ok 18 - Int++ 0+converted is double
ok 19 - Int++ 0.0+converted is double
[snip]
ok 44 - Str 1.0 ''.converted is str
# Looks like you failed 2 tests of 44.
Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/44 subtests
t/65-prefer-number3.t ..
1..44
ok 1 - int constant
ok 2 - double constant
ok 3 - str constant
[snip]
ok 12 - str var not changed
not ok 13 - Int converted is a string
# Failed test 'Int converted is a string'
# at t/65-prefer-number3.t line 37.
ok 14 - Int 0+converted is double
ok 15 - Int 0.0+converted is double
ok 16 - Int converted again is double
not ok 17 - Int++ converted is a string
# Failed test 'Int++ converted is a string'
# at t/65-prefer-number3.t line 47.
ok 18 - Int++ 0+converted is double
ok 19 - Int++ 0.0+converted is double
ok 20 - Int++ converted again is double
[snip]
ok 44 - Str 1.0 ''.converted is str
# Looks like you failed 2 tests of 44.
Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/44 subtests
Test Summary Report
-------------------
t/64-prefer-number.t (Wstat: 512 Tests: 44 Failed: 2)
Failed tests: 13, 17
Non-zero exit status: 2
t/65-prefer-number3.t (Wstat: 512 Tests: 44 Failed: 2)
Failed tests: 13, 17
Non-zero exit status: 2
Files=2, Tests=88, 0 wallclock secs ( 0.05 usr 0.02 sys + 0.20 cusr 0.04 csys = 0.30 CPU)
Result: FAIL
Also affected: BRUMMETT/Devel-Chitin-0.18.tar.gz
Partial output:
$ bleadprove -vb t/20-optree-deparse.t
t/20-optree-deparse.t ..
# Seeded srand with seed '20220225' from local date.
1..29
# Subtest: t/20-optree-deparse/01-assignment
1..11
ok 1 - t/20-optree-deparse/01-assignment/array_ref_assignment
ok 2 - t/20-optree-deparse/01-assignment/array_ref_slice_assignment
ok 3 - t/20-optree-deparse/01-assignment/hasf_ref_slice_assignment
ok 4 - t/20-optree-deparse/01-assignment/hash_assignment
ok 5 - t/20-optree-deparse/01-assignment/hash_ref_assignment
ok 6 - t/20-optree-deparse/01-assignment/hash_slice_assignment
ok 7 - t/20-optree-deparse/01-assignment/list-assignment
ok 8 - t/20-optree-deparse/01-assignment/list_index_assignment
ok 9 - t/20-optree-deparse/01-assignment/list_slice
ok 10 - t/20-optree-deparse/01-assignment/list_slice_assignment
ok 11 - t/20-optree-deparse/01-assignment/scalar_ref_assignment
ok 1 - Subtest: t/20-optree-deparse/01-assignment
[snip]
# Subtest: t/20-optree-deparse/12-i-o
1..29
ok 1 - t/20-optree-deparse/12-i-o/binmode
ok 2 - t/20-optree-deparse/12-i-o/close
ok 3 - t/20-optree-deparse/12-i-o/closedir
ok 4 - t/20-optree-deparse/12-i-o/dbmclose
ok 5 - t/20-optree-deparse/12-i-o/dbmopen
ok 6 - t/20-optree-deparse/12-i-o/die
ok 7 - t/20-optree-deparse/12-i-o/eof
ok 8 - t/20-optree-deparse/12-i-o/fileno
ok 9 - t/20-optree-deparse/12-i-o/flock
ok 10 - t/20-optree-deparse/12-i-o/getc
ok 11 - t/20-optree-deparse/12-i-o/print
ok 12 - t/20-optree-deparse/12-i-o/printf
ok 13 - t/20-optree-deparse/12-i-o/read
ok 14 - t/20-optree-deparse/12-i-o/readdir
ok 15 - t/20-optree-deparse/12-i-o/readline
ok 16 - t/20-optree-deparse/12-i-o/rewinddir
Use of uninitialized value $string in pattern match (m//) at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 866.
Use of uninitialized value $string in concatenation (.) or string at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 871.
Use of uninitialized value $string in pattern match (m//) at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 866.
Use of uninitialized value $string in concatenation (.) or string at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 871.
Use of uninitialized value $string in pattern match (m//) at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 866.
Use of uninitialized value $string in concatenation (.) or string at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 871.
not ok 17 - t/20-optree-deparse/12-i-o/seek
# Failed test 't/20-optree-deparse/12-i-o/seek'
# at t/20-optree-deparse.t line 90.
# +-----------------------+----+-------------------------------+
# | GOT | OP | CHECK |
# +-----------------------+----+-------------------------------+
# | $a = seek(F, 10, );\n | eq | $a = seek(F, 10, SEEK_CUR);\n |
# | $fh;\n | | $fh;\n |
# | seek($fh, -10, );\n | | seek($fh, -10, SEEK_END);\n |
# | seek(*$fh, 0, )\n | | seek(*$fh, 0, SEEK_SET)\n |
# +-----------------------+----+-------------------------------+
# Showing whitespace:
# >>$a = seek(F, 10, );<<
# >>$fh;<<
# >>seek($fh, -10, );<<
# >>seek(*$fh, 0, )<<
# $@:
# Tree:
UNOP leavesub (KIDS) BARE, SLICE 0x802444300
LISTOP lineseq (KIDS, PARENS) 0x802444dc8
COP nextstate (WANT_LIST, WANT_VOID) (eval 177):2 0x802445060
BINOP sassign (KIDS, STACKED, WANT_LIST, WANT_VOID) 0x8024453a8
LISTOP seek (KIDS, WANT_LIST, WANT_SCALAR) 0x8024454b0
OpTree null (ex-pp_pushmark) (WANT_LIST, WANT_SCALAR) 0x8024457c8
UNOP rv2gv (KIDS, SPECIAL, WANT_LIST, WANT_SCALAR) BARE, SLICE 0x802445af8
UNOP entersub (KIDS, STACKED, WANT_LIST, WANT_SCALAR) 0x802445bb8
BINOP null (ex-pp_list) (KIDS, WANT_LIST, WANT_SCALAR) 0x802445ed0
OpTree pushmark (WANT_LIST, WANT_SCALAR) 0x802447228
UNOP null (ex-pp_rv2cv) (KIDS, WANT_LIST, WANT_SCALAR) KVSLICE 0x802447558
PADOP gv (WANT_LIST, WANT_SCALAR) 0x802447630
SVOP const (WANT_LIST, WANT_SCALAR) 10 0x802447978
SVOP const (SPECIAL, WANT_LIST, WANT_SCALAR) '' 0x802447b40
OpTree padsv (MOD, REF, SPECIAL, WANT_LIST, WANT_SCALAR) $a 0x802447c18
COP nextstate (WANT_LIST, WANT_VOID) (eval 177):3 0x802447d20
OpTree padsv (MOD, WANT_LIST, WANT_VOID) $fh 0x802447e28
COP nextstate (WANT_LIST, WANT_VOID) (eval 177):4 0x802447f00
LISTOP seek (KIDS, WANT_LIST, WANT_VOID) 0x802447fd8
OpTree null (ex-pp_pushmark) (WANT_LIST, WANT_SCALAR) 0x8024490d8
UNOP rv2gv (KIDS, SPECIAL, WANT_LIST, WANT_SCALAR) BARE, SLICE 0x802449180
OpTree padsv (WANT_LIST, WANT_SCALAR) $fh 0x802449240
SVOP const (WANT_LIST, WANT_SCALAR) -10 0x802449300
SVOP const (SPECIAL, WANT_LIST, WANT_SCALAR) '' 0x802449408
COP nextstate (WANT_LIST, WANT_VOID) (eval 177):5 0x8024494e0
LISTOP seek (KIDS, WANT_LIST, WANT_SCALAR) 0x8024495e8
OpTree null (ex-pp_pushmark) (WANT_LIST, WANT_SCALAR) 0x8024496c0
UNOP rv2gv (KIDS, REF, WANT_LIST, WANT_SCALAR) BARE, SLICE 0x802449768
OpTree padsv (WANT_LIST, WANT_SCALAR) $fh 0x802449828
SVOP const (WANT_LIST, WANT_SCALAR) 0 0x8024498e8
SVOP const (SPECIAL, WANT_LIST, WANT_SCALAR) '' 0x8024499f0
Use of uninitialized value $string in pattern match (m//) at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 866.
Use of uninitialized value $string in concatenation (.) or string at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 871.
Use of uninitialized value $string in pattern match (m//) at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 866.
Use of uninitialized value $string in concatenation (.) or string at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 871.
Use of uninitialized value $string in pattern match (m//) at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 866.
Use of uninitialized value $string in concatenation (.) or string at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 871.
ok 18 - t/20-optree-deparse/12-i-o/seekdir
ok 19 - t/20-optree-deparse/12-i-o/select
ok 20 - t/20-optree-deparse/12-i-o/select-syscall
ok 21 - t/20-optree-deparse/12-i-o/syscall
ok 22 - t/20-optree-deparse/12-i-o/sysread
Use of uninitialized value $string in pattern match (m//) at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 866.
Use of uninitialized value $string in concatenation (.) or string at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 871.
Use of uninitialized value $string in pattern match (m//) at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 866.
Use of uninitialized value $string in concatenation (.) or string at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 871.
Use of uninitialized value $string in pattern match (m//) at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 866.
Use of uninitialized value $string in concatenation (.) or string at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 871.
not ok 23 - t/20-optree-deparse/12-i-o/sysseek
# Failed test 't/20-optree-deparse/12-i-o/sysseek'
# at t/20-optree-deparse.t line 90.
# +--------------------------+----+----------------------------------+
# | GOT | OP | CHECK |
# +--------------------------+----+----------------------------------+
# | $a = sysseek(F, 10, );\n | eq | $a = sysseek(F, 10, SEEK_CUR);\n |
# | $fh;\n | | $fh;\n |
# | sysseek($fh, -10, );\n | | sysseek($fh, -10, SEEK_END);\n |
# | sysseek(*$fh, 0, )\n | | sysseek(*$fh, 0, SEEK_SET)\n |
# +--------------------------+----+----------------------------------+
# Showing whitespace:
# >>$a = sysseek(F, 10, );<<
# >>$fh;<<
# >>sysseek($fh, -10, );<<
# >>sysseek(*$fh, 0, )<<
# $@:
# Tree:
UNOP leavesub (KIDS) BARE, SLICE 0x8024958a0
LISTOP lineseq (KIDS, PARENS) 0x802495eb8
COP nextstate (WANT_LIST, WANT_VOID) (eval 183):1 0x802496498
BINOP sassign (KIDS, STACKED, WANT_LIST, WANT_VOID) 0x8024967e0
LISTOP sysseek (KIDS, WANT_LIST, WANT_SCALAR) 0x8024968e8
OpTree null (ex-pp_pushmark) (WANT_LIST, WANT_SCALAR) 0x802496c00
UNOP rv2gv (KIDS, SPECIAL, WANT_LIST, WANT_SCALAR) 0x802496f30
UNOP entersub (KIDS, STACKED, WANT_LIST, WANT_SCALAR) 0x8022d6018
BINOP null (ex-pp_list) (KIDS, WANT_LIST, WANT_SCALAR) 0x8022d6330
OpTree pushmark (WANT_LIST, WANT_SCALAR) 0x8022d6660
UNOP null (ex-pp_rv2cv) (KIDS, WANT_LIST, WANT_SCALAR) KVSLICE 0x8022d6990
PADOP gv (WANT_LIST, WANT_SCALAR) 0x8022d6a68
SVOP const (WANT_LIST, WANT_SCALAR) 10 0x8022d6db0
SVOP const (SPECIAL, WANT_LIST, WANT_SCALAR) '' 0x8022d6f78
OpTree padsv (MOD, REF, SPECIAL, WANT_LIST, WANT_SCALAR) $a 0x8022d8078
COP nextstate (WANT_LIST, WANT_VOID) (eval 183):2 0x8022d8180
OpTree padsv (MOD, WANT_LIST, WANT_VOID) $fh 0x8022d8288
COP nextstate (WANT_LIST, WANT_VOID) (eval 183):3 0x8022d8360
LISTOP sysseek (KIDS, WANT_LIST, WANT_VOID) 0x8022d8438
OpTree null (ex-pp_pushmark) (WANT_LIST, WANT_SCALAR) 0x8022d8510
UNOP rv2gv (KIDS, SPECIAL, WANT_LIST, WANT_SCALAR) 0x8022d85b8
OpTree padsv (WANT_LIST, WANT_SCALAR) $fh 0x8022d8678
SVOP const (WANT_LIST, WANT_SCALAR) -10 0x8022d8738
SVOP const (SPECIAL, WANT_LIST, WANT_SCALAR) '' 0x8022d8840
COP nextstate (WANT_LIST, WANT_VOID) (eval 183):4 0x8022d8918
LISTOP sysseek (KIDS, WANT_LIST, WANT_SCALAR) 0x8022d8a20
OpTree null (ex-pp_pushmark) (WANT_LIST, WANT_SCALAR) 0x8022d8af8
UNOP rv2gv (KIDS, REF, WANT_LIST, WANT_SCALAR) 0x8022d8ba0
OpTree padsv (WANT_LIST, WANT_SCALAR) $fh 0x8022d8c60
SVOP const (WANT_LIST, WANT_SCALAR) 0 0x8022d8d20
SVOP const (SPECIAL, WANT_LIST, WANT_SCALAR) '' 0x8022d8e28
Use of uninitialized value $string in pattern match (m//) at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 866.
Use of uninitialized value $string in concatenation (.) or string at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 871.
Use of uninitialized value $string in pattern match (m//) at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 866.
Use of uninitialized value $string in concatenation (.) or string at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 871.
Use of uninitialized value $string in pattern match (m//) at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 866.
Use of uninitialized value $string in concatenation (.) or string at /usr/home/jkeenan/.cpanm/work/1645814451.17065/Devel-Chitin-0.18/blib/lib/Devel/Chitin/OpTree.pm line 871.
ok 24 - t/20-optree-deparse/12-i-o/syswrite
ok 25 - t/20-optree-deparse/12-i-o/tell
ok 26 - t/20-optree-deparse/12-i-o/telldir
ok 27 - t/20-optree-deparse/12-i-o/truncate
ok 28 - t/20-optree-deparse/12-i-o/warn
ok 29 - t/20-optree-deparse/12-i-o/write
not ok 12 - Subtest: t/20-optree-deparse/12-i-o
# Failed test 'Subtest: t/20-optree-deparse/12-i-o'
# at t/20-optree-deparse.t line 43.
[snip]
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/29 subtests
Test Summary Report
-------------------
t/20-optree-deparse.t (Wstat: 256 Tests: 29 Failed: 1)
Failed test: 12
Non-zero exit status: 1
Files=1, Tests=29, 0 wallclock secs ( 0.07 usr 0.02 sys + 0.39 cusr 0.05 csys = 0.52 CPU)
Result: FAIL
@nwc10, can you take a look? ^^ Thanks.
The Storable::AMF tests literally do stuff like this:
$a = 1;
$b = "".$a;
$var = "Int++";
ok( is_amf_string($a) , "$var converted is a string" );
Clearly, that is no longer going to work, and it never should have had such an expectation. Looks like these tests should be modified to gracefully handle the new situation (or possibly just be deleted)
Devel::Chitin is using B for some deep introspection. It's calling $sv->PV
on B::SV values that no longer have a PV.
I'm not sure how to handle this, possibly this needs a fix in B.
The Storable::AMF tests literally do stuff like this:
$a = 1; $b = "".$a; $var = "Int++"; ok( is_amf_string($a) , "$var converted is a string" );
Clearly, that is no longer going to work, and it never should have had such an expectation. Looks like these tests should be modified to gracefully handle the new situation (or possibly just be deleted)
I think we're going to need some stock explanation of why and how downstream code must change to adapt to the (1 character!) change in https://github.com/Perl/perl5/commit/bb5bc9. An explanation that an ordinary Perl user -- someone without a deep understanding of XS or sv.c
can understand. An explanation that we can paste into a bug ticket for an affected CPAN module.
Any suggestions?
Devel::Chitin is using B for some deep introspection. It's calling
$sv->PV
on B::SV values that no longer have a PV.I'm not sure how to handle this, possibly this needs a fix in B.
@iabyn, @tonycoz, @atoomic, @toddr -- can you take a look ^^?
Devel::Chitin is using B for some deep introspection. It's calling
$sv->PV
on B::SV values that no longer have a PV.I'm not sure how to handle this, possibly this needs a fix in B.
The code in question appears to be https://github.com/brummett/Devel-Chitin/search?q=%24sv-%3Epv
I can't tell if it's in a higher call but it seems like they need to do something like:
$sv->FLAGS & B::SVf_POK or die("This isn't a PV");
Before they can safely do $sv->PV
. They're not and so it's failing 🤷🏻
I can't tell if it's in a higher call but it seems like they need to do something like:
$sv->FLAGS & B::SVf_POK or die("This isn't a PV");
Before they can safely do $sv->PV. They're not and so it's failing 🤷🏻
That is one approach. The other would be to also return a string if SVp_POK
is set. I'm currently leaning to that, but I'm maybe I'm not seeing some repercussion of that.
Also affected: BRUMMETT/Devel-Chitin-0.18.tar.gz
Ticket opened upstream at RT 141554.
Description
Commit v5.35.8-110-gbb5bc97fde/da1c (bb5bc9) stopped also GRIAN/Storable-AMF-1.23.tar.gz from succeeding with the test suite. We had this commit already in #19450 ; that issue has been closed, but the fix provided over there does not help for Storable::AMF 1.23 ; you can already find sample fail reports on http://matrix.cpantesters.org/?dist=Storable-AMF+1.23
Ticket created upstream at RT 141555.
Thank you very much. Jim Keenan
I can't tell if it's in a higher call but it seems like they need to do something like:
$sv->FLAGS & B::SVf_POK or die("This isn't a PV");
Before they can safely do $sv->PV. They're not and so it's failing 🤷🏻
That is one approach. The other would be to also return a string if
SVp_POK
is set. I'm currently leaning to that, but I'm maybe I'm not seeing some repercussion of that.
Right. My problem is not what flag so much as it's unclear if it was a mistake to ever enter the sub if it wasn't set and so the problem is in the caller, not that sub.
I can't tell if it's in a higher call but it seems like they need to do something like:
$sv->FLAGS & B::SVf_POK or die("This isn't a PV");
Before they can safely do $sv->PV. They're not and so it's failing 🤷🏻
That is one approach. The other would be to also return a string if
SVp_POK
is set. I'm currently leaning to that, but I'm maybe I'm not seeing some repercussion of that.Right. My problem is not what flag so much as it's unclear if it was a mistake to ever enter the sub if it wasn't set and so the problem is in the caller, not that sub.
It's a bug in Devel::Chitin
You can expose it with earlier versions of perl with something like:
use constant foo => do { my $val = "Four"; $val = 4; $val };
my $q = foo;
The code assumes that anything that is SVt_PV or a subclass must have a string. it can't cope with a PVIV that is actually an IV. That code above looks like this:
$ perl -MO=Concise -e 'use constant foo => do { my $val = "Four"; $val = 4; $val }; my $q = foo'
6 <@> leave[1 ref] vKP/REFC ->(end)
1 <0> enter v ->2
2 <;> nextstate(main 190 -e:1) v:{ ->3
5 <2> sassign vKS/2 ->6
3 <$> const[PVIV 4] s*/FOLD ->4
4 <0> padsv[$q:190,191] sRM*/LVINTRO ->5
-e syntax OK
ie const[PVIV 4]
not the [IV 4]
you'd see from something sane:
$ perl -MO=Concise -e 'use constant foo => 4; my $q = foo'
6 <@> leave[1 ref] vKP/REFC ->(end)
1 <0> enter v ->2
2 <;> nextstate(main 187 -e:1) v:{ ->3
5 <2> sassign vKS/2 ->6
3 <$> const[IV 4] s*/FOLD ->4
4 <0> padsv[$q:187,188] sRM*/LVINTRO ->5
-e syntax OK
The fix (for the above bug, and the regression we see here), is something like this:
$ diff -u lib/Devel/Chitin/OpTree/SVOP.pm~ lib/Devel/Chitin/OpTree/SVOP.pm
--- lib/Devel/Chitin/OpTree/SVOP.pm~ 2018-10-25 22:55:02.000000000 +0000
+++ lib/Devel/Chitin/OpTree/SVOP.pm 2022-02-26 07:03:05.549223734 +0000
@@ -6,6 +6,8 @@
use strict;
use warnings;
+use B qw(SVf_IOK SVf_NOK SVf_POK);
+
sub pp_const {
my $self = shift;
my %params = @_;
@@ -19,13 +21,12 @@
for (my $mg = $sv->MAGIC; $mg; $mg = $mg->MOREMAGIC) {
return $mg->PTR if $mg->TYPE eq 'V';
}
-
- } elsif ($sv->isa('B::PV')) {
+ } elsif ($sv->FLAGS & SVf_POK) {
return $self->_quote_sv($sv, %params);
- } elsif ($sv->isa('B::NV')) {
- return $sv->NV;
- } elsif ($sv->isa('B::IV')) {
+ } elsif ($sv->FLAGS & SVf_IOK) {
return $sv->int_value;
+ } elsif ($sv->FLAGS & SVf_NOK) {
+ return $sv->NV;
} elsif ($sv->isa('B::SPECIAL')) {
'<???pp_const B::SPECIAL ' . $B::specialsv_name[$$sv] . '>';
(adapted from something in B
, which clearly is more robust)
I was working on that last night, but it got too late and I hadn't quite nailed it.
The cause of the confusion is that effectively the regression tests do this:
use Fcntl qw(SEEK_SET SEEK_CUR SEEK_END); # omit
BEGIN {
my %seek_flags = (
SEEK_SET() => 'SEEK_SET',
SEEK_CUR() => 'SEEK_CUR',
SEEK_END() => 'SEEK_END',
);
}
my $a = seek(F, 10, SEEK_CUR);
my $fh;
seek($fh, -10, SEEK_END);
seek(*$fh, 0, SEEK_SET)
and constructing that hash (in lib/Devel/Chitin/OpTree/LISTOP.pm
) causes the constants in Fcntl to be converted from IV
to PVIV
as a side effect of "reading" the numbers as strings.
Which we've just changed so as not to set SVf_POK
I might not get time to look at this further for at least 4 days.
On Sat, 26 Feb 2022 at 00:23, Leon Timmermans @.***> wrote:
I can't tell if it's in a higher call but it seems like they need to do something like:
$sv->FLAGS & B::SVf_POK or die("This isn't a PV");
Before they can safely do $sv->PV. They're not and so it's failing 🤷🏻
That is one approach. The other would be to also return a string if POKp is set. I'm currently leaning to that, but I'm maybe I'm not seeing some repercussion of that.
That makes a lot more sense to me.
Yves
-- perl -Mre=debug -e "/just|another|perl|hacker/"
The Storable::AMF tests literally do stuff like this:
$a = 1; $b = "".$a; $var = "Int++"; ok( is_amf_string($a) , "$var converted is a string" );
Clearly, that is no longer going to work, and it never should have had such an expectation. Looks like these tests should be modified to gracefully handle the new situation (or possibly just be deleted)
If you look at Grian/Storable-AMF@70ff9715265e639a034c44b01201ab7c5eda0041 you'll see how it handled the previous analogous change, when stringification of NVs stopped setting SVf_POK:
@@ -55,7 +55,8 @@ $a = 1.0;
# DoubleVar
$a = 1.0;
$b = "$a";
$var = "Double";
-ok( is_amf_string($a) , "$var converted is a string" );
+my %string_number=(0=>1,2=>1);
+ok( $string_number{ byte_amf_string($a)} , "$var converted is a string" );
ok( ! is_amf_string(0+$a) , "$var 0+converted is double" );
ok( ! is_amf_string(0.0+$a,$nop), "$var 0.0+converted is double" );
ok( ! is_amf_string($a, $nop) , "$var converted again is double" );
with:
sub is_amf_string{
ord( freeze( $_[0], $_[1]||0 )) == 2;
}
sub byte_amf_string{
ord( freeze( $_[0], $_[1]||0 ));
That change is clearly not the world's most robust piece of code logic.
I think I get the intent of the tests, but (again) I have run out of time for now. I think that the intent is if you have this:
$ perl -MDevel::Peek -e '$foo = "42"; sqrt $foo; Dump $foo'
SV = PVNV(0x55b3fc452240) at 0x55b3fc4801a0
REFCNT = 1
FLAGS = (NOK,POK,IsCOW,pNOK,pPOK)
IV = 0
NV = 42
PV = 0x55b3fc4c18d0 "42"\0
CUR = 2
LEN = 10
COW_REFCNT = 1
1) does it stringify as a number, or a string? (default options) 2) and does setting the option change that
and analogous tests for PVIVs which have both POK
and IOK
(the ones failing today)
There were then later tests added for "if I used the value in arithmetic does it change?" which I think are superfluous to what the module tests.
So I think that the correct fix is to reduce the module's regression tests down to what it actually needs to test the module (not perl's behaviours, that have changed). ie start with a string, and do an integer or a floating point operation on it to force IOK or NOK on as well, but with POK still set.
But I've run out of time (for the next few days at least) to figure this out properly.
I don't believe this should block the release of v5.36.0 but have not (yet) added a non-5.36-blocker label, as I'm hoping we'll see a new release.
@nwc10 What do you think about the value of adding a "how this may break your slightly wrong code" section to perldelta?
What do you think about the value of adding a "how this may break your slightly wrong code" section to perldelta?
I'm really not sure if there is a useful wording. Since the merge we've uncovered (I think) 2 things that were bugs in the core that needed fixing. The rest grouped as
B
to read flags (and the module in question was already subtly buggy - we just exposed it directly in an existing test)but >50% of these were "we exposed existing bugs in your code"
and the real problem is that it's "your code, or any of your dependency chain". I'm not sure how to offer specific advice that's tight enough to be useful, but simulateously suggests that all wierd upgrade woes are caused by this (because most won't be)
I guess it's "your or your dependencies used B
directly to read flags, or you or your dependencies rely exactly on how your serealiser stores data structures" but as much "and they were fragile and you didn't know this before"
I am now marking this as non-blocking.
I believe we should close this ticket, for the following reasons:
Hence, there's nothing more that P5P can do at this time. I'm self-assigning this ticket for the purpose of closing it within 14 days unless someone wants to carry the discussion forward.
I believe we should close this ticket, for the following reasons:
1. Storable-AMF is the only upstream distro still needing action. 2. The upstream maintainer has not responded to the [bug ticket](https://rt.cpan.org/Ticket/Display.html?id=141555) I filed in February about the problem. 3. There has not been a new release of this distribution in more than 6 years.
Hence, there's nothing more that P5P can do at this time. I'm self-assigning this ticket for the purpose of closing it within 14 days unless someone wants to carry the discussion forward.
No objection heard to closing this ticket; doing so now.
Description
Commit v5.35.8-110-gbb5bc97fde/da1c (https://github.com/perl/perl5/commit/bb5bc9) stopped also GRIAN/Storable-AMF-1.23.tar.gz from succeeding with the test suite. We had this commit already in https://github.com/Perl/perl5/issues/19450 ; that issue has been closed, but the fix provided over there does not help for Storable::AMF 1.23 ; you can already find sample fail reports on http://matrix.cpantesters.org/?dist=Storable-AMF+1.23
Steps to Reproduce
cpan -i GRIAN/Storable-AMF-1.23.tar.gz
Expected behavior
Should successfully test and install the module
Perl configuration