produce redeclaration warning on C<our $foo; { our $foo; ... }>

p4raw-id: //depot/perl@4891

diff --git a/op.c b/op.c
index 76e6b4d..42e2994 100644 (file)
--- a/op.c
+++ b/op.c
@@ -153,22 +153,39 @@ Perl_pad_allocmy(pTHX_ char *name)
     }
     if (ckWARN(WARN_UNSAFE) && AvFILLp(PL_comppad_name) >= 0) {
        SV **svp = AvARRAY(PL_comppad_name);
-       for (off = AvFILLp(PL_comppad_name); off > PL_comppad_name_floor; off--) {
+       HV *ourstash = (PL_curstash ? PL_curstash : PL_defstash);
+       PADOFFSET top = AvFILLp(PL_comppad_name);
+       for (off = top; off > PL_comppad_name_floor; off--) {
            if ((sv = svp[off])
                && sv != &PL_sv_undef
                && (SvIVX(sv) == PAD_MAX || SvIVX(sv) == 0)
+               && (PL_in_my != KEY_our
+                   || ((SvFLAGS(sv) & SVpad_OUR) && GvSTASH(sv) == ourstash))
                && strEQ(name, SvPVX(sv)))
            {
-               if (PL_in_my != KEY_our
-                   || GvSTASH(sv) == (PL_curstash ? PL_curstash : PL_defstash))
+               Perl_warner(aTHX_ WARN_UNSAFE,
+                   "\"%s\" variable %s masks earlier declaration in same %s", 
+                   (PL_in_my == KEY_our ? "our" : "my"),
+                   name,
+                   (SvIVX(sv) == PAD_MAX ? "scope" : "statement"));
+               --off;
+               break;
+           }
+       }
+       if (PL_in_my == KEY_our) {
+           while (off >= 0 && off <= top) {
+               if ((sv = svp[off])
+                   && sv != &PL_sv_undef
+                   && ((SvFLAGS(sv) & SVpad_OUR) && GvSTASH(sv) == ourstash)
+                   && strEQ(name, SvPVX(sv)))
                {
                    Perl_warner(aTHX_ WARN_UNSAFE,
-                       "\"%s\" variable %s masks earlier declaration in same %s", 
-                       (PL_in_my == KEY_our ? "our" : "my"),
-                       name,
-                       (SvIVX(sv) == PAD_MAX ? "scope" : "statement"));
+                       "\"our\" variable %s redeclared", name);
+                   Perl_warner(aTHX_ WARN_UNSAFE,
+                       "(Did you mean \"local\" instead of \"our\"?)\n");
+                   break;
                }
-               break;
+               --off;
            }
        }
     }

diff --git a/pod/perldelta.pod b/pod/perldelta.pod
index b90db4c..11a35f4 100644 (file)
--- a/pod/perldelta.pod
+++ b/pod/perldelta.pod
@@ -1549,11 +1549,6 @@ A tutorial on managing class data for object modules.
 
 =over 4
 
-=item "my sub" not yet implemented
-
-(F) Lexically scoped subroutines are not yet implemented.  Don't try that
-yet.
-
 =item "%s" variable %s masks earlier declaration in same %s
 
 (W) A "my" or "our" variable has been redeclared in the current scope or statement,
@@ -1562,6 +1557,16 @@ always a typographical error.  Note that the earlier variable will still exist
 until the end of the scope or until all closure referents to it are
 destroyed.
 
+=item "my sub" not yet implemented
+
+(F) Lexically scoped subroutines are not yet implemented.  Don't try that
+yet.
+
+=item "our" variable %s redeclared
+
+(W) You seem to have already declared the same global once before in the
+current lexical scope.
+
 =item '!' allowed only after types %s
 
 (F) The '!' is allowed in pack() and unpack() only after certain types.
@@ -1802,6 +1807,11 @@ just use C<if (%hash) { # not empty }> for example.
 
 See Server error.
 
+=item Did you mean "local" instead of "our"?
+
+(W) Remember that "our" does not localize the declared global variable.
+You have declared it again in the same lexical scope, which seems superfluous.
+
 =item Document contains no data
 
 See Server error.

diff --git a/pod/perldiag.pod b/pod/perldiag.pod
index 2c0f04a..9c5b33e 100644 (file)
--- a/pod/perldiag.pod
+++ b/pod/perldiag.pod
@@ -31,6 +31,14 @@ C<"%(-?@> sort before the letters, while C<[> and C<\> sort after.
 
 =over 4
 
+=item "%s" variable %s masks earlier declaration in same %s
+
+(W) A "my" or "our" variable has been redeclared in the current scope or statement,
+effectively eliminating all access to the previous instance.  This is almost
+always a typographical error.  Note that the earlier variable will still exist
+until the end of the scope or until all closure referents to it are
+destroyed.
+
 =item "my sub" not yet implemented
 
 (F) Lexically scoped subroutines are not yet implemented.  Don't try that
@@ -42,19 +50,16 @@ yet.
 to try to declare one with a package qualifier on the front.  Use local()
 if you want to localize a package variable.
 
-=item "%s" variable %s masks earlier declaration in same %s
-
-(W) A "my" or "our" variable has been redeclared in the current scope or statement,
-effectively eliminating all access to the previous instance.  This is almost
-always a typographical error.  Note that the earlier variable will still exist
-until the end of the scope or until all closure referents to it are
-destroyed.
-
 =item "no" not allowed in expression
 
 (F) The "no" keyword is recognized and executed at compile time, and returns
 no useful value.  See L<perlmod>.
 
+=item "our" variable %s redeclared
+
+(W) You seem to have already declared the same global once before in the
+current lexical scope.
+
 =item "use" not allowed in expression
 
 (F) The "use" keyword is recognized and executed at compile time, and returns
@@ -1284,6 +1289,11 @@ See Server error.
 
 (W) You probably referred to an imported subroutine &FOO as $FOO or some such.
 
+=item Did you mean "local" instead of "our"?
+
+(W) Remember that "our" does not localize the declared global variable.
+You have declared it again in the same lexical scope, which seems superfluous.
+
 =item Did you mean $ or @ instead of %?
 
 (W) You probably said %hash{$key} when you meant $hash{$key} or @hash{@keys}.

diff --git a/t/pragma/strict-vars b/t/pragma/strict-vars
index dc11f5d..9352c4b 100644 (file)
--- a/t/pragma/strict-vars
+++ b/t/pragma/strict-vars
@@ -339,3 +339,18 @@ ${foo} = 10;
 our $foo;
 EXPECT
 "our" variable $foo masks earlier declaration in same scope at - line 7.
+########
+
+# multiple our declarations in same scope, same package, warning
+use strict 'vars';
+use warnings;
+our $foo;
+{
+    our $foo;
+    package Foo;
+    our $foo;
+}
+EXPECT
+"our" variable $foo redeclared at - line 7.
+(Did you mean "local" instead of "our"?)
+Name "Foo::foo" used only once: possible typo at - line 9.