-
Notifications
You must be signed in to change notification settings - Fork 562
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
[PATCH] "use warnings 'FATAL';" and related have no effect #13521
Comments
From @haukexDear Perl 5 Porters, The statements use warnings "FATAL"; have no effect on the status of warnings at all. An unsuspecting user may (accidentally) use one of the first two and At the very least, I think the above should generate warnings or errors However, after thinking about it a bit, what makes the most sense to me use warnings FATAL => "all"; Based on this idea I created a patch which I have attached for your I've also included a patch that fixes a missing word in perllexwarn.pod. Best Regards, Flags: Site configuration information for perl 5.19.8: Configured by haukex at Mon Jan 13 02:26:42 CET 2014. Summary of my perl5 (revision 5 version 19 subversion 8) configuration: @INC for perl 5.19.8: Environment for perl 5.19.8: PATH=/opt/perl5.18/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games |
From @haukexFrom 611d05d25a8e43e1f29118b7b29e4300d5463eef Mon Sep 17 00:00:00 2001 This is a multi-part message in MIME format. pod/perllexwarn.pod | 2 +- --------------1.8.5.2 Inline Patchdiff --git a/pod/perllexwarn.pod b/pod/perllexwarn.pod
index 0c849a4..0e65cbc 100644
--- a/pod/perllexwarn.pod
+++ b/pod/perllexwarn.pod
@@ -386,7 +386,7 @@ When run it produces this output
Useless use of length in void context at fatal line 7.
The scope where C<length> is used has escalated the C<void> warnings
-category into a fatal error, so the program terminates immediately it
+category into a fatal error, so the program terminates immediately when it
encounters the warning.
To explicitly turn off a "FATAL" warning you just disable the warning
--------------1.8.5.2--
This is a multi-part message in MIME format. Until now, the statements use warnings "FATAL"; had no effect on the status of warnings at all. This change causes them AUTHORS | 1 + --------------1.8.5.2 Inline Patchdiff --git a/AUTHORS b/AUTHORS
index 2d6c583..65eae39 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -459,6 +459,7 @@ Harmen <[email protected]>
Harmon S. Nine <[email protected]>
Harri Pasanen <[email protected]>
Harry Edmon <[email protected]>
+Hauke D <[email protected]>
Helmut Jarausch <[email protected]>
Henrik Tougaard <[email protected]>
Herbert Breunung <[email protected]>
diff --git a/lib/warnings.pm b/lib/warnings.pm
index 440bfdd..119ca3e 100644
--- a/lib/warnings.pm
+++ b/lib/warnings.pm
@@ -5,7 +5,7 @@
package warnings;
-our $VERSION = '1.20';
+our $VERSION = '1.21';
# Verify that we're called correctly so that warnings will work.
# see also strict.pm.
@@ -421,6 +421,9 @@ sub import
$mask |= $Bits{'all'} ;
$mask |= $DeadBits{'all'} if vec($mask, $Offsets{'all'}+1, 1);
}
+
+ # append 'all' when implied (after a lone "FATAL" or "NONFATAL")
+ push @_, 'all' if @_==1 && ( $_[0] eq 'FATAL' || $_[0] eq 'NONFATAL' );
# Empty @_ is equivalent to @_ = 'all' ;
${^WARNING_BITS} = @_ ? _bits($mask, @_) : $mask | $Bits{all} ;
@@ -438,7 +441,8 @@ sub unimport
$mask |= $DeadBits{'all'} if vec($mask, $Offsets{'all'}+1, 1);
}
- push @_, 'all' unless @_;
+ # append 'all' when implied (empty import list or after a lone "FATAL")
+ push @_, 'all' if !@_ || @_==1 && $_[0] eq 'FATAL';
foreach my $word ( @_ ) {
if ($word eq 'FATAL') {
diff --git a/pod/perllexwarn.pod b/pod/perllexwarn.pod
index 0e65cbc..b00db82 100644
--- a/pod/perllexwarn.pod
+++ b/pod/perllexwarn.pod
@@ -403,6 +403,20 @@ except for those in the "syntax" category.
use warnings FATAL => 'all', NONFATAL => 'syntax';
+As of Perl 5.20, you can shorten C<< use warnings FATAL => 'all'; >> like
+this:
+
+ use v5.20; # this is *required* for the following line to work
+ use warnings 'FATAL'; # normally: use warnings FATAL => 'all';
+
+Because this requires Perl 5.20, you should not use it if you want your
+scripts to remain backwards compatible, and spell it out instead.
+(In previous versions of Perl, the statements
+C<< use warnings 'FATAL'; >>, C<< use warnings 'NONFATAL'; >>
+and C<< no warnings 'FATAL'; >> actually had no effect on the status of
+warnings at all. As of Perl 5.20 they behave as if they included the
+C<< => 'all' >> portion.)
+
B<NOTE:> Users of FATAL warnings, especially
those using C<< FATAL => 'all' >>
should be fully aware that they are risking future portability of their
diff --git a/regen/warnings.pl b/regen/warnings.pl
index e8dcf4a..72774ca 100644
--- a/regen/warnings.pl
+++ b/regen/warnings.pl
@@ -476,7 +476,7 @@ close_and_rename($lexwarn);
__END__
package warnings;
-our $VERSION = '1.20';
+our $VERSION = '1.21';
# Verify that we're called correctly so that warnings will work.
# see also strict.pm.
@@ -691,6 +691,9 @@ sub import
$mask |= $Bits{'all'} ;
$mask |= $DeadBits{'all'} if vec($mask, $Offsets{'all'}+1, 1);
}
+
+ # append 'all' when implied (after a lone "FATAL" or "NONFATAL")
+ push @_, 'all' if @_==1 && ( $_[0] eq 'FATAL' || $_[0] eq 'NONFATAL' );
# Empty @_ is equivalent to @_ = 'all' ;
${^WARNING_BITS} = @_ ? _bits($mask, @_) : $mask | $Bits{all} ;
@@ -708,7 +711,8 @@ sub unimport
$mask |= $DeadBits{'all'} if vec($mask, $Offsets{'all'}+1, 1);
}
- push @_, 'all' unless @_;
+ # append 'all' when implied (empty import list or after a lone "FATAL")
+ push @_, 'all' if !@_ || @_==1 && $_[0] eq 'FATAL';
foreach my $word ( @_ ) {
if ($word eq 'FATAL') {
diff --git a/t/lib/warnings/7fatal b/t/lib/warnings/7fatal
index 6eeac74..add39b1 100644
--- a/t/lib/warnings/7fatal
+++ b/t/lib/warnings/7fatal
@@ -433,3 +433,59 @@ print STDERR "The End.\n" ;
EXPECT
Unsuccessful open on filename containing newline at - line 5.
close() on unopened filehandle fred at - line 6.
+########
+
+# 'use warnings NONFATAL=>"all"' should be the same as 'use warnings'
+use warnings NONFATAL=>"all" ;
+my $a = oct "7777777777777777777777777777777777778" ;
+EXPECT
+Integer overflow in octal number at - line 4.
+Illegal octal digit '8' ignored at - line 4.
+Octal number > 037777777777 non-portable at - line 4.
+########
+
+# 'use warnings "NONFATAL"' should be the same as 'use warnings'
+use warnings "NONFATAL";
+my $a = oct "7777777777777777777777777777777777778" ;
+EXPECT
+Integer overflow in octal number at - line 4.
+Illegal octal digit '8' ignored at - line 4.
+Octal number > 037777777777 non-portable at - line 4.
+########
+
+# 'use warnings "FATAL"' should be the same as 'use warnings FATAL=>"all"'
+use warnings "FATAL" ;
+{
+ no warnings ;
+ my $a =+ 1 ;
+}
+my $a =+ 1 ;
+print STDERR "The End.\n" ;
+EXPECT
+Reversed += operator at - line 8.
+########
+
+# 'no warnings FATAL=>"all"' should be the same as 'no warnings'
+use warnings ;
+{
+ no warnings FATAL=>"all" ;
+ my $a =+ 1 ;
+}
+my $a =+ 1 ;
+print STDERR "The End.\n" ;
+EXPECT
+Reversed += operator at - line 8.
+The End.
+########
+
+# 'no warnings "FATAL"' should be the same as 'no warnings'
+use warnings ;
+{
+ no warnings "FATAL" ;
+ my $a =+ 1 ;
+}
+my $a =+ 1 ;
+print STDERR "The End.\n" ;
+EXPECT
+Reversed += operator at - line 8.
+The End.
--------------1.8.5.2-- |
From @tonycozOn Sun Jan 12 18:23:01 2014, haukex@zero-g.net wrote:
This change makes sense to me. The only possible problem I can see is if some module is calling I plan to apply this unless someone responds with an argument against it. Tony |
The RT System itself - Status changed from 'new' to 'open' |
From @haukexOn Mon Jan 13 16:22:34 2014, tonyc wrote:
Hi, Thanks for taking this! I investigated a bit more and found that while the central point of the issue is still the same, it wasn't 100% accurate to say there is currently no effect on warnings at all. I've attached a quick test script to look at how the warning bits change. On Perl v5.10.1, the statements really do have no effect on the warnings status at all. On Perl v5.18.2, it looks like only the $DEFAULT warnings are turned on, but not made fatal by both "use warnings 'NONFATAL';" and "use warnings 'FATAL';". That behavior still doesn't really make sense and I doubt if anyone is using it in this undocumented way on purpose, so I'm sticking with my suggestion. So I could make the perllexwarn wording more accurate and provide an updated patch tomorrow or the day after, if that's not too late. Regards, P.S. I sent a similar comment to perlbug-followup@perl.org yesterday but it hasn't shown up here yet, sorry if this turns out to be a dupe. |
From @haukex#!/usr/bin/env perl print "This is Perl $^V\n"; sub show { print sprintf("%-24s: ",shift),unpack("b*",${^WARNING_BITS}),"\n" } BEGIN { show "before" } no warnings "FATAL"; use warnings; |
From @haukexHi, Investigating this a little further shows that while the spirit of the See the attached test script: on Perl v5.18.2, it looks like the default Best Regards, |
From @haukex#!/usr/bin/env perl print "This is Perl $^V\n"; sub show { print sprintf("%-24s: ",shift),unpack("b*",${^WARNING_BITS}),"\n" } BEGIN { show "before" } no warnings "FATAL"; use warnings; |
From @khwilliamsonOn 01/13/2014 09:14 PM, Hauke D via RT wrote:
It's never too late; although at times it may be too late for the |
From @tonycozOn Mon Jan 13 20:14:59 2014, haukex@zero-g.net wrote:
An updated patch is fine. I don't think this is so super-critical that is has to go in 5.20 - and Tony |
From @haukexHi, Attached is a new version of the patch. I've updated the documentation and expanded the tests a little bit. Also, this patch doesn't contain the warnings.pm version number update from 1.20 to 1.21 like it did before because the recent commit 2fe4abd did the same thing and I wasn't sure if the version number needs to be upped again? Unless anyone notices anything else I think this patch should be all set. Regards, |
From @haukex |
From @tonycozOn Wed Jan 15 16:45:25 2014, haukex@zero-g.net wrote:
Applied as e214621 and c91312d (with a minor AUTHORS alignment fix), with a version bump in 76ff28b, since there's been a release. Tony |
@tonycoz - Status changed from 'open' to 'resolved' |
From @haukexHi, Thanks! Regards, Am 22.01.2014 06:37, schrieb Tony Cook via RT:
|
Migrated from rt.perl.org#120977 (status was 'resolved')
Searchable as RT120977$
The text was updated successfully, but these errors were encountered: