PATCH (5.005_57): defined(@a) now deprecated

To: perl5-porters@perl.com
Message-ID: <19990527200544.13330.qmail@plover.com>

p4raw-id: //depot/cfgperl@3498

9 files changed, 103 insertions, 19 deletions

diff --git a/lib/Carp.pm b/lib/Carp.pm
index f8f750a5d7..5fb8809900 100644
--- a/lib/Carp.pm
+++ b/lib/Carp.pm
@@ -215,7 +215,7 @@ sub shortmess {	# Short-circuit &longmess if called via multiple packages
 
     # merge all the caller's @ISA packages into %isa.
     @isa{@{"${prevpack}::ISA"}} = ()
-	if(defined @{"${prevpack}::ISA"});
+	if(@{"${prevpack}::ISA"});
 
     # now we crawl up the calling stack and look at all the packages in
     # there.  For each package, we look to see if it has an @ISA and then
@@ -223,7 +223,7 @@ sub shortmess {	# Short-circuit &longmess if called via multiple packages
     # our caller is a derived class of that package and its calls can also
     # be ignored
     while (($pack,$file,$line) = caller($i++)) {
-	if(defined @{$pack . "::ISA"}) {
+	if(@{$pack . "::ISA"}) {
 	    my @i = @{$pack . "::ISA"};
 	    my %i;
 	    @i{@i} = ();
@@ -247,7 +247,7 @@ sub shortmess {	# Short-circuit &longmess if called via multiple packages
 	if ($extra-- > 0) {
 	    %isa = ($pack,1);
 	    @isa{@{$pack . "::ISA"}} = ()
-		if(defined @{$pack . "::ISA"});
+		if(@{$pack . "::ISA"});
 	}
 	else {
 	    # OK!  We've got a candidate package.  Time to construct the
diff --git a/op.c b/op.c
index fa286f841a..de3d9f9338 100644
--- a/op.c
+++ b/op.c
@@ -5009,6 +5009,30 @@ ck_lfun(OP *o)
 }
 
 OP *
+ck_defined(OP *o)		/* 19990527 MJD */
+{
+    if (ckWARN(WARN_DEPRECATED)) {
+      switch (cUNOPo->op_first->op_type) {
+      case OP_RV2AV:
+      case OP_PADAV:
+      case OP_AASSIGN:		/* Is this a good idea? */
+	warner(WARN_DEPRECATED, "defined(@array) is deprecated (and not really meaningful)");
+	warner(WARN_DEPRECATED, "(Maybe you should just omit the defined()?)\n");
+	break;
+      case OP_RV2HV:
+      case OP_PADHV:
+	warner(WARN_DEPRECATED, "defined(%hash) is deprecated (and not really meaningful)");
+	warner(WARN_DEPRECATED, "(Maybe you should just omit the defined()?)\n");
+	break;
+      default:
+	/* no warning */
+	break;
+      }
+    }
+    return ck_rfun(o);
+}
+
+OP *
 ck_rfun(OP *o)
 {
     OPCODE type = o->op_type;
diff --git a/opcode.h b/opcode.h
index a4efb5acba..b6143eac72 100644
--- a/opcode.h
+++ b/opcode.h
@@ -1482,7 +1482,7 @@ EXT OP * (CPERLscope(*PL_check)[]) _((OP *op)) = {
 	ck_null,	/* schop */
 	ck_spair,	/* chomp */
 	ck_null,	/* schomp */
-	ck_rfun,	/* defined */
+	ck_defined,	/* defined */
 	ck_lfun,	/* undef */
 	ck_fun,		/* study */
 	ck_lfun,	/* pos */
diff --git a/opcode.pl b/opcode.pl
index 7b6e79655b..c763675864 100755
--- a/opcode.pl
+++ b/opcode.pl
@@ -307,7 +307,7 @@ chop		chop			ck_spair	mts%	L
 schop		scalar chop		ck_null		stu%	S?
 chomp		safe chop		ck_spair	mts%	L
 schomp		scalar safe chop	ck_null		stu%	S?
-defined		defined operator	ck_rfun		isu%	S?
+defined		defined operator	ck_defined	isu%	S?
 undef		undef operator		ck_lfun		s%	S?
 study		study			ck_fun		su%	S?
 pos		match position		ck_lfun		stu%	S?
diff --git a/pod/perldelta.pod b/pod/perldelta.pod
index 7d8c0cc607..06346a645b 100644
--- a/pod/perldelta.pod
+++ b/pod/perldelta.pod
@@ -473,6 +473,18 @@ by Perl.
 (W) You used the C<open(FH, "| command")> or C<open(FH, "command |")>
 construction, but the command was missing or blank.
 
+=item defined(@array) is deprecated (and not really meaningful)
+
+(D) defined() is not usually useful on arrays because it checks for an
+undefined I<scalar> value.  If you want to see if the array is empty,
+just use C<if (@array) { # not empty }> for example.  
+
+=item defined(%hash) is deprecated (and not really meaningful)
+
+(D) defined() is not usually useful on hashes because it checks for an
+undefined I<scalar> value.  If you want to see if the hash is empty,
+just use C<if (%hash) { # not empty }> for example.  
+
 =head1 Obsolete Diagnostics
 
 Todo.
diff --git a/pod/perldiag.pod b/pod/perldiag.pod
index 688e847085..0084f9c6da 100644
--- a/pod/perldiag.pod
+++ b/pod/perldiag.pod
@@ -1105,6 +1105,18 @@ times more than it has returned.  This probably indicates an infinite
 recursion, unless you're writing strange benchmark programs, in which
 case it indicates something else.
 
+=item defined(@array) is deprecated (and not really meaningful)
+
+(D) defined() is not usually useful on arrays because it checks for an
+undefined I<scalar> value.  If you want to see if the array is empty,
+just use C<if (@array) { # not empty }> for example.  
+
+=item defined(%hash) is deprecated (and not really meaningful)
+
+(D) defined() is not usually useful on hashes because it checks for an
+undefined I<scalar> value.  If you want to see if the hash is empty,
+just use C<if (%hash) { # not empty }> for example.  
+
 =item Delimiter for here document is too long
 
 (F) In a here document construct like C<E<lt>E<lt>FOO>, the label
diff --git a/pod/perlfunc.pod b/pod/perlfunc.pod
index ed3de62a23..4043301a5e 100644
--- a/pod/perlfunc.pod
+++ b/pod/perlfunc.pod
@@ -912,24 +912,14 @@ should use C<defined> only when you're questioning the integrity of what
 you're trying to do.  At other times, a simple comparison to C<0> or C<""> is
 what you want.
 
-Currently, using C<defined> on an entire array or hash reports whether
-memory for that aggregate has ever been allocated.  So an array you set
-to the empty list appears undefined initially, and one that once was full
-and that you then set to the empty list still appears defined.  You
-should instead use a simple test for size:
+Use of C<defined> on aggregates (hashes and arrays) is deprecated.  It
+used to report whether memory for that aggregate has ever been
+allocated.  This behavior may disappear in future versions of Perl.
+You should instead use a simple test for size:
 
     if (@an_array) { print "has array elements\n" }
     if (%a_hash)   { print "has hash members\n"   }
 
-Using C<undef> on these, however, does clear their memory and then report
-them as not defined anymore, but you shouldn't do that unless you don't
-plan to use them again, because it saves time when you load them up
-again to have memory already ready to be filled.  The normal way to 
-free up space used by an aggregate is to assign the empty list.
-
-This counterintuitive behavior of C<defined> on aggregates may be
-changed, fixed, or broken in a future release of Perl.
-
 See also L</undef>, L</exists>, L</ref>.
 
 =item delete EXPR
diff --git a/pp_proto.h b/pp_proto.h
index 76ac6f499c..7f5728200d 100644
--- a/pp_proto.h
+++ b/pp_proto.h
@@ -2,6 +2,7 @@ PERL_CKDEF(ck_anoncode)
 PERL_CKDEF(ck_bitop)
 PERL_CKDEF(ck_concat)
 PERL_CKDEF(ck_delete)
+PERL_CKDEF(ck_defined)
 PERL_CKDEF(ck_eof)
 PERL_CKDEF(ck_eval)
 PERL_CKDEF(ck_exec)
diff --git a/t/pragma/warn/op b/t/pragma/warn/op
index b0202f7fdf..c72534a15f 100644
--- a/t/pragma/warn/op
+++ b/t/pragma/warn/op
@@ -87,6 +87,16 @@
      (Maybe you meant system() when you said exec()?
  	exec "true" ; my $a
 
+     defined(@array) is deprecated (and not really meaningful)
+     (Maybe you should just omit the defined()?)
+	defined @a ;
+	my @a ; defined @a ;
+	defined (@a = (1,2,3)) ;
+
+     defined(%hash) is deprecated (and not really meaningful)
+     (Maybe you should just omit the defined()?)
+	defined %h ;
+	my %h ; defined %h ;
 
 __END__
 # op.c
@@ -543,3 +553,38 @@ my $a
 EXPECT
 Statement unlikely to be reached at - line 4.
 (Maybe you meant system() when you said exec()?)
+########
+# op.c
+use warning 'deprecated' ;
+defined(@a);
+EXPECT
+defined(@array) is deprecated (and not really meaningful) at - line 3.
+(Maybe you should just omit the defined()?)
+########
+# op.c
+use warning 'deprecated' ;
+my @a; defined(@a);
+EXPECT
+defined(@array) is deprecated (and not really meaningful) at - line 3.
+(Maybe you should just omit the defined()?)
+########
+# op.c
+use warning 'deprecated' ;
+defined(@a = (1,2,3));
+EXPECT
+defined(@array) is deprecated (and not really meaningful) at - line 3.
+(Maybe you should just omit the defined()?)
+########
+# op.c
+use warning 'deprecated' ;
+defined(%h);
+EXPECT
+defined(%hash) is deprecated (and not really meaningful) at - line 3.
+(Maybe you should just omit the defined()?)
+########
+# op.c
+use warning 'deprecated' ;
+my %h; defined(%h);
+EXPECT
+defined(%hash) is deprecated (and not really meaningful) at - line 3.
+(Maybe you should just omit the defined()?)