BBC: Blead Breaks Variable::Magic, Devel::Caller #20114
Comments
|
Looing at a cpan test report1: so I'm guessing this was "caused" by commit 9fdd7fc : ( @richardleach ) (so the change is probably expected and Variable::Magic needs fixing) Footnotes |
|
Bisection points to this commit: @richardleach, can you take a look? |
jkeenan
added
BBC
|
Also affected: RCLAMP/Devel-Caller-2.06.tar.gz |
Patch for Devel::Caller supplied in https://rt.cpan.org/Ticket/Display.html?id=144051 |
Patch for Variable::Magic supplied in https://rt.cpan.org/Ticket/Display.html?id=144052 |
jkeenan
changed the title
|
In addition to the test failures previously reported in Variable::Magic, we're also getting failures in 2 test files which are only run on threaded builds (which implies that the patch already sent upstream for this module will not be sufficient). CPAN distribution Variable-Magic is experiencing test failures against Perl 5 blead. CPANtesters results here. Failures occur against both unthreaded and threaded builds. Sample failures in a threaded build on FreeBSD-12: Bisecting points to the same commit previously identified: 9fdd7fc |
|
Thanks for the details. I've updated https://rt.cpan.org/Ticket/Display.html?id=144052 with a revised patch for Variable::Magic. |
|
Also affected: |
|
Thanks, I'll take a look this evening. |
|
IMO we should define an "opcode version" magic var, define, whatever, that people can test. If they look at our optree they should check the opcode-version, if they are not up to date they should not claim their tests are broken, they should report they have not updated the module to handle that latest version. Calling these types of issue an "error" is really unhelpful. We have to be able to make changes to the optree that perl executes. Heck you could argue we have the right to /entirely eliminate any idea of an optree at all/ as it is all just an implementation detail of the language internals itself. As such there is a big distinction between "this is broken because we changed how old things worked" and "this broken because we no longer do things that way". From the module point of view its much of a muchness, the module just doesn't work, but there is a big difference between an unadvertised (or even unanticipated) change of behavior in an existing opcode, and an advertised deliberate change to use a totally new opcode. Eg, "I havent update this module to work with this perl" is quite different from "you broke this module". Until we have an opcode version we simply have nothing to test against, and a cpan module author has no other choice but to "just fail the tests", which then make core dev look bad when in fact its just a natural part of improving the language. |
|
The Mixin::Event::Dispatch and Test::Weaken breakage was down to a bug, which has been fixed in 6f62971 Variable::Magic and Devel::Caller failures weren't caused by this bug, they still need the submitted patches (or similar) applying. |
|
Also affected: ZEFRAM/Memoize-Once-0.002.tar.gz @richardleach , can you have a look, please? |
|
Ping. Or would it be preferred that I write a new ticket for Memoize::Once and whatever comes next? |
|
Adding to this ticket is fine if you like. I had a quick squint at Memoize::Once, but am waiting until this weekend to properly dig into it. |
|
Also affected: OALDERS/App-perlvars-0.000004.tar.gz |
I'm looking at |
I think I understand what Note: I have a busy work-week ahead and minor surgery on Friday, so might not get back to this until next weekend/the week after. |
|
Alternatively, since The previous idea is cleaner and doesn't mean that core would have to be modified just to make this module work. On the other hand, the |
|
Late-night realization: that's probably all complete nonsense, and the "problem" is that the peephole optimization for PADSV_STORE doesn't expect the op to be reached via op_other, as I've not seen that anywhere other than in the optree output by |
|
Merged a very minor and straightforward tweak to PADSV_STORE's peephole optimization to skip the kind of SASSIGN ops emitted by |
|
Summarizing the still-broken packages from this issue:
|
The CPANtesters reports for Test-Vars are not consistently showing failures; only 4 runs on Linux are doing so. Just now, I was able to install Test::Vars against Perl 5 blead at 8975221 on FreeBSD-12 -- but when I then tried to install App::perlvars against that same perl, I got failures similar to those being reported at http://matrix.cpantesters.org/?dist=App-perlvars+0.000004. Hence, patching Test::Vars might not be part of the resolution of this ticket. |
I can, however, confirm that 9fdd7fc is the breaking commit for App::perlvars. |
|
I think package Local::Unused;
use strict;
use warnings;
my $foo;
sub bar {
my $unused;
my $one;
my $two;
my $three;
my $four = 4;
}
1;Previously this was reporting that 4 variables were unused. Now it's reporting that there are 5, with To be clear, it's detecting the vars in |
That's interesting. I wonder if 6f62971 is what made the difference. In that example, the only thing touched by the PADSV_STORE optimization in blead is the Just so I understand correctly:
Under blead, Test::Vars reports correctly for 'sub bar', or just less broken than before?
That should be detected as unused? |
|
The documentation for Test::Vars includes this subsection: MECHANISM While "Perl::Critic", the backend of "Test::Perl::Critic", scans the source code as text, this modules scans the compiled opcodes (or AST: abstract syntax tree) using the "B" module. See also "B" and its submodules. Now I know next to nothing about "compiled opcodes" or abstract syntax trees, but just the sound of that suggests that Test-Vars's functioning and output is going to be very sensitive to changes in Perl's guts. What's weird here is that that sensitivity is not being reported consistently in its own CPANtesters results. Failures are only showing up consistently in a different CPAN distro's CPANtesters results. Moreover, the criterion for what makes a variable "unused" lies entirely within Test-Vars (or the minds of its author and users). AFAICT, (Note in passing: In this, Perl is different from C. So I'm inclined to say that this problem should be reported to https://github.com/houseabsolute/p5-Test-Vars/issues. I further note that Test-Vars is listed as being in need of a new maintainer. Unfortunately, that means there's no clear path forward for App-perlvars. |
Right. It's essentially useful as a linter. I've discussed it a bit here: https://www.olafalders.com/2022/02/22/finding-unused-perl-variables/ |
Under blead it gets everything correct for
In my mind, yes. But under earlier perl versions it doesn't get detected as unused either. I don't know the internals of |
Yes, that's probably best in the first instance, otherwise I'll just be guessing about what's the intended behaviour. I've created houseabsolute/p5-Test-Vars#47 for now. |
Didn't you say it was checking the vars in bar()? Makes sense to me it wouldnt be picked up in there. |
|
|
Status update:
|
|
I see Devel::Caller still has not been patched. Is it abandonware? |
Not sure, am going to cross-post the patch to https://github.com/pjcj/Devel--Cover in case that's the preferred method. |
Ignore me. Wrong module in haste! |
@richardc - are you able to do a release of Devel::Caller to apply https://rt.cpan.org/Ticket/Display.html?id=144051? (Would it make your life easier if I did a PR to https://github.com/richardc/perl-devel-caller?) |
This patch is by Richard Leach, who added the new opcodes to perl 5.37. I am just pushing it because I already had it applied to a git repo of Devel::Caller. This fixes Perl/perl5#20114
|
@richardleach I just created richardc/perl-devel-caller#2 I already had it set up in a repo, it was easy to do. |
|
Variable::Magic fixed. Devel::Cover already represented by #19869 |
The other CPAN breakage that was of concern in this issue was Devel::Caller, not Devel::Cover. Devel::Caller is still failing its tests. |
|
Thanks, oops! |
Devel-Caller has not had a new CPAN release in ten years, but is still considered useful enough that it was picking up reverse dependencies as recently as last year. A patch was submitted upstream in https://rt.cpan.org/Ticket/Display.html?id=144051. Does anyone know how to contact upstream maintainer Richard Clamp? @richardc |
|
Hello, this mention showed up in my inbox. I'll look at that that patch later today. |
|
Re-closed, thanks so much @richardc |
This is a bug report for perl from "Carlos Guevara" carlos@carlosguevara.com,
generated with the help of perlbug 1.42 running under perl 5.37.3.
[Please describe your issue here]
BBC: Blead Breaks Variable::Magic
Please see http://matrix.cpantesters.org/?dist=Variable::Magic
[Please do not change anything below this line]
Flags:
category=core
severity=low
Site configuration information for perl 5.37.3:
Configured by cpan at Thu Aug 18 01:16:20 EDT 2022.
Summary of my perl5 (revision 5 version 37 subversion 3) configuration:
Commit id: bbae2b2
Platform:
osname=openbsd
osvers=7.1
archname=OpenBSD.amd64-openbsd
uname='openbsd cjg-openbsd7 7.1 generic#3 amd64 '
config_args='-des -Dprefix=
/bin/perl-blead -Dscriptdir=/bin/perl-blead/bin -Dusedevel -Duse64bitall'hint=recommended
useposix=true
d_sigaction=define
useithreads=undef
usemultiplicity=undef
use64bitint=define
use64bitall=define
uselongdouble=undef
usemymalloc=n
default_inc_excludes_dot=define
Compiler:
cc='cc'
ccflags ='-fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
optimize='-O2'
cppflags='-fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
ccversion=''
gccversion='OpenBSD Clang 13.0.0'
gccosandvers=''
intsize=4
longsize=8
ptrsize=8
doublesize=8
byteorder=12345678
doublekind=3
d_longlong=define
longlongsize=8
d_longdbl=define
longdblsize=16
longdblkind=3
ivtype='long'
ivsize=8
nvtype='double'
nvsize=8
Off_t='off_t'
lseeksize=8
alignbytes=8
prototype=define
Linker and Libraries:
ld='cc'
ldflags ='-Wl,-E -fstack-protector-strong -L/usr/local/lib'
libpth=/usr/lib/clang/13.0.0/lib /usr/lib /usr/local/lib
libs=-lpthread -lm -lutil -lc
perllibs=-lpthread -lm -lutil -lc
libc=/usr/lib/libc.so.96.1
so=so
useshrplib=false
libperl=libperl.a
gnulibc_version=''
Dynamic Linking:
dlsrc=dl_dlopen.xs
dlext=so
d_dlsymun=undef
ccdlflags=' '
cccdlflags='-DPIC -fPIC '
lddlflags='-shared -fPIC -L/usr/local/lib -fstack-protector-strong'
@inc for perl 5.37.3:
/home/cpan/bin/perl-blead/lib/site_perl/5.37.3/OpenBSD.amd64-openbsd
/home/cpan/bin/perl-blead/lib/site_perl/5.37.3
/home/cpan/bin/perl-blead/lib/5.37.3/OpenBSD.amd64-openbsd
/home/cpan/bin/perl-blead/lib/5.37.3
Environment for perl 5.37.3:
HOME=/home/cpan
LANG (unset)
LANGUAGE (unset)
LC_ALL=C
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/home/cpan/bin/perl-blead/bin:/home/cpan/bin:/home/cpan/bin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/X11R6/bin:/usr/local/bin:/usr/local/sbin:/usr/games
PERL_BADLANG (unset)
SHELL=/usr/local/bin/bash
The text was updated successfully, but these errors were encountered: