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
Bleadperl v5.25.5-43-g607ee43 breaks CDOLAN/Devel-EnforceEncapsulation-0.51.tar.gz #15650
Comments
From @dcollinsnWhile smoking CPAN modules against various versions of Perl, I noticed the Bisects to: bad - non-zero exit from /tmp/lX36LYqWLD/bin/perl -I /home/cpanbisect/.cpan Speed up compilation of overload.pm a smidge. Measured with the following crude perl script calling perf. Perl print 'PERL', (`perf stat -r100 perl -e 1 2>&1`)[10]; Produced the following results on my machine: PERL 5,800,051 instructions # 1.05 insns per cycle ( +- 0.06% ) While the numbers did fluctuate between runs, the new code was :040000 040000 e92b68c47128f987038e0a5c2c4b99d361a9561e Diagnostics attached. Output buffering makes it look like some messages are t/api.t ........... 1/52 overload arg '${}' is invalid at (eval 14) line 1. # Failed test 'no warning on unencapsulated class' # Failed test '...but it does raise a warning' Perl -V of a failing perl: Summary of my perl5 (revision 5 version 25 subversion 6) configuration: Characteristics of this binary (from libperl): Happy to provide any other information that may be helpful. Respectfully, |
From @dcollinsnI have added James Raspass to the CC list, as he is the original author of the patch. James, this ticket describes a regression introduced by the patch you emailed to the list, which was exposed by a CPAN module called Devel::EnforceEncapsulation. I'd appreciate any diagnosis you can provide. -- |
From @dcollinsnFound it. Patch forthcoming, once I write a test. -- |
From [Unknown Contact. See original ticket]Found it. Patch forthcoming, once I write a test. -- |
From @dcollinsnThe problem was that the patch changed the "ops list" hash from having '1' as the value to having 'undef'. This was checked in two places in order to determine if a warning should be issued - but only one of them was changed. I've attached a patch to correct the bug, and to test for it. -- |
From @dcollinsn0001-RT-129851-overload.pm-add-a-missing-exists.patchFrom 2505c9c45649eed83cc03eb1e008cbfbd1fef9f9 Mon Sep 17 00:00:00 2001
From: Dan Collins <dcollinsn@gmail.com>
Date: Tue, 11 Oct 2016 14:54:16 -0400
Subject: [PATCH] [RT #129851] overload.pm: add a missing 'exists'
607ee4356 changed the hash of permitted ops from having '1' as a value
to having undef as a value. This also changed one of the warning points
from checking for truthiness to existence. However, a second warning
was accidentally left checking for truthiness. This commit fixes that
oversight, and adds a regression test for warnings in this case.
---
lib/overload.pm | 2 +-
t/lib/overload_fallback.t | 14 +++++++++++++-
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/lib/overload.pm b/lib/overload.pm
index 758b67d..df074df 100644
--- a/lib/overload.pm
+++ b/lib/overload.pm
@@ -63,7 +63,7 @@ sub unimport {
*{$package . "::(("} = \&nil;
for (@_) {
warnings::warnif("overload arg '$_' is invalid")
- unless $ops_seen{$_};
+ unless exists $ops_seen{$_};
delete $ {$package . "::"}{$_ eq 'fallback' ? '()' : "(" .$_};
}
}
diff --git a/t/lib/overload_fallback.t b/t/lib/overload_fallback.t
index 6b50042..802fec3 100644
--- a/t/lib/overload_fallback.t
+++ b/t/lib/overload_fallback.t
@@ -1,6 +1,6 @@
use warnings;
use strict;
-use Test::Simple tests => 2;
+use Test::Simple tests => 3;
use overload '""' => sub { 'stringvalue' }, fallback => 1;
@@ -16,3 +16,15 @@ use overload '+' => sub { die "unused"; };
my $x = bless {}, 'main';
ok (eval {$x eq 'stringvalue'}, 'fallback worked again');
+{
+ my $warned = 0;
+ local $SIG{__WARN__} = sub { $warned++; };
+
+ eval q{
+ use overload '${}', 'fallback';
+ no overload '${}', 'fallback';
+ };
+
+ ok($warned == 0, 'no overload should not warn');
+}
+
--
2.9.3
|
From [Unknown Contact. See original ticket]The problem was that the patch changed the "ops list" hash from having '1' as the value to having 'undef'. This was checked in two places in order to determine if a warning should be issued - but only one of them was changed. I've attached a patch to correct the bug, and to test for it. -- |
From @jkeenanOn Tue Oct 11 12:10:37 2016, dcollinsn@gmail.com wrote:
Pushed for testing or for testing the failing module to: -- |
The RT System itself - Status changed from 'new' to 'open' |
From @jkeenanOn Tue, 11 Oct 2016 19:10:37 GMT, dcollinsn@gmail.com wrote:
Dan, I reviewed the status of this RT today. The branch I created for smoke-testing didn't show great results. I tried to apply your patch to blead and got merge conflicts. t/lib/overload.t may have changed. Can you investigate? Thank you very much. -- |
From @jkeenanOn Tue, 06 Dec 2016 21:37:27 GMT, jkeenan wrote:
Dan, when you get a chance, could you review this RT? Thank you very much. -- |
From @iabynOn Sun, Dec 25, 2016 at 06:05:32PM -0800, James E Keenan via RT wrote:
I've just updated the commit to work with blead, confirmed that it fixes -- |
From @jkeenanOn Mon, 26 Dec 2016 12:13:41 GMT, davem wrote:
I was able to build a threaded DEBUGGING perl at this commit point: ##### ... and then use 'cpanm' to successfully test and install Devel-EnforceEncapsulation. Resolving ticket. -- |
@jkeenan - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#129851 (status was 'resolved')
Searchable as RT129851$
The text was updated successfully, but these errors were encountered: