"warnings" pragma silently ignores postfixed "FATAL" import parameter

[p5pRT]

Migrated from rt.perl.org#128540 (status was 'open')

Searchable as RT128540$

[p5pRT]

From the.rob.dixon@gmail.com

This is a bug report for perl from the.rob.dixon@​gmail.com,
generated with the help of perlbug 1.40 running under perl 5.24.0.

---

The documentation for the `warnings` pragma has this

* The presence of the word "FATAL" in the category list will escalate*
* warnings in those categories into fatal errors in that lexical scope.*

which works fine for the examples given, like

  use warnings FATAL => 'all'

This also works fine if the category is defaulted. For instance

  use warnings 'FATAL'

is equivalent to the previous example.

`FATAL` may be also used to separate fatal categories from non-fatal ones,
so that

  use warnings qw/ uninitialized FATAL deprecated experimental /

makes "deprecated" and "experimental" warnings fatal while "uninitialized"
warnings
are enabled but non-fatal

However the `FATAL` is silently ineffective if it is used without any
following
categories.

The doeumentation would have us believe that

  use warnings qw/ all FATAL /

should work fine, but the FATAL has no effect at all and gives no message
to say so.
This is identical to `use warnings`

I suggest that this should result in a compilation failure in the same way
as
"Unknown warnings category 'xx'". The documentation should also be
corrected to
explain that the position of `FATAL` within the import parameters is
significant

---

---

Flags​:
  category=core
  severity=medium

---

Site configuration information for perl 5.24.0​:

Configured by strawberry-perl at Tue May 10 21​:33​:22 2016.

Summary of my perl5 (revision 5 version 24 subversion 0) configuration​:

  Platform​:
  osname=MSWin32, osvers=6.3, archname=MSWin32-x64-multi-thread
  uname='Win32 strawberry-perl 5.24.0.1 #1 Tue May 10 21​:30​:49 2016 x64'
  config_args='undef'
  hint=recommended, useposix=true, d_sigaction=undef
  useithreads=define, usemultiplicity=define
  use64bitint=define, use64bitall=undef, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='gcc', ccflags =' -s -O2 -DWIN32 -DWIN64 -DCONSERVATIVE
-DPERL_TEXTMODE_SCRIPTS -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS
-fwrapv -fno-strict-aliasing -mms-bitfields',
  optimize='-s -O2',
  cppflags='-DWIN32'
  ccversion='', gccversion='4.9.2', gccosandvers=''
  intsize=4, longsize=4, ptrsize=8, doublesize=8, byteorder=12345678,
doublekind=3
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16,
longdblkind=3
  ivtype='long long', ivsize=8, nvtype='double', nvsize=8, Off_t='long
long', lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='g++', ldflags ='-s -L"C​:\strawberry\perl\lib\CORE"
-L"C​:\strawberry\c\lib"'
  libpth=C​:\strawberry\c\lib C​:\strawberry\c\x86_64-w64-mingw32\lib
C​:\strawberry\c\lib\gcc\x86_64-w64-mingw32\4.9.2
  libs=-lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32
-ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr
-lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32
  perllibs=-lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32
-ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr
-lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32
  libc=, so=dll, useshrplib=true, libperl=libperl524.a
  gnulibc_version=''
  Dynamic Linking​:
  dlsrc=dl_win32.xs, dlext=xs.dll, d_dlsymun=undef, ccdlflags=' '
  cccdlflags=' ', lddlflags='-mdll -s -L"C​:\strawberry\perl\lib\CORE"
-L"C​:\strawberry\c\lib"'

---

@​INC for perl 5.24.0​:
  C​:/Strawberry/perl/site/lib/MSWin32-x64-multi-thread
  C​:/Strawberry/perl/site/lib
  C​:/Strawberry/perl/vendor/lib
  C​:/Strawberry/perl/lib
  .

---

Environment for perl 5.24.0​:
  HOME (unset)
  LANG (unset)
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=C​:\PHP\;C​:\ProgramData\Oracle\Java\javapath;C​:\Program Files
(x86)\NVIDIA
Corporation\PhysX\Common;C​:\WINDOWS\system32;C​:\WINDOWS;C​:\WINDOWS\System32\Wbem;C​:\WINDOWS\System32\WindowsPowerShell\v1.0\;C​:\Strawberry\perl\bin;C​:\Strawberry\perl\site\bin;C​:\Strawberry\c\bin;C​:\Python27\;C​:\Python27\Scripts;E​:\Perl\source\public;C​:\ProgramData\chocolatey\bin;C​:\Program
Files\Git\cmd;C​:\ffmpeg\bin;C​:\SQLite;C​:\PHP;C​:\tools\php
  PERL_BADLANG (unset)
  SHELL (unset)

[p5pRT]

@dcollinsn - Status changed from 'new' to 'open'

[p5pRT]

From @dcollinsn

Two parts to this​: one is fixing the documentation (patch 0001), the other is adding the proposed warning (patch 0002). Both seem pretty straightforward.

I can't imagine that this is an issue that comes up very often, but it doesn't cost much to add the warning. I tested it (temporarily making it a croak), and ran `make test` - no tests failed (yet), so this doesn't break anything in our test suite that currently works. However, there's no guarantee that somewhere, someone doesn't have something like

  use warnings 'all', 'FATAL' => @​fatal_warnings;

who is using this behavior on purpose, and is now going to get an unnecessary warning.

Example​:

  $ ./perl -Ilib -e 'use warnings qw(all); $a = test; print "Alive!\n";'
  Unquoted string "test" may clash with future reserved word at -e line 1.
  Alive!
  $ ./perl -Ilib -e 'use warnings qw(FATAL); $a = test; print "Alive!\n";'
  Unquoted string "test" may clash with future reserved word at -e line 1.
  $ ./perl -Ilib -e 'use warnings qw(all FATAL); $a = test; print "Alive!\n";'
  FATAL or NONFATAL as final warnings category has no effect at -e line 1.
  Unquoted string "test" may clash with future reserved word at -e line 1.
  Alive!

[p5pRT]

From @dcollinsn

0001-RT-128540-lib-warnings.pm-documentation-for-FATAL.patch

From 86200781a075b815ae9c9733785bb79f49a93408 Mon Sep 17 00:00:00 2001
From: Dan Collins <dcollinsn@gmail.com>
Date: Tue, 5 Jul 2016 10:41:31 -0400
Subject: [PATCH 1/2] [RT #128540] lib/warnings.pm: documentation for FATAL

---
 lib/warnings.pm   | 16 +++++++++++++++-
 regen/warnings.pl | 16 +++++++++++++++-
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/lib/warnings.pm b/lib/warnings.pm
index d607f31..cb7d99c 100644
--- a/lib/warnings.pm
+++ b/lib/warnings.pm
@@ -872,7 +872,8 @@ is now a top-level category in its own right.
 X<warning, fatal>
 
 The presence of the word "FATAL" in the category list will escalate
-warnings in those categories into fatal errors in that lexical scope.
+warnings in categories following the word "FATAL" into fatal errors
+in that lexical scope.
 
 B<NOTE:> FATAL warnings should be used with care, particularly
 C<< FATAL => 'all' >>.
@@ -959,6 +960,19 @@ except for those in the "syntax" category.
 
     use warnings FATAL => 'all', NONFATAL => 'syntax';
 
+Note that the location of the word "FATAL" or "NONFATAL" in the argument
+list is relevant, as it only applies to warning categories following it.
+So:
+
+    use warnings qw(all FATAL experimental)
+
+will enable all warnings, but only experimental warnings will be made
+fatal. Similarly:
+
+    use warnings qw(all FATAL)
+
+will not make any warnings fatal.
+
 As of Perl 5.20, instead of C<< use warnings FATAL => 'all'; >> you can
 use:
 
diff --git a/regen/warnings.pl b/regen/warnings.pl
index 815c735..4d1cde1 100644
--- a/regen/warnings.pl
+++ b/regen/warnings.pl
@@ -1001,7 +1001,8 @@ is now a top-level category in its own right.
 X<warning, fatal>
 
 The presence of the word "FATAL" in the category list will escalate
-warnings in those categories into fatal errors in that lexical scope.
+warnings in categories following the word "FATAL" into fatal errors
+in that lexical scope.
 
 B<NOTE:> FATAL warnings should be used with care, particularly
 C<< FATAL => 'all' >>.
@@ -1088,6 +1089,19 @@ except for those in the "syntax" category.
 
     use warnings FATAL => 'all', NONFATAL => 'syntax';
 
+Note that the location of the word "FATAL" or "NONFATAL" in the argument
+list is relevant, as it only applies to warning categories following it.
+So:
+
+    use warnings qw(all FATAL experimental)
+
+will enable all warnings, but only experimental warnings will be made
+fatal. Similarly:
+
+    use warnings qw(all FATAL)
+
+will not make any warnings fatal.
+
 As of Perl 5.20, instead of C<< use warnings FATAL => 'all'; >> you can
 use:
 
-- 
2.8.1

[p5pRT]

From @dcollinsn

0002-RT-128540-lib-warnings.pm-warn-for-useless-FATAL.patch

From 5f919e998f53df04355376129330b70e5c7e7746 Mon Sep 17 00:00:00 2001
From: Dan Collins <dcollinsn@gmail.com>
Date: Tue, 5 Jul 2016 10:42:29 -0400
Subject: [PATCH 2/2] [RT #128540] lib/warnings.pm: warn for useless FATAL

When FATAL (or NONFATAL) is used as the only warnings category,
it is interpreted as FATAL => all. However, when it is used as
the last of several categories, it silently has no effect. This
adds a warning for this case.
---
 lib/warnings.pm   | 13 +++++++++++++
 regen/warnings.pl | 13 +++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/lib/warnings.pm b/lib/warnings.pm
index cb7d99c..ee590ee 100644
--- a/lib/warnings.pm
+++ b/lib/warnings.pm
@@ -249,6 +249,14 @@ sub Croaker
     Carp::croak(@_);
 }
 
+sub Carper
+{
+    require Carp; # this initializes %CarpInternal
+    local $Carp::CarpInternal{'warnings'};
+    delete $Carp::CarpInternal{'warnings'};
+    Carp::carp(@_);
+}
+
 sub _bits {
     my $mask = shift ;
     my $catmask ;
@@ -299,6 +307,11 @@ sub import
 
     # Empty @_ is equivalent to @_ = 'all' ;
     ${^WARNING_BITS} = @_ ? _bits($mask, @_) : $mask | $Bits{all} ;
+
+    # Warn if last word of import directive is FATAL or NONFATAL
+    # (except for lone FATAL or NONFATAL)
+    Carper("FATAL or NONFATAL as final warnings category has no effect")
+        if @_>1 && ( $_[-1] eq 'FATAL' || $_[-1] eq 'NONFATAL' );
 }
 
 sub unimport
diff --git a/regen/warnings.pl b/regen/warnings.pl
index 4d1cde1..e771b08 100644
--- a/regen/warnings.pl
+++ b/regen/warnings.pl
@@ -508,6 +508,14 @@ sub Croaker
     Carp::croak(@_);
 }
 
+sub Carper
+{
+    require Carp; # this initializes %CarpInternal
+    local $Carp::CarpInternal{'warnings'};
+    delete $Carp::CarpInternal{'warnings'};
+    Carp::carp(@_);
+}
+
 sub _bits {
     my $mask = shift ;
     my $catmask ;
@@ -558,6 +566,11 @@ sub import
 
     # Empty @_ is equivalent to @_ = 'all' ;
     ${^WARNING_BITS} = @_ ? _bits($mask, @_) : $mask | $Bits{all} ;
+
+    # Warn if last word of import directive is FATAL or NONFATAL
+    # (except for lone FATAL or NONFATAL)
+    Carper("FATAL or NONFATAL as final warnings category has no effect")
+        if @_>1 && ( $_[-1] eq 'FATAL' || $_[-1] eq 'NONFATAL' );
 }
 
 sub unimport
-- 
2.8.1

[p5pRT]

From [Unknown Contact. See original ticket]

Two parts to this​: one is fixing the documentation (patch 0001), the other is adding the proposed warning (patch 0002). Both seem pretty straightforward.

I can't imagine that this is an issue that comes up very often, but it doesn't cost much to add the warning. I tested it (temporarily making it a croak), and ran `make test` - no tests failed (yet), so this doesn't break anything in our test suite that currently works. However, there's no guarantee that somewhere, someone doesn't have something like

  use warnings 'all', 'FATAL' => @​fatal_warnings;

who is using this behavior on purpose, and is now going to get an unnecessary warning.

Example​:

  $ ./perl -Ilib -e 'use warnings qw(all); $a = test; print "Alive!\n";'
  Unquoted string "test" may clash with future reserved word at -e line 1.
  Alive!
  $ ./perl -Ilib -e 'use warnings qw(FATAL); $a = test; print "Alive!\n";'
  Unquoted string "test" may clash with future reserved word at -e line 1.
  $ ./perl -Ilib -e 'use warnings qw(all FATAL); $a = test; print "Alive!\n";'
  FATAL or NONFATAL as final warnings category has no effect at -e line 1.
  Unquoted string "test" may clash with future reserved word at -e line 1.
  Alive!