no strict 'garbage'

[p5pRT]

Migrated from rt.perl.org#17061 (status was 'resolved')

Searchable as RT17061$

[p5pRT]

From @lizmat

Created by @lizmat

I just spent the better part of an hour wondering why a redefine error wouldn't go away. It was caused by me doing a​:

  no strict 'redefine';

rather than a​:

  no warnings 'redefine';

but strict.pm didn't complain\, so I never realised that it was there where the error was.

Included is a patch that should cause the above to warn with​:

  Don't know how to "no strict qw(redefine)"

A patch for the test is also attached.

Perl Info

``` Flags: category=core severity=low Site configuration information for perl v5.8.0: Configured by liz at Tue Aug 20 12:44:35 CEST 2002. Summary of my perl5 (revision 5.0 version 8 subversion 0) configuration: Platform: osname=linux, osvers=2.4.18, archname=i686-linux-thread-multi uname='linux echt.dijkmat.nl 2.4.18 #8 smp mon mar 25 22:28:36 cet 2002 i686 unknown ' config_args='-de -Dusethreads' hint=recommended, useposix=true, d_sigaction=define usethreads=define use5005threads=undef useithreads=define usemultiplicity=define useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm', optimize='-O2', cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -I/usr/local/include -I/usr/include/gdbm' ccversion='', gccversion='2.96 20000731 (Red Hat Linux 7.1 2.96-98)', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=4, prototype=define Linker and Libraries: ld='cc', ldflags =' -L/usr/local/lib' libpth=/usr/local/lib /lib /usr/lib libs=-lnsl -lgdbm -ldb -ldl -lm -lpthread -lc -lcrypt -lutil perllibs=-lnsl -ldl -lm -lpthread -lc -lcrypt -lutil libc=/lib/libc-2.2.4.so, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.2.4' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic' cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib' Locally applied patches: @INC for perl v5.8.0: /usr/local/lib/perl5/5.8.0/i686-linux-thread-multi /usr/local/lib/perl5/5.8.0 /usr/local/lib/perl5/site_perl/5.8.0/i686-linux-thread-multi /usr/local/lib/perl5/site_perl/5.8.0 /usr/local/lib/perl5/site_perl/5.7.3 /usr/local/lib/perl5/site_perl . Environment for perl v5.8.0: HOME=/home/liz LANG=en_US LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/local/bin:/bin:/usr/bin:/usr/X11R6/bin:/home/liz/bin PERL_BADLANG (unset) SHELL=/bin/bash ```

[p5pRT]

From @lizmat

Inline Patch

```diff --- strict.pm.original Sun Jun 2 13:26:50 2002 +++ strict.pm Fri Sep 6 21:15:52 2002 @@ -100,7 +100,20 @@ sub bits { my $bits = 0; - foreach my $s (@_){ $bits |= $bitmask{$s} || 0; }; + my @wrong; + foreach my $s (@_){ + push( @wrong,$s ) unless exists $bitmask{$s}; + $bits |= $bitmask{$s} || 0; + } + if (@wrong) { + my $useno = { + __PACKAGE__.'::import' => 'use', + __PACKAGE__.'::unimport' => 'no' + }->{(caller(1))[3]}; + require Carp; + Carp::croak( + qq/Don't know how to "$useno /.__PACKAGE__.qq/ qw(@wrong)"/ ); + } $bits; } ```

[p5pRT]

From @lizmat

strict.t.patch

```diff --- strict.t.original Fri Sep 6 21:17:48 2002 +++ strict.t Fri Sep 6 21:26:51 2002 @@ -36,7 +36,7 @@ undef $/; -print "1..", scalar @prgs, "\n"; +print "1..", (scalar @prgs) + 1, "\n"; for (@prgs){ @@ -98,3 +98,6 @@ foreach (@temps) { unlink $_ if $_ } } + +eval qq(no strict 'garbage'); +print "ok ",++$i,"\n" if $@ =~ /^Don't know how to "no strict qw\(garbage\)/; ```

[p5pRT]

From @lizmat

Attachment includes an improved version of the test patch. It now also checks "use strict 'garbage'" and "use/no strict qw(foo bar)".

Liz

[p5pRT]

From @lizmat

Inline Patch

```diff --- strict.t.original Fri Sep 6 21:17:48 2002 +++ strict.t Sat Sep 7 13:41:41 2002 @@ -36,7 +36,7 @@ undef $/; -print "1..", scalar @prgs, "\n"; +print "1..", (scalar @prgs) + 4, "\n"; for (@prgs){ @@ -98,3 +98,15 @@ foreach (@temps) { unlink $_ if $_ } } + +eval qq(use strict 'garbage'); +print "ok ",++$i,"\n" if $@ =~ /^Don't know how to "use strict qw\(garbage\)/; + +eval qq(no strict 'garbage'); +print "ok ",++$i,"\n" if $@ =~ /^Don't know how to "no strict qw\(garbage\)/; + +eval qq(use strict qw(foo bar)); +print "ok ",++$i,"\n" if $@ =~ /^Don't know how to "use strict qw\(foo bar\)/; + +eval qq(no strict qw(foo bar)); +print "ok ",++$i,"\n" if $@ =~ /^Don't know how to "no strict qw\(foo bar\)/; ```

[p5pRT]

From @hvds

Elizabeth Mattijsen (via RT) \perlbug@&#8203;perl\.org wrote​: [ C\< no strict 'redefine'; > doesn't complain. ]

:Included is a patch that should cause the above to warn with​: : : Don't know how to "no strict qw(redefine)"

As I remember it\, this behaviour was deliberate so that if we added additional optional strictures in the future you could take advantage of them without your code breaking under older perls.

I'm not sure how relevant that approach still is; I'll put this patch in for now\, with the proviso that it may come out again before 5.10.

Hugo

[p5pRT]

From @hvds

Elizabeth Mattijsen (via RT) \perlbug@&#8203;perl\.org wrote​: :Included is a patch that should cause the above to warn with​: : : Don't know how to "no strict qw(redefine)"

Thanks\, this and the additional tests patch applied as #17869.

A few minor points for patches​: - please try to be strict in your use of standard formatting and whitespace - please supply diffs relative to the top level perl directory or higher - please don't supply components of a single change as multiple attachments; do that only if the changes are distinctly separate from each other - please try to make sure that tests print "not ok" if they fail

Cheers\,

Hugo

[p5pRT]

From @hvds

Elizabeth Mattijsen (via RT) \perlbug@&#8203;perl\.org wrote​: :Included is a patch that should cause the above to warn with​: : : Don't know how to "no strict qw(redefine)"

Hmm\, this causes a failure in ext/Storable/t/code.t\, I think because it is using a Safe compartment that thinks it knows everything a 'use strict qw/ refs /' can do. It's not clear to me what the fault is here.

Hugo

[p5pRT]

From @hvds

hv@​crypt.org wrote​: :Elizabeth Mattijsen (via RT) \perlbug@&#8203;perl\.org wrote​: :​:Included is a patch that should cause the above to warn with​: :​: :​: Don't know how to "no strict qw(redefine)" : :Hmm\, this causes a failure in ext/Storable/t/code.t\, I think :because it is using a Safe compartment that thinks it knows :everything a 'use strict qw/ refs /' can do. It's not clear to :me what the fault is here.

I've temporarily changed the core Storable to skip those tests until we sort this out\, as change #17872.

Hugo

[p5pRT]

From @nwc10

On Sun\, Sep 08\, 2002 at 05​:57​:44PM +0100\, hv@​crypt.org wrote​:

- please try to make sure that tests print "not ok" if they fail

File/Find/t/find.t tests 1 and 2 are doubly naughty

1​: they don't print not ok if they fail 2​: they happily print ok several times if they find more copies of the file   than they expect. This causes t/TEST to report a test failure at test 189   (one after the last test) which is somewhat confusing.

The appended patch cures both of these. I chose to use a package global rather than a lexical as it seemed the intent of the first two tests was to be simple\, and relying on lexical variables in closures (instead of simple anonymous subroutines) didn't feel "simple"

I wasn't sure if the whole test should assimilated by Test​::More-of-borg\, but I thought I'd leave a patch to the usual suspect :-)

Or maybe it should use the $case code.

Nicholas Clark -- Even better than the real thing​: http​://nms-cgi.sourceforge.net/

Inline Patch

```diff --- lib/File/Find/t/find.t.orig Mon Mar 11 06:01:17 2002 +++ lib/File/Find/t/find.t Sun Sep 8 17:42:00 2002 @@ -51,12 +51,23 @@ BEGIN { cleanup(); -find({wanted => sub { print "ok 1\n" if $_ eq 'commonsense.t'; } }, +$::count_commonsense = 0; +find({wanted => sub { ++$::count_commonsense if $_ eq 'commonsense.t'; } }, File::Spec->curdir); +if ($::count_commonsense == 1) { + print "ok 1\n"; +} else { + print "not ok 1 # found $::count_commonsense files named 'commonsense.t'\n"; +} -finddepth({wanted => sub { print "ok 2\n" if $_ eq 'commonsense.t'; } }, +$::count_commonsense = 0; +finddepth({wanted => sub { ++$::count_commonsense if $_ eq 'commonsense.t'; } }, File::Spec->curdir); - +if ($::count_commonsense == 1) { + print "ok 2\n"; +} else { + print "not ok 2 # found $::count_commonsense files named 'commonsense.t'\n"; +} my $case = 2; my $FastFileTests_OK = 0; ```

[p5pRT]

From @petdance

I wasn't sure if the whole test should assimilated by Test​::More-of-borg\, but I thought I'd leave a patch to the usual suspect :-)

As far as I'm concerned\, anything that uses funcs to print test results rather than hardcoded ok/not ok and test counts is a step in the right direction....

-- 'Andy Lester andy@​petdance.com Programmer/author petdance.com Daddy parsley.org/quinn Jk'=~/.+/s;print((split//\,$&)   [unpack'C*'\,"n2]3%+>\"34.'%&.'^%4+!o.'"])

[p5pRT]

From @eserte

hv@​crypt.org writes​:

Elizabeth Mattijsen (via RT) \perlbug@&#8203;perl\.org wrote​: :Included is a patch that should cause the above to warn with​: : : Don't know how to "no strict qw(redefine)"

Hmm\, this causes a failure in ext/Storable/t/code.t\, I think because it is using a Safe compartment that thinks it knows everything a 'use strict qw/ refs /' can do. It's not clear to me what the fault is here.

With the strict.pm patch\, "use strict" will fail in a safe compartment if "​:still_to_be_decided" or "caller" in not in the permitted opset. Try this script with and without the strict.pm patch​:

use Safe; $s = new Safe; $s->permit(qw(​:default require)); $s->reval("use strict; 1") or die $@​; __END__

Nevertheless\, here a patch for the code.t test if the strict.pm patch won't be retracted​:

Inline Patch

```diff --- ext/Storable/t/code.t.orig Sun Sep 8 22:04:11 2002 +++ ext/Storable/t/code.t Sun Sep 8 22:10:22 2002 @@ -52,13 +52,13 @@ { package Another::Package; sub foo { __PACKAGE__ } } @obj = - ([\&code, # code reference - sub { 6*7 }, - $blessed_code, # blessed code reference - \&Another::Package::foo, # code in another package - sub ($$;$) { 0 }, # prototypes - sub { print "test\n" }, - \&Test::ok, # large scalar + ([\&code, # 0: code reference + sub { 6*7 }, # 1: + $blessed_code, # 2: blessed code reference + \&Another::Package::foo, # 3: code in another package + sub ($$;$) { 0 }, # 4: prototypes + sub { print "test\n" }, # 5: + \&Test::ok, # 6: large scalar ], {"a" => sub { "srt" }, "b" => \&code}, @@ -203,7 +203,7 @@ { my $safe = new Safe; - $safe->permit(qw(:default require)); + $safe->permit(qw(:default require caller)); local $Storable::Eval = sub { $safe->reval(shift) }; for my $def ([0 => "JAPH", @@ -214,13 +214,17 @@ $freezed = freeze $obj[0]->[$i]; $@ = ""; eval { $thawed = thaw $freezed }; - skip(q{ok($@, ""}); - skip(q{$thawed->(), $res}); + ok($@, ""); + ok($thawed->(), $res); } + $safe = new Safe; + $safe->permit(qw(:default require)); + $Storable::Eval = sub { $safe->reval(shift) }; + $freezed = freeze $obj[0]->[6]; eval { $thawed = thaw $freezed }; - ok($@ =~ /trapped/); + ok($@, qr/trapped/); if (0) { # Disable or fix this test if the internal representation of Storable -- ```

Slaven Rezic - slaven.rezic@​berlin.de   babybike - routeplanner for cyclists in Berlin   handheld (e.g. Compaq iPAQ with Linux) version of bbbike   http​://bbbike.sourceforge.net

[p5pRT]

From @nwc10

On Sun\, Sep 08\, 2002 at 02​:50​:39PM +0100\, hv@​crypt.org wrote​:

Elizabeth Mattijsen (via RT) \perlbug@&#8203;perl\.org wrote​: [ C\< no strict 'redefine'; > doesn't complain. ]

:Included is a patch that should cause the above to warn with​: : : Don't know how to "no strict qw(redefine)"

As I remember it\, this behaviour was deliberate so that if we added additional optional strictures in the future you could take advantage of them without your code breaking under older perls.

no strict 'strict'; use strict qw(refs vars future whatever);

?

or a lexical warning that defaults to fatal and on? [strict.pm would only need to require warnings if someone used a category other than the current three]

I think that would need another bit in $^H - are they in short supply?

Nicholas Clark -- Even better than the real thing​: http​://nms-cgi.sourceforge.net/

[p5pRT]

From @lizmat

At 02​:50 PM 9/8/02 +0100\, hv@​crypt.org wrote​:

Elizabeth Mattijsen (via RT) \perlbug@&#8203;perl\.org wrote​: [ C\< no strict 'redefine'; > doesn't complain. ] ​:Included is a patch that should cause the above to warn with​: ​: Don't know how to "no strict qw(redefine)" As I remember it\, this behaviour was deliberate so that if we added additional optional strictures in the future you could take advantage of them without your code breaking under older perls.

Maybe the croak() would need to be changed to a warn() then? So that you would at least get some feedback if there is something that is not understood?

Liz

[p5pRT]

From @lizmat

At 10​:17 PM 9/8/02 +0200\, Slaven Rezic wrote​:

+ $safe->permit(qw(​:default require caller));

The caller() is just in there to find out whether a "use" or "no" was being done. I could rewrite this so that this is passed as a parameter to the subroutine​: this would add overhead to the common case though\, whereas the overhead now is just a single exists() on a hash.

Liz

[p5pRT]

From @hvds

Elizabeth Mattijsen \liz@&#8203;dijkmat\.nl wrote​: :At 10​:17 PM 9/8/02 +0200\, Slaven Rezic wrote​: :>+ $safe->permit(qw(​:default require caller)); : :The caller() is just in there to find out whether a "use" or "no" was being :done. I could rewrite this so that this is passed as a parameter to the :subroutine​: this would add overhead to the common case though\, whereas the :overhead now is just a single exists() on a hash.

An alternative would be to reword the complaint so that it doesn't need to know. C\< warn "Unknown stricture '$s'"; > may even be enough.

Hugo

[p5pRT]

From @lizmat

At 11​:28 AM 9/9/02 +0100\, hv@​crypt.org wrote​:

Elizabeth Mattijsen \liz@&#8203;dijkmat\.nl wrote​: ​:At 10​:17 PM 9/8/02 +0200\, Slaven Rezic wrote​: ​:>+ $safe->permit(qw(​:default require caller)); ​:The caller() is just in there to find out whether a "use" or "no" was being ​:done. I could rewrite this so that this is passed as a parameter to the ​:subroutine​: this would add overhead to the common case though\, whereas the ​:overhead now is just a single exists() on a hash. An alternative would be to reword the complaint so that it doesn't need to know. C\< warn "Unknown stricture '$s'"; > may even be enough.

Do you want me to submit a new patch for that?

Liz

[p5pRT]

From @rgs

Nicholas Clark wrote​:

File/Find/t/find.t tests 1 and 2 are doubly naughty

Thanks\, patch applied as #17884.

[p5pRT]

From @hvds

Slaven Rezic \slaven\.rezic@&#8203;berlin\.de wrote​: :With the strict.pm patch\, "use strict" will fail in a safe compartment :if "​:still_to_be_decided" or "caller" in not in the permitted opset.

I've now checked in the patch below as #17986; this removes the use of 'caller' in strict.pm\, and tightens the Safe compartment in Storable's t/code.t tests to match.

I'm not sure that the wording of the error message is ideal\, but I suspect it's good enough.

Hugo ==== //depot/perl/ext/Storable/t/code.t#4 - /src/hv/perl/ext/Storable/t/code.t ==== @​@​ -239\,7 +239\,7 @​@​ {   my $safe = new Safe;   # because of opcodes used in "use strict"​: - $safe->permit(qw(​:default require caller)); + $safe->permit(qw(​:default require));   local $Storable​::Eval = sub { $safe->reval(shift) };

  $freezed = freeze $obj[0]->[1]; ==== //depot/perl/lib/strict.pm#16 - /src/hv/perl/lib/strict.pm ==== @​@​ -16\,12 +16\,8 @​@​   $bits |= $bitmask{$s} || 0;   }   if (@​wrong) { - my $useno = { - __PACKAGE__.'​::import' => 'use'\, - __PACKAGE__.'​::unimport' => 'no' - }->{ (caller(1))[3] };   require Carp; - Carp​::croak("Don't know how to '$useno ".__PACKAGE__." qw(@​wrong)'"); + Carp​::croak("Unknown 'strict' tag(s) '@​wrong'");   }   $bits; } ==== //depot/perl/lib/strict.t#7 - /src/hv/perl/lib/strict.t ==== @​@​ -100\,17 +100\,17 @​@​ }

eval qq(use strict 'garbage'); -print +($@​ =~ /^Don't know how to 'use strict qw\(garbage\)/) +print +($@​ =~ /^Unknown 'strict' tag\(s\) 'garbage'/)   ? "ok ".++$i."\n" : "not ok ".++$i."\t# $@​";

eval qq(no strict 'garbage'); -print +($@​ =~ /^Don't know how to 'no strict qw\(garbage\)/) +print +($@​ =~ /^Unknown 'strict' tag\(s\) 'garbage'/)   ? "ok ".++$i."\n" : "not ok ".++$i."\t# $@​";

eval qq(use strict qw(foo bar)); -print +($@​ =~ /^Don't know how to 'use strict qw\(foo bar\)/) +print +($@​ =~ /^Unknown 'strict' tag\(s\) 'foo bar'/)   ? "ok ".++$i."\n" : "not ok ".++$i."\t# $@​";

eval qq(no strict qw(foo bar)); -print +($@​ =~ /^Don't know how to 'no strict qw\(foo bar\)/) +print +($@​ =~ /^Unknown 'strict' tag\(s\) 'foo bar'/)   ? "ok ".++$i."\n" : "not ok ".++$i."\t# $@​";

[p5pRT]

From @eserte

hv@​crypt.org writes​:

Slaven Rezic \slaven\.rezic@&#8203;berlin\.de wrote​: :With the strict.pm patch\, "use strict" will fail in a safe compartment :if "​:still_to_be_decided" or "caller" in not in the permitted opset.

I've now checked in the patch below as #17986; this removes the use of 'caller' in strict.pm\, and tightens the Safe compartment in Storable's t/code.t tests to match.

I'm not sure that the wording of the error message is ideal\, but I suspect it's good enough.

Thanks\, and you can add this (documentation) patch​:

Inline Patch

```diff --- ext/Storable/Storable.pm.orig Thu Oct 10 16:21:39 2002 +++ ext/Storable/Storable.pm Thu Oct 10 16:22:01 2002 @@ -810,7 +810,7 @@ compartment: use strict; my $safe = new Safe; # because of opcodes used in "use strict": - $safe->permit(qw(:default require caller)); + $safe->permit(qw(:default require)); local $Storable::Deparse = 1; local $Storable::Eval = sub { $safe->reval($_[0]) }; my $serialized = freeze(sub { 42 }); -- ```

Slaven Rezic - slaven.rezic@​berlin.de

  Berlin Perl Mongers - http​://berliner.pm.org

[p5pRT]

From @hvds

Slaven Rezic \slaven\.rezic@&#8203;berlin\.de wrote​: :hv@​crypt.org writes​: :> I've now checked in the patch below as #17986; this removes the use of :> 'caller' in strict.pm\, and tightens the Safe compartment in Storable's :> t/code.t tests to match. [...] :Thanks\, and you can add this (documentation) patch​:

Thanks\, applied as #18024.

Hugo

[p5pRT]

@jhi - Status changed from 'new' to 'resolved'