From e6ae140be50febcc553136016dc238d3ac6e259e Mon Sep 17 00:00:00 2001 From: Jonathan Rubin Date: Fri, 18 May 2018 16:35:32 -0400 Subject: [PATCH 1/4] Test that DEMOLISH usage won't emit warnings --- t/bugs/DEMOLISH_warnings.t | 15 +++++++++++++++ t/lib/Demolition/Demolisher.pm | 6 ++++++ t/lib/Demolition/OnceRemoved.pm | 9 +++++++++ 3 files changed, 30 insertions(+) create mode 100644 t/bugs/DEMOLISH_warnings.t create mode 100644 t/lib/Demolition/Demolisher.pm create mode 100644 t/lib/Demolition/OnceRemoved.pm diff --git a/t/bugs/DEMOLISH_warnings.t b/t/bugs/DEMOLISH_warnings.t new file mode 100644 index 000000000..0db745374 --- /dev/null +++ b/t/bugs/DEMOLISH_warnings.t @@ -0,0 +1,15 @@ +use strict; +use warnings; + +use lib 't/lib'; +use Test::More tests => 1; + +my @warnings; +BEGIN { + $SIG{__WARN__} = sub { push @warnings, @_ }; +} + +use Demolition::OnceRemoved; + +is scalar @warnings, 0, "No warnings" + or diag explain \@warnings; diff --git a/t/lib/Demolition/Demolisher.pm b/t/lib/Demolition/Demolisher.pm new file mode 100644 index 000000000..f1a7c1118 --- /dev/null +++ b/t/lib/Demolition/Demolisher.pm @@ -0,0 +1,6 @@ +package Demolition::Demolisher; +use Moose; + +sub DEMOLISH { } + +1; diff --git a/t/lib/Demolition/OnceRemoved.pm b/t/lib/Demolition/OnceRemoved.pm new file mode 100644 index 000000000..217f95af5 --- /dev/null +++ b/t/lib/Demolition/OnceRemoved.pm @@ -0,0 +1,9 @@ +package Demolition::OnceRemoved; +use strict; +use warnings; +use Demolition::Demolisher; + +my $d = Demolition::Demolisher->new; +$d->DEMOLISH; + +1; From 885f2b38dacf6396991cab13776a198ff66775e4 Mon Sep 17 00:00:00 2001 From: Jonathan Rubin Date: Fri, 18 May 2018 16:36:56 -0400 Subject: [PATCH 2/4] Suppress "DEMOLISH used only once" warnings See https://rt.cpan.org/Public/Bug/Display.html?id=124194 for more details --- lib/Moose/Object.pm | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/Moose/Object.pm b/lib/Moose/Object.pm index a15005d82..3e6c4fa47 100644 --- a/lib/Moose/Object.pm +++ b/lib/Moose/Object.pm @@ -78,7 +78,10 @@ sub DEMOLISHALL { foreach my $class (@isa) { no strict 'refs'; - my $demolish = *{"${class}::DEMOLISH"}{CODE}; + my $demolish = do { + no warnings 'once'; + *{"${class}::DEMOLISH"}{CODE}; + }; $self->$demolish($in_global_destruction) if defined $demolish; } From 1f0ebe400ef9b7f98f6895975aa272a513e25c47 Mon Sep 17 00:00:00 2001 From: Jonathan Rubin Date: Wed, 27 Jun 2018 10:28:36 -0400 Subject: [PATCH 3/4] Demolish warnings-removal tweaks Per pull request #167, add comments better explaining why these changes are necessary, and use Test::Warnings --- lib/Moose/Object.pm | 2 ++ t/bugs/DEMOLISH_warnings.t | 12 +++--------- t/lib/Demolition/OnceRemoved.pm | 4 +++- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/Moose/Object.pm b/lib/Moose/Object.pm index 3e6c4fa47..69c471bf4 100644 --- a/lib/Moose/Object.pm +++ b/lib/Moose/Object.pm @@ -78,6 +78,8 @@ sub DEMOLISHALL { foreach my $class (@isa) { no strict 'refs'; + # If a child module implements DEMOLISH and its parent does not + # then the access below can be the only reference to that parent's sub my $demolish = do { no warnings 'once'; *{"${class}::DEMOLISH"}{CODE}; diff --git a/t/bugs/DEMOLISH_warnings.t b/t/bugs/DEMOLISH_warnings.t index 0db745374..3a9de3ff9 100644 --- a/t/bugs/DEMOLISH_warnings.t +++ b/t/bugs/DEMOLISH_warnings.t @@ -2,14 +2,8 @@ use strict; use warnings; use lib 't/lib'; -use Test::More tests => 1; - -my @warnings; -BEGIN { - $SIG{__WARN__} = sub { push @warnings, @_ }; -} +use Test::More tests => 2; # Test::Warnings re-tests had_no_warnings implicitly +use Test::Requires qw(Test::Warnings); use Demolition::OnceRemoved; - -is scalar @warnings, 0, "No warnings" - or diag explain \@warnings; +Test::Warnings::had_no_warnings("No DEMOLISH warnings"); diff --git a/t/lib/Demolition/OnceRemoved.pm b/t/lib/Demolition/OnceRemoved.pm index 217f95af5..c7f2e3b8f 100644 --- a/t/lib/Demolition/OnceRemoved.pm +++ b/t/lib/Demolition/OnceRemoved.pm @@ -3,7 +3,9 @@ use strict; use warnings; use Demolition::Demolisher; +# This variable is only in scope during the initial `use` +# As it leaves scope, Perl will call DESTROY on it +# (and Moose::Object will then go through its DEMOLISHALL method) my $d = Demolition::Demolisher->new; -$d->DEMOLISH; 1; From a0361cb57a3dcd9261d173f6d1add1b9c1c6064f Mon Sep 17 00:00:00 2001 From: Jonathan Rubin Date: Tue, 10 Jul 2018 17:21:22 -0400 Subject: [PATCH 4/4] Review Tweaks - take two Again per pull request #167, move some comments around, and better use Test::Warnings --- t/bugs/DEMOLISH_warnings.t | 8 +++++++- t/lib/Demolition/OnceRemoved.pm | 3 --- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/t/bugs/DEMOLISH_warnings.t b/t/bugs/DEMOLISH_warnings.t index 3a9de3ff9..bcb63cdf8 100644 --- a/t/bugs/DEMOLISH_warnings.t +++ b/t/bugs/DEMOLISH_warnings.t @@ -2,8 +2,14 @@ use strict; use warnings; use lib 't/lib'; -use Test::More tests => 2; # Test::Warnings re-tests had_no_warnings implicitly +use Test::More; use Test::Requires qw(Test::Warnings); +Test::Warnings->import(':no_end_test'); +# Demolition::OnceRemoved has a variable only in scope during the initial `use` +# As it leaves scope, Perl will call DESTROY on it +# (and Moose::Object will then go through its DEMOLISHALL method) use Demolition::OnceRemoved; Test::Warnings::had_no_warnings("No DEMOLISH warnings"); + +done_testing(); diff --git a/t/lib/Demolition/OnceRemoved.pm b/t/lib/Demolition/OnceRemoved.pm index c7f2e3b8f..8b52d126b 100644 --- a/t/lib/Demolition/OnceRemoved.pm +++ b/t/lib/Demolition/OnceRemoved.pm @@ -3,9 +3,6 @@ use strict; use warnings; use Demolition::Demolisher; -# This variable is only in scope during the initial `use` -# As it leaves scope, Perl will call DESTROY on it -# (and Moose::Object will then go through its DEMOLISHALL method) my $d = Demolition::Demolisher->new; 1;