Inconsistent warnings with "our"

p5pRT commented 24 years ago

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

Searchable as RT2915$

p5pRT commented 24 years ago

From mjtg@cus.cam.ac.uk

Shouldn't

perl5.6.0 -we 'my $x; our $x; $x=0'

generate some sort of "redeclared" warning? The other cases (two "my"s or two "our"s or "our" before "my") all do.

And is it appropriate that

perl5.6.0 -we 'our $x'

generates a "used only once"\, but the corresponding "my" does not? Particularly since "our" is meant as a replacement for "use vars"\, and one of "use vars" purposes is suppression of this warning.

Mike Guy

% perl5.6.0 -V Summary of my perl5 (revision 5.0 version 6 subversion 0) configuration: Platform: osname=solaris\, osvers=2.6\, archname=sun4-solaris uname='' config_args='-dOes -f confug.sh' hint=previous\, useposix=true\, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=undef d_sfio=undef uselargefiles=define use64bitint=undef use64bitall=undef uselongdouble=undef usesocks=undef Compiler: cc='gcc'\, optimize='-O'\, gccversion=2.7.2.3 cppflags='-I/usr/local/include -I/opt/local/include -DREG_INFTY=22786' ccflags ='-I/usr/local/include -I/opt/local/include -DREG_INFTY=22786' stdchar='unsigned char'\, d_stdstdio=define\, usevfork=false intsize=4\, longsize=4\, ptrsize=4\, doublesize=8 d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=16 ivtype='long'\, ivsize=4\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=4 alignbytes=8\, usemymalloc=y\, prototype=define Linker and Libraries: ld='gcc'\, ldflags =' -L/usr/local/lib -L/opt/local/lib' libpth=/usr/local/lib /opt/local/lib /lib /usr/lib /usr/ccs/lib libs=-lsocket -lnsl -ldl -lm -lc -lcrypt libc=/lib/libc.so\, so=so\, useshrplib=false\, libperl=libperl.a Dynamic Linking: dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags=' ' cccdlflags='-fpic'\, lddlflags='-G -L/usr/local/lib -L/opt/local/lib'

Characteristics of this binary (from libperl): Compile-time options: USE_LARGE_FILES Built under solaris Compiled at Mar 24 2000 18:47:00 @INC: /home/mjtg/perl5.6.0/lib /home/mjtg/perl5.6.0/lib /home/mjtg/perl5.6.0/lib /home/mjtg/perl5.6.0/lib .

p5pRT commented 21 years ago

From stevep@marketview.co.nz

Created by stevep@marketview.co.nz

$ perl -w

sub a { my $x; my $x; }

sub b { my $x; our $x; }

sub c { our $x; my $x; }

sub d { our $x; our $x; }

Gives me the following warnings:

"my" variable $x masks earlier declaration in same scope at - line 4. "my" variable $x masks earlier declaration in same scope at - line 14. "our" variable $x masks earlier declaration in same scope at - line 19.

It would seem to me that if 'd' is considered worthy of generating a warning that 'b' should certainly be.

(This also happens with perl v5.8.0\, but I haven't put that into production yet\, so can't use that perlbugged report)

Perl Info

``` Flags: category=core severity=low Site configuration information for perl v5.6.1: Configured by stevep at Wed Sep 5 14:14:16 NZST 2001. Summary of my perl5 (revision 5.0 version 6 subversion 1) configuration: Platform: osname=linux, osvers=2.2.17-14, archname=i686-linux uname='linux spinneret 2.2.17-14 #1 mon feb 5 17:53:36 est 2001 i686 unknown ' config_args='' hint=recommended, useposix=true, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=undef d_sfio=undef uselargefiles=define usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef Compiler: cc='cc', ccflags ='-fno-strict-aliasing -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2', cppflags='-fno-strict-aliasing' ccversion='', gccversion='egcs-2.91.66 19990314/Linux (egcs-1.1.2 release)', 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, usemymalloc=n, prototype=define Linker and Libraries: ld='cc', ldflags =' -L/usr/local/lib' libpth=/usr/local/lib /lib /usr/lib libs=-lnsl -lndbm -lgdbm -ldb -ldl -lm -lc -lposix -lcrypt -lutil perllibs=-lnsl -ldl -lm -lc -lposix -lcrypt -lutil libc=/lib/libc-2.1.3.so, so=so, useshrplib=false, libperl=libperl.a 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.6.1: /usr/local/lib/perl5/5.6.1/i686-linux /usr/local/lib/perl5/5.6.1 /usr/local/lib/perl5/site_perl/5.6.1/i686-linux /usr/local/lib/perl5/site_perl/5.6.1 /usr/local/lib/perl5/site_perl . Environment for perl v5.6.1: HOME=/home/stevep LANG=en_US LANGUAGE (unset) LC_ALL=en_US LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/local/bin:/bin:/usr/bin:/usr/X11R6/bin:/home/stevep/bin PERL_BADLANG (unset) SHELL=/bin/bash2 ```

p5pRT commented 21 years ago

From @mjdominus

Steve Piner says:

sub d { our $x; our $x; }

"our" variable $x masks earlier declaration in same scope at - line 19.

It would seem to me that if 'd' is considered worthy of generating a warning that 'b' should certainly be.

I agree with your logic\, and I agree that 'b' should generate a warning.

But it seems to me that 'd' should *not* generate a warning. Since the alias created by the redundant declaration has the same name *and* the same referent\, it can't possibly cause any problems.

So perhaps whoever adds the warning for 'b' should get rid of the warning for 'd'.

p5pRT commented 21 years ago

From @rgarcia

Mark-Jason Dominus \mjd@plover\.com wrote:

Steve Piner says:

sub d { our $x; our $x; }

"our" variable $x masks earlier declaration in same scope at - line 19.

It would seem to me that if 'd' is considered worthy of generating a warning that 'b' should certainly be.

I agree with your logic\, and I agree that 'b' should generate a warning.

But it seems to me that 'd' should *not* generate a warning. Since the alias created by the redundant declaration has the same name *and* the same referent\, it can't possibly cause any problems.

And there's no 'masking' of the first $x by the second\, since they're the same variable. At least it should generate another warning\, I mean this one :

$ perl -wle 'our $x; { our $x }' "our" variable $x redeclared at -e line 1. (Did you mean "local" instead of "our"?)

perldiag says :

=item "our" variable %s redeclared

(W misc) You seem to have already declared the same global once before in the current lexical scope.

I think it's worth warning the user.

p5pRT commented 18 years ago

From @schwern

[mjtg@cus.cam.ac.uk - Thu Mar 30 21:52:59 2000]:

Shouldn't
perl5\.6\.0 \-we 'my $x; our $x; $x=0'
generate some sort of "redeclared" warning? The other cases (two "my"s or two "our"s or "our" before "my") all do.

This is still an issue in 5.9.x. I'd agree\, there should be a warning particularly because all other combinations issue a warning:

$ bleadperl -lwe 'my $x = 42; our $x = 23; print $x' 23 $ bleadperl -lwe 'my $x = 42; my $x = 23; print $x' "my" variable $x masks earlier declaration in same scope at -e line 1. 23 $ bleadperl -lwe 'our $x = 42; my $x = 23; print $x' "my" variable $x masks earlier declaration in same scope at -e line 1. 23 $ bleadperl -lwe 'our $x = 42; our $x = 23; print $x' "our" variable $x masks earlier declaration in same scope at -e line 1. 23

And is it appropriate that
    perl5\.6\.0 \-we 'our $x'
generates a "used only once"\, but the corresponding "my" does not?

This appears to have been resolved.

p5pRT commented 18 years ago

From rick@bort.ca

On Tue\, Jul 12\, 2005 at 08:29:58PM -0700\, Michael G Schwern via RT wrote:

This is still an issue in 5.9.x. I'd agree\, there should be a warning particularly because all other combinations issue a warning:

$ bleadperl -lwe 'my $x = 42; our $x = 23; print $x' 23 $ bleadperl -lwe 'my $x = 42; my $x = 23; print $x' "my" variable $x masks earlier declaration in same scope at -e line 1. 23 $ bleadperl -lwe 'our $x = 42; my $x = 23; print $x' "my" variable $x masks earlier declaration in same scope at -e line 1. 23 $ bleadperl -lwe 'our $x = 42; our $x = 23; print $x' "our" variable $x masks earlier declaration in same scope at -e line 1. 23

I agree too. The following patch will make the first case warn too. Note that it also changes this current behaviour:

% perl -wle 'our $p; package X; our $p;'
% perl -wle 'our $p; package X; my $p;' "my" variable $p masks earlier declaration in same scope at -e line 1.

so that the second case doesn't warn. I can't see any reason for the second case to warn if the first doesn't.

-- Rick Delaney rick@bort.ca

Inline Patch

```diff diff -ruN perl-current/pad.c perl-current-dev/pad.c --- perl-current/pad.c 2005-07-12 20:49:00.000000000 -0400 +++ perl-current-dev/pad.c 2005-07-13 01:44:01.477332772 -0400 @@ -515,8 +515,7 @@ && sv != &PL_sv_undef && !SvFAKE(sv) && (SvIVX(sv) == PAD_MAX || SvIVX(sv) == 0) - && (!is_our - || ((SvFLAGS(sv) & SVpad_OUR) && GvSTASH(sv) == ourstash)) + && (!(SvFLAGS(sv) & SVpad_OUR) || GvSTASH(sv) == ourstash) && strEQ(name, SvPVX_const(sv))) { Perl_warner(aTHX_ packWARN(WARN_MISC), diff -ruN perl-current/t/lib/warnings/pad perl-current-dev/t/lib/warnings/pad --- perl-current/t/lib/warnings/pad 2003-11-14 18:55:28.000000000 -0500 +++ perl-current-dev/t/lib/warnings/pad 2005-07-13 01:49:09.614260624 -0400 @@ -33,18 +33,27 @@ my $x ; my $x ; my $y = my $y ; +my $p; +package X; +my $p; +package main; no warnings 'misc' ; my $x ; my $y ; EXPECT "my" variable $x masks earlier declaration in same scope at - line 4. "my" variable $y masks earlier declaration in same statement at - line 5. +"my" variable $p masks earlier declaration in same scope at - line 8. ######## # pad.c use warnings 'misc' ; our $x ; our $x ; our $y = our $y ; +our $p; +package X; +our $p; +package main; no warnings 'misc' ; our $x ; our $y ; @@ -57,6 +66,10 @@ our $x ; my $x ; our $y = my $y ; +our $p; +package X; +my $p; +package main; no warnings 'misc' ; our $z ; my $z ; @@ -66,18 +79,22 @@ "my" variable $y masks earlier declaration in same statement at - line 5. ######## # pad.c -# TODO not implemented yet use warnings 'misc' ; my $x ; our $x ; my $y = our $y ; +my $p; +package X; +our $p; +package main; no warnings 'misc' ; my $z ; our $z ; my $t = our $t ; EXPECT -"our" variable $x masks earlier declaration in same scope at - line 5. -"our" variable $y masks earlier declaration in same statement at - line 6. +"our" variable $x masks earlier declaration in same scope at - line 4. +"our" variable $y masks earlier declaration in same statement at - line 5. +"our" variable $p masks earlier declaration in same scope at - line 8. ######## # pad.c use warnings 'closure' ; @@ -219,6 +236,13 @@ ######## use warnings 'misc' ; +my $x; +{ + my $x; +} +EXPECT +######## +use warnings 'misc' ; our $x; { our $x; @@ -227,6 +251,20 @@ "our" variable $x redeclared at - line 4. (Did you mean "local" instead of "our"?) ######## +use warnings 'misc' ; +our $x; +{ + my $x; +} +EXPECT +######## +use warnings 'misc' ; +my $x; +{ + our $x; +} +EXPECT +######## # an our var being introduced should suppress errors about global syms use strict; use warnings; ```

p5pRT commented 18 years ago

From @schwern

On Wed\, Jul 13\, 2005 at 02:15:49AM -0400\, Rick Delaney wrote:

I agree too. The following patch will make the first case warn too. Note that it also changes this current behaviour:
% perl \-wle 'our $p; package X; our $p;'      
% perl \-wle 'our $p; package X; my $p;' 
"my" variable $p masks earlier declaration in same scope at \-e line 1\.
so that the second case doesn't warn. I can't see any reason for the second case to warn if the first doesn't.

I can.

$ perl -wle 'our $p = 42; package X; print $p;' 42 $ perl -wle 'our $p = 42; package X; our $p = 23; print $p;' 23

That's clearly masking. "our" is lexical even crossing package boundries (something I never realized). Declaring the same lexical variable in the same lexical scope\, whether by my or our\, is masking.

-- Michael G Schwern schwern@pobox.com http://www.pobox.com/~schwern You are wicked and wrong to have broken inside and peeked at the implementation and then relied upon it. -- tchrist in \31832\.969261130@chthon

p5pRT commented 18 years ago

From rick@bort.ca

On Wed\, Jul 13\, 2005 at 02:01:40PM -0700\, Michael G Schwern wrote:

On Wed\, Jul 13\, 2005 at 02:15:49AM -0400\, Rick Delaney wrote:
I agree too. The following patch will make the first case warn too. Note that it also changes this current behaviour:
% perl \-wle 'our $p; package X; our $p;'      
% perl \-wle 'our $p; package X; my $p;' 
"my" variable $p masks earlier declaration in same scope at \-e line 1\.
so that the second case doesn't warn. I can't see any reason for the second case to warn if the first doesn't.
I can.

$ perl -wle 'our $p = 42; package X; print $p;' 42 $ perl -wle 'our $p = 42; package X; our $p = 23; print $p;' 23

That's clearly masking. "our" is lexical even crossing package boundries (something I never realized). Declaring the same lexical variable in the same lexical scope\, whether by my or our\, is masking.

There is no question that it's masking but the above case was specifically coded to not produce a warning before I ever looked at the code. All I did was make it consistent\, IMO. Presumably a warning is undesirable for code like

use strict; package Foo::Bar; our @ISA = qw(Foo);
package Foo::Baz; our @ISA = qw(Foo);

where it would just be a nuisance. And as you point out\, the cross-package scope can be a bit surprising. If anything I think the use of an our() variable outside its package is more deserving of a warning.

package Foo::Bar; our @ISA = qw(Bar);
package Foo::Baz; @ISA = qw(Baz); # we just trashed @Foo::Bar::ISA!

But then\, that would just annoy someone else who's using our() as described in perlfunc (which specifically describes when warnings are supposed to be produced).

-- Rick Delaney rick@bort.ca

p5pRT commented 18 years ago

From @sciurius

Rick Delaney \rick@bort\.ca writes:

use strict;
package Foo&#8203;::Bar;
our @&#8203;ISA = qw\(Foo\);

package Foo&#8203;::Baz;
our @&#8203;ISA = qw\(Foo\);

This is where we have "use base" for.

-- Johan

p5pRT commented 18 years ago

From rick@bort.ca

On Thu\, Jul 14\, 2005 at 02:05:26PM +0200\, Johan Vromans wrote:

This is where we have "use base" for.

Whatever\, it was just an example. Can we get this bug resolved\, please?

-- Rick Delaney rick@bort.ca

p5pRT commented 18 years ago

From @RandalSchwartz

"Rick" == Rick Delaney \rick@bort\.ca writes:

Rick> On Thu\, Jul 14\, 2005 at 02:05:26PM +0200\, Johan Vromans wrote:

This is where we have "use base" for.

Rick> Whatever\, it was just an example. Can we get this bug resolved\, Rick> please?

By "resolved"\, you mean "my $x; our $x;" needs masking warning\, not that the semantics of "our" are changed\, correct?

-- Randal L. Schwartz - Stonehenge Consulting Services\, Inc. - +1 503 777 0095 \merlyn@stonehenge\.com \<URL:http://www.stonehenge.com/merlyn/> Perl/Unix/security consulting\, Technical writing\, Comedy\, etc. etc. See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

p5pRT commented 18 years ago

From rick@bort.ca

On Thu\, Jul 14\, 2005 at 09:21:50AM -0700\, Randal L. Schwartz wrote:

"Rick" == Rick Delaney \rick@bort\.ca writes: Rick> Whatever\, it was just an example. Can we get this bug resolved\, Rick> please?

By "resolved"\, you mean "my $x; our $x;" needs masking warning\, not that the semantics of "our" are changed\, correct?

Absolutely. I think Schwern had the good sense to change the subject for the discussion of crazy ideas. ;-)

-- Rick Delaney rick@bort.ca

p5pRT commented 18 years ago

From @schwern

So\, to sum up... I think we all agree these should all warn.

my $x; our $x; # this currently does not our $x; my $x; my $x; my $x; our $x; our $x;

and that this should not

package Foo; our $x;

package Bar; our $x;

but this appears to be up in the air:

package Foo; my $x;

package Bar; our $x;

Currently this warns. Rick's patch to fix "my $x; our $x;" changes this so that it does not warn on the grounds that the "package Foo; our $x; package Bar; our $x" case doens't warn either. But I say that's a special case to do with the fact that our's mix of two scoping mechanisms\, lexical and package\, runs afoul of multi-package-in-one-file programming idioms.

package Foo; our @ISA = qw(...); # or $VERSION or @EXPORT or ...

package Bar; our @ISA = qw(...);

There's no corresponding idiom for "package Foo; my $x; package Bar; our $x" or vice-versa. Therefore\, it should warn about the mask.

p5pRT commented 18 years ago

From rick@bort.ca

On Thu\, Jul 14\, 2005 at 04:17:14PM -0700\, Michael G Schwern via RT wrote:

So\, to sum up... I think we all agree these should all warn.
my $x;  our $x;  \# this currently does not
our $x; my $x;
my $x;  my $x;
our $x; our $x;
and that this should not
package Foo;
our $x;

package Bar;
our $x;
but this appears to be up in the air:
package Foo;
my $x;

package Bar;
our $x;
Currently this warns. Rick's patch to fix "my $x; our $x;" changes this so that it does not warn on the grounds that the "package Foo; our $x; package Bar; our $x" case doens't warn either.

My patch doesn't change that case. It still warns. It does change

package Foo; our $x;

package Bar; my $x;

This currently warns. After the patch it doesn't. The reason is as described above. More specifically\, we don't see a need for a warning when we mask an our variable that was declared in a different package than the current package we are declaring in. I think whether we are declaring a my or our variable should make no difference. They are both masking the same thing in the exact same way.

But I say that's a special case to do with the fact that our's mix of two scoping mechanisms\, lexical and package\, runs afoul of multi-package-in-one-file programming idioms.
package Foo;
our @&#8203;ISA = qw$\.\.\.$;  \# or $VERSION or @&#8203;EXPORT or \.\.\.

package Bar;
our @&#8203;ISA = qw$\.\.\.$;
There's no corresponding idiom for "package Foo; my $x; package Bar; our $x" or vice-versa. Therefore\, it should warn about the mask.

I am almost convinced by this argument.

-- Rick Delaney rick@bort.ca

p5pRT commented 18 years ago

From @schwern

On Thu\, Jul 14\, 2005 at 08:14:26PM -0400\, Rick Delaney wrote:

But I say that's a special case to do with the fact that our's mix of two scoping mechanisms\, lexical and package\, runs afoul of multi-package-in-one-file programming idioms.
package Foo;
our @&#8203;ISA = qw$\.\.\.$;  \# or $VERSION or @&#8203;EXPORT or \.\.\.

package Bar;
our @&#8203;ISA = qw$\.\.\.$;
There's no corresponding idiom for "package Foo; my $x; package Bar; our $x" or vice-versa. Therefore\, it should warn about the mask.
I am almost convinced by this argument.

What are your reservations?

I'd add to the argument that:

package Foo; our $x = 42;

package Bar; my $x = 23;

has been warning since 5.6.x and I can't recall anyone complaining. Switching my and our around doesn't really change the scenario.

-- Michael G Schwern schwern@pobox.com http://www.pobox.com/~schwern ROCKS FALL! EVERYONE DIES! http://www.somethingpositive.net/sp05032002.shtml

p5pRT commented 18 years ago

From rick@bort.ca

On Thu\, Jul 14\, 2005 at 07:01:58PM -0700\, Michael G Schwern wrote:

On Thu\, Jul 14\, 2005 at 08:14:26PM -0400\, Rick Delaney wrote:

I am almost convinced by this argument.

What are your reservations?

I just don't like exceptions. And since the case we're talking about is pretty unlikely to come up in Real Life\, I'm not particulary motivated to change the patch. But I have no objection to some committer tweaking it before submitting if they feel it is important to retain the warning.

I'd add to the argument that:
package Foo;
our $x = 42;

package Bar;
my $x = 23;
has been warning since 5.6.x and I can't recall anyone complaining.

I'm sure very few people have ever seen that warning for that scenario.

As I write I'm feeling more like it's correct to warn in this case even if it is unlikely to happen. The feeling may start to bug me enough to change the patch but don't hold your breath.

-- Rick Delaney rick@bort.ca

p5pRT commented 18 years ago

From @rgarcia

On 7/15/05\, Michael G Schwern via RT \perlbug\-followup@perl\.org wrote:

So\, to sum up... I think we all agree these should all warn.
my $x;  our $x;  \# this currently does not

That's a bug.

our $x; my $x;
my $x;  my $x;
our $x; our $x;

I disagree. This shouldn't warn. There is no masking\, as demonstrated by : $ perl -e 'our $x=42; our $x; print $x' 42

and that this should not
package Foo;
our $x;

package Bar;
our $x;

Yes.

but this appears to be up in the air:
package Foo;
my $x;

package Bar;
our $x;
Currently this warns. Rick's patch to fix "my $x; our $x;" changes this so that it does not warn on the grounds that the "package Foo; our $x; package Bar; our $x" case doens't warn either. But I say that's a special case to do with the fact that our's mix of two scoping mechanisms\, lexical and package\, runs afoul of multi-package-in-one-file programming idioms.

I agree with this.

I fact I think I'd prefer not to change the cases where the masking warning is emitted purely based on the presence on package declarations. Declaring a default namespace is one thing\, and doesn't affect lexically-scoped names specified with my/our. The two declaration operations are orthogonal. Don't mix them and avoid special cases.

p5pRT commented 18 years ago

From rick@bort.ca

On Fri\, Jul 15\, 2005 at 05:01:46PM +0200\, Rafael Garcia-Suarez wrote:

On 7/15/05\, Michael G Schwern via RT \perlbug\-followup@perl\.org wrote:
our $x; our $x;
I disagree. This shouldn't warn. There is no masking\, as demonstrated by : $ perl -e 'our $x=42; our $x; print $x' 42

Hmm\, I'm inclined to agree. But let me just point out

http://public.activestate.com/cgi-bin/perlbrowse?patch=21725

:-P

and that this should not
package Foo;
our $x;

package Bar;
our $x;
Yes.

And yet here there is masking\, so not warning is an exception. Though I accept the reasons behind this exception.

-- Rick Delaney rick@bort.ca

p5pRT commented 18 years ago

From rick@bort.ca

Here's a new summary of masking warnings:
# Warns my $x; my $x; # yes my $x; our $x; # yes our $x; my $x; # yes our $x; our $x; # no

# Warns my $x; package X; my $x; # yes my $x; package X; our $x; # yes our $x; package X; my $x; # yes our $x; package X; our $x; # no

I'm happy with this. Is anyone not?

-- Rick Delaney rick@bort.ca

p5pRT commented 18 years ago

From @rgs

Rick Delaney wrote:

On Fri\, Jul 15\, 2005 at 05:01:46PM +0200\, Rafael Garcia-Suarez wrote:
On 7/15/05\, Michael G Schwern via RT \perlbug\-followup@perl\.org wrote:
our $x; our $x;
I disagree. This shouldn't warn. There is no masking\, as demonstrated by : $ perl -e 'our $x=42; our $x; print $x' 42
Hmm\, I'm inclined to agree. But let me just point out
http&#8203;://public\.activestate\.com/cgi\-bin/perlbrowse?patch=21725

Yes. I'm aware of this\, the TODO test is in my TODO list since a long time and I've thought about all this... so I changed my mind :p

and that this should not
package Foo;
our $x;

package Bar;
our $x;
Yes.
And yet here there is masking\, so not warning is an exception. Though I accept the reasons behind this exception.

p5pRT commented 18 years ago

From @schwern

On Fri\, Jul 15\, 2005 at 11:46:42AM -0400\, Rick Delaney wrote:

Here's a new summary of masking warnings:

                 \# Warns
 my $x;  my $x;  \# yes
 my $x; our $x;  \# yes
our $x;  my $x;  \# yes
our $x; our $x;  \# no

Its not a mask but its useless and probably an oversight on the part of the programmer. Maybe a "Useless use of our"? Or "Repeated use of our"?

I'm fine with dealing with that separately.

-- Michael G Schwern schwern@pobox.com http://www.pobox.com/~schwern Ahh email\, my old friend. Do you know that revenge is a dish that is best served cold? And it is very cold on the Internet!

p5pRT commented 18 years ago

From @rgarcia

On 7/15/05\, Michael G Schwern \schwern@pobox\.com wrote:

our $x; our $x;  \# no
Its not a mask but its useless and probably an oversight on the part of the programmer. Maybe a "Useless use of our"? Or "Repeated use of our"?

I'm fine with dealing with that separately.

There's already the warning ""our" variable %s redeclared" that can be re-used for this.

p5pRT commented 18 years ago

From @rgarcia

On 7/13/05\, Rick Delaney \rick@bort\.ca wrote:

I agree too. The following patch will make the first case warn too. Note that it also changes this current behaviour:
% perl \-wle 'our $p; package X; our $p;'
% perl \-wle 'our $p; package X; my $p;'
"my" variable $p masks earlier declaration in same scope at \-e line 1\.

I've applied a slightly modified version of your patch to bleadperl\, to match the semantics defined earlier :

Change 25179 on 2005/07/19 by rgs@bloom

Overhaul the semantics of the warning ""%s" variable %s masks earlier declaration"\, based on a patch by Rick Delaney. Now we have : my $x; my $x; # warns my $x; our $x; # warns our $x; my $x; # warns our $x; our $x; # silent

http://public.activestate.com/cgi-bin/perlbrowse?patch=25179

The presence of a package declaration between the two lexical variable delacarations doesn't change the warning case.

I think the "our variable redeclared" warning can be extended to the case

our $x; our $x;

but specifically not to the case

our $x; package X; our $x;

since in this latest case the 2nd $x is bound to $X::x and not $main::x. Agreed ?

p5pRT commented 18 years ago

From rick@bort.ca

On Tue\, Jul 19\, 2005 at 12:17:43PM +0200\, Rafael Garcia-Suarez wrote:

I think the "our variable redeclared" warning can be extended to the case
our $x; our $x;
but specifically not to the case
our $x; package X; our $x;
since in this latest case the 2nd $x is bound to $X::x and not $main::x. Agreed ?

Agreed.

Thanks\,

-- Rick Delaney rick@bort.ca

p5pRT commented 18 years ago

From rick@bort.ca

On Tue\, Jul 19\, 2005 at 12:00:00PM +0200\, Rafael Garcia-Suarez wrote:

On 7/15/05\, Michael G Schwern \schwern@pobox\.com wrote:
our $x; our $x;  \# no
Its not a mask but its useless and probably an oversight on the part of the programmer. Maybe a "Useless use of our"? Or "Repeated use of our"?

I'm fine with dealing with that separately.
There's already the warning ""our" variable %s redeclared" that can be re-used for this.

Right. Do we want the

"\t(Did you mean \"local\" instead of \"our\"?)\n"

part for this? It is unlikely that the programmer meant local in this case.

-- Rick Delaney rick@bort.ca

p5pRT commented 18 years ago

From @rgs

Rick Delaney wrote:

On Tue\, Jul 19\, 2005 at 12:00:00PM +0200\, Rafael Garcia-Suarez wrote:
On 7/15/05\, Michael G Schwern \schwern@pobox\.com wrote:
our $x; our $x;  \# no
Its not a mask but its useless and probably an oversight on the part of the programmer. Maybe a "Useless use of our"? Or "Repeated use of our"?

I'm fine with dealing with that separately.
There's already the warning ""our" variable %s redeclared" that can be re-used for this.
Right. Do we want the
"\\t$Did you mean \\"local\\" instead of \\"our\\"?$\\n"
part for this? It is unlikely that the programmer meant local in this case.

Good suggestion. I'll have a patch ready soon.

p5pRT commented 18 years ago

From @rgs

Rafael Garcia-Suarez wrote:

There's already the warning ""our" variable %s redeclared" that can be re-used for this.

Right. Do we want the
"\\t$Did you mean \\"local\\" instead of \\"our\\"?$\\n"
part for this? It is unlikely that the programmer meant local in this case.
Good suggestion.

Implemented by :

Change 25187 by rgs@bloom on 2005/07/19 14:12:38

Extend the the "our variable redeclared" warning to the case: our $x; our $x; and add more tests

Affected files ...

... //depot/perl/pad.c#68 edit ... //depot/perl/t/lib/strict/vars#7 edit ... //depot/perl/t/lib/warnings/pad#8 edit

Differences ...

==== //depot/perl/pad.c#68 (text) ====

@@ -515\,9 +515\,10 @@ && sv != &PL_sv_undef && !SvFAKE(sv) && (SvIVX(sv) == PAD_MAX || SvIVX(sv) == 0) - && !(is_our && (SvFLAGS(sv) & SVpad_OUR)) && strEQ(name\, SvPVX_const(sv))) { + if (is_our && (SvFLAGS(sv) & SVpad_OUR)) + break; /* "our" masking "our" */ Perl_warner(aTHX_ packWARN(WARN_MISC)\, "\"%s\" variable %s masks earlier declaration in same %s"\, (is_our ? "our" : "my")\, @@ -540\,8 +541\,9 @@ { Perl_warner(aTHX_ packWARN(WARN_MISC)\, "\"our\" variable %s redeclared"\, name); - Perl_warner(aTHX_ packWARN(WARN_MISC)\, - "\t(Did you mean \"local\" instead of \"our\"?)\n"); + if (off \<= PL_comppad_name_floor) + Perl_warner(aTHX_ packWARN(WARN_MISC)\, + "\t(Did you mean \"local\" instead of \"our\"?)\n"); break; } } while ( off-- > 0 );

==== //depot/perl/t/lib/strict/vars#7 (text) ==== [... rest trimmed\, cf APC for full text ...]

p5pRT commented 18 years ago

@rgs - Status changed from 'open' to 'resolved'

p5pRT commented 18 years ago

From @schwern

On Tue\, Jul 19\, 2005 at 12:17:43PM +0200\, Rafael Garcia-Suarez wrote:

I think the "our variable redeclared" warning can be extended to the case
our $x; our $x;
but specifically not to the case
our $x; package X; our $x;
since in this latest case the 2nd $x is bound to $X::x and not $main::x. Agreed ?

Yep.

-- Michael G Schwern schwern@pobox.com http://www.pobox.com/~schwern Don't try the paranormal until you know what's normal. -- "Lords and Ladies" by Terry Prachett

p5pRT commented 18 years ago

From @demerphq

On 7/14/05\, Johan Vromans \jvromans@squirrel\.nl wrote:

Rick Delaney \rick@bort\.ca writes:
use strict;
package Foo&#8203;::Bar;
our @&#8203;ISA = qw$Foo$;

package Foo&#8203;::Baz;
our @&#8203;ISA = qw$Foo$;
This is where we have "use base" for.

"use base" sucks. It uses horrible heuristics to do its thing and gets things wrong disturbingly often. IMO its much preferable to avoid it. As an example play around with it with multiple packages in a single file\, likewise play around with evaling packages into existance at run time\, and watch use base act like its been smoking crack.

-- perl -Mre=debug -e "/just|another|perl|hacker/"

p5pRT commented 18 years ago

From RandyS@ThePierianSpring.org

demerphq wrote:

On 7/14/05\, Johan Vromans \jvromans@squirrel\.nl wrote:

Rick Delaney \rick@bort\.ca writes:

use strict; package Foo::Bar; our @ISA = qw(Foo);

package Foo::Baz; our @ISA = qw(Foo);

This is where we have "use base" for.

"use base" sucks. It uses horrible heuristics to do its thing and gets things wrong disturbingly often. IMO its much preferable to avoid it. As an example play around with it with multiple packages in a single file\, likewise play around with evaling packages into existance at run time\, and watch use base act like its been smoking crack.

What would break if base.pm checked %:: to see if the package is defined /after/ it fails the C\<eval "require $base"> ? That would seem to solve a large class of problems.

Randy.

p5pRT commented 18 years ago

From @schwern

On Wed\, Jul 20\, 2005 at 11:33:01PM -0400\, Randy W. Sims wrote:

"use base" sucks. It uses horrible heuristics to do its thing and gets things wrong disturbingly often. IMO its much preferable to avoid it. As an example play around with it with multiple packages in a single file\, likewise play around with evaling packages into existance at run time\, and watch use base act like its been smoking crack.

What do you expect it to do in these cases? "use base" happens at compile time. In the case of the eval your class comes into existence at run time. In the multi package one I assume you're thinking of this:

package Foo; use base 'Bar';

package Bar; $bar = 42;

Where\, similar problem\, "use base 'Bar'" happens at compile time so Bar does not yet exist.

The only solution I can think of is to remove the checks that the base class exists. Or if there was some way for "base" to defer its checks until after its caller has finished compiling\, but I don't think that's possible.

What would break if base.pm checked %:: to see if the package is defined /after/ it fails the C\<eval "require $base"> ? That would seem to solve a large class of problems.

It does do that.

-- Michael G Schwern schwern@pobox.com http://www.pobox.com/~schwern You are wicked and wrong to have broken inside and peeked at the implementation and then relied upon it. -- tchrist in \31832\.969261130@chthon

Perl / perl5