New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Perl 5.18.1 breaks Devel::Cover #13176
Comments
From @jkeenanAt some point between Perl 5.18.0 and 5.18.1, Devel::Cover began to fail Here are the CPAN Testers Perl/Platform version matrices for http://matrix.cpantesters.org/?dist=Devel-Cover%201.06;perl=5.18.0;reports=1 http://matrix.cpantesters.org/?dist=Devel-Cover%201.06;perl=5.18.1;reports=1 With one unusual exception, D::C 1.06 has built and tested successfully However, when we turn to the matrix for Perl 5.18.1, we see D::C failing * They are the same failures I got yesterday when trying to install D::C * They are the same failures seen in the one, unusual failure report for When diagnosing this yesterday, I tried to install a slightly older This has been reported in D-C's bug queue: In that report, the poster notes what I also observed: On top of Can someone with a big enough box try at a 'git bisect' on this? Thank you very much. |
From @jkeenanSummary of my perl5 (revision 5 version 18 subversion 1) configuration: Characteristics of this binary (from libperl): |
From @pjcjOn Sun, Aug 18, 2013 at 06:09:56AM -0700, James E Keenan wrote:
In fact, all tests which had to do with actually collecting any coverage I'm very sorry that I haven't had much time recently to look at I've managed to create a minimal test case: $ perl5.18.0 -MB -E 'say B::main_cv->GV // "undef"' Jim and I think we have identified the offending commit. It is: commit e6c4c33 CvGV is no longer a simple struct member access Inline Patchdiff --git a/ext/B/B.pm b/ext/B/B.pm
index 8856a32..bd2cf66 100644
--- a/ext/B/B.pm
+++ b/ext/B/B.pm
@@ -15,7 +15,7 @@ require Exporter;
# walkoptree comes from B.xs
BEGIN {
- $B::VERSION = '1.42';
+ $B::VERSION = '1.42_01';
@B::EXPORT_OK = ();
# Our BOOT code needs $VERSION set, and will append to @EXPORT_OK.
@@ -1014,6 +1014,10 @@ For constant subroutines, returns the constant SV returned by the subroutine.
=item const_sv
+=item NAME_HEK
+
+Returns the name of a lexical sub, otherwise C<undef>.
+
=back
=head2 B::HV Methods
diff --git a/ext/B/B.xs b/ext/B/B.xs
index e2ebdad..d933a56 100644
--- a/ext/B/B.xs
+++ b/ext/B/B.xs
@@ -1380,7 +1380,6 @@ IVX(sv)
B::IO::IoFLAGS = PVIO_flags_ix
B::AV::MAX = PVAV_max_ix
B::CV::STASH = PVCV_stash_ix
- B::CV::GV = PVCV_gv_ix
B::CV::FILE = PVCV_file_ix
B::CV::OUTSIDE = PVCV_outside_ix
B::CV::OUTSIDE_SEQ = PVCV_outside_seq_ix
@@ -1873,6 +1872,27 @@ const_sv(cv)
PPCODE:
PUSHs(make_sv_object(aTHX_ (SV *)cv_const_sv(cv)));
+void
+GV(cv)
+ B::CV cv
+ PREINIT:
+ GV *gv;
+ CODE:
+ gv = CvGV(cv);
+ ST(0) = gv ? make_sv_object((SV*)gv) : &PL_sv_undef;
+
+#if PERL_VERSION > 17
+
+SV *
+NAME_HEK(cv)
+ B::CV cv
+ CODE:
+ RETVAL = CvNAMED(cv) ? newSVhek(CvNAME_HEK(cv)) : &PL_sv_undef;
+ OUTPUT:
+ RETVAL
+
+#endif
+
MODULE = B PACKAGE = B::HV PREFIX = Hv
STRLEN
diff --git a/ext/B/t/b.t b/ext/B/t/b.t
index a065375..d58d2e0 100644
--- a/ext/B/t/b.t
+++ b/ext/B/t/b.t
@@ -376,4 +376,43 @@ SKIP: {
is($op->name, "leavesub", "overlay: orig name");
}
+{ # [perl #118525]
+ {
+ sub foo {}
+ my $cv = B::svref_2object(\&foo);
+ ok($cv, "make a B::CV from a non-anon sub reference");
+ isa_ok($cv, "B::CV");
+ my $gv = $cv->GV;
+ ok($gv, "we get a GV from a GV on a normal sub");
+ isa_ok($gv, "B::GV");
+ is($gv->NAME, "foo", "check the GV name");
+ SKIP:
+ { # do we need these version checks?
+ skip "no HEK before 5.18", 1 if $] < 5.018;
+ is($cv->NAME_HEK, undef, "no hek for a global sub");
+ }
+ }
+
+SKIP:
+ {
+ skip "no HEK before 5.18", 4 if $] < 5.018;
+ eval <<'EOS'
+ {
+ use feature 'lexical_subs';
+ no warnings 'experimental::lexical_subs';
+ my sub bar {};
+ my $cv = B::svref_2object(\&bar);
+ ok($cv, "make a B::CV from a lexical sub reference");
+ isa_ok($cv, "B::CV");
+ my $gv = $cv->GV;
+ is($gv, undef, "GV on a lexical sub is NULL");
+ my $hek = $cv->NAME_HEK;
+ is($hek, "bar", "check the NAME_HEK");
+ }
+ 1;
+EOS
+ or die "lexical_subs test failed to compile: $@";
+ }
+}
+
done_testing();
-- Paul Johnson - paul@pjcj.net |
The RT System itself - Status changed from 'new' to 'open' |
From @jkeenanOn Sun Aug 18 08:53:37 2013, paul@pjcj.net wrote:
If this indeed is the commit associated with the D::C test failures, Of course, by its very nature, Devel::Cover has to muck about in the This will affect how we address this problem. If the problematic commit If, however, the problematic commit was needed to correct a regression, (Of course, all the above is true regardless of whether e6c4c33 is the How should we proceed? Thank you very much. |
From @pjcjOn Sun, Aug 18, 2013 at 09:49:46AM -0700, James E Keenan via RT wrote:
I think the reason is detailed in 323602a: +L<B> has been upgraded from version 1.44 to 1.45. So my guess is that what we have here is probably a buggy bug fix. What -- |
From @cpansproutOn Sun Aug 18 08:53:37 2013, paul@pjcj.net wrote:
Youch! I find B’s interface so arcane it’s very hard to miss things
You can blame it all on me. He was following my recommendations. It is worth changing this back? Any code depending on it will now have -- Father Chrysostomos |
From @pjcjOn Sun, Aug 18, 2013 at 10:52:35AM -0700, Father Chrysostomos via RT wrote:
In this particular case, we're consistent with B::SPECIAL all the way
As far as Devel::Cover is concerned, I really don't mind. I have a However, countering your argument about churn is the argument that Then again, does anyone know of any code apart from Devel::Cover which -- |
From @tonycozOn Sun, Aug 18, 2013 at 10:52:35AM -0700, Father Chrysostomos via RT wrote:
I think I was confused by a comment in #118525 that a B::NULL was the
I agree with this, but have no problems changing it back. Tony |
From @tonycozOn Tue, Aug 20, 2013 at 12:57:14AM +0200, Paul Johnson wrote:
The same change in behaviour is in blead, and I haven't seen a BBC Tony |
From @jkeenanOn Mon Aug 19 15:57:54 2013, paul@pjcj.net wrote:
If we have a consensus that we introduced a bug between 5.18.0 and
Well, as you pointed out, "Few people will have upgraded." So that But, speaking for myself, I hate being without Devel::Cover on any Thank you very much. |
From zefram@fysh.orgPaul Johnson wrote:
This seems wise. I reckon the change of representation is a bug. -zefram |
From @ntyniOn Tue, Aug 20, 2013 at 12:57:14AM +0200, Paul Johnson wrote:
Hi, as it happens, we're switching Debian unstable/testing to 5.18.1 any day Paul, could you please make your patch available somewhere, even if it's Thanks for your work, |
From @pjcjOn Wed, Aug 21, 2013 at 10:21:23PM +0300, Niko Tyni wrote:
I've just released Devel::Cover 1.07 which should work correctly on The tests fail on 5.19.2 and 5.19.3 due to changed line numbers, but I -- |
From @jkeenanOn Wed Aug 21 15:30:23 2013, paul@pjcj.net wrote:
And it does indeed work on Perl 5.18.1. I just now installed D-C 1.07
Since the complaint in the OP has been addressed, I'm marking this Thank you very much. |
@jkeenan - Status changed from 'open' to 'resolved' |
From @andkPaul Johnson <paul@pjcj.net> writes:
Same as https://rt-archive.perl.org/perl5/Ticket/Display.html?id=118931 -- |
From @nwc10On Tue, Aug 20, 2013 at 09:08:08AM +1000, Tony Cook wrote:
Code which wants to work with v5.18.0 and v5.18.1 already has to deal with If B::SPECIAL is the "right" answer (both because it was the previous result To me, it doesn't look like churn. It looks like a beneficial bugfix. Nicholas Clark |
Migrated from rt.perl.org#119351 (status was 'resolved')
Searchable as RT119351$
The text was updated successfully, but these errors were encountered: