Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add warning for exists($a[0]) and delete($a[0]) #11785

Closed
p5pRT opened this issue Dec 4, 2011 · 20 comments
Closed

Add warning for exists($a[0]) and delete($a[0]) #11785

p5pRT opened this issue Dec 4, 2011 · 20 comments

Comments

@p5pRT
Copy link

@p5pRT p5pRT commented Dec 4, 2011

Migrated from rt.perl.org#105278 (status was 'rejected')

Searchable as RT105278$

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 4, 2011

From @ikegami

Created by @ikegami

Hi,

exists($a[$i]) and delete($a[0]) are deprecated, yet they don't emit a
deprecation warning. There has been talk about removing them from core or
changing their functionality, which required the deprecated warning to be
emitted. The attached patches add a deprecation warning for those
operations.

I have previously gotten most use of exists($a[$i]) and delete($a[0])
removed from the cpan/ directory, but two remain​:

- Object​::Accessor has one remaining use, and there's an open ticket to
have it fixed.
- Tie​::File needs to have C<< no warnings 'deprecated'; >> added around its
tests for its EXISTS and DELETE handler. I'll be happy to submit a patch
once this is committed.

Thanks,
Eric

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.14.0:

Configured by gecko at Thu May 12 09:33:39 2011.

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

  Platform:
    osname=MSWin32, osvers=5.2, archname=MSWin32-x86-multi-thread
    uname=''
    config_args='undef'
    hint=recommended, useposix=true, d_sigaction=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='C:/Progs/PERL51~4/site/bin/gcc.exe', ccflags ='-DNDEBUG -DWIN32
-D_CONSOLE -DNO_STRICT -DPERL_TEXTMODE_SCRIPTS -DUSE_SITECUSTOMIZE
-DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO
-D_USE_32BIT_TIME_T -DHASATTRIBUTE -fno-strict-aliasing -mms-bitfields',
    optimize='-O2',
    cppflags='-DWIN32'
    ccversion='', gccversion='3.4.5 (mingw-vista special r3)',
gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=8
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='__int64',
lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='C:\Progs\PERL51~4\site\bin\g++.exe', ldflags
='-L"C:\Progs\perl5140-ap1400\lib\CORE"'
    libpth=\lib
    libs=-lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32
-lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm
-lversion -lodbc32 -lodbccp32 -lcomctl32 -lmsvcrt
    perllibs=-lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32
-lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm
-lversion -lodbc32 -lodbccp32 -lcomctl32 -lmsvcrt
    libc=msvcrt.lib, so=dll, useshrplib=true, libperl=perl514.lib
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
    cccdlflags=' ', lddlflags='-mdll -L"C:\Progs\perl5140-ap1400\lib\CORE"'

Locally applied patches:
    ACTIVEPERL_LOCAL_PATCHES_ENTRY


@INC for perl 5.14.0:
    C:/Progs/perl5140-ap1400/site/lib
    C:/Progs/perl5140-ap1400/lib
    .


Environment for perl 5.14.0:
    HOME (unset)
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=C:\Program Files (x86)\NVIDIA
Corporation\PhysX\Common;c:\bin;C:\Progs\perl5140-ap1400\site\bin;C:\Progs\perl5140-ap1400\bin;C:\Program
Files
(x86)\UltraEdit;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Program
Files\Common Files\Microsoft Shared\Windows Live;C:\Program Files
(x86)\Common Files\Microsoft Shared\Windows Live;C:\Program
Files\WIDCOMM\Bluetooth Software\;C:\Program Files\WIDCOMM\Bluetooth
Software\syswow64;C:\Program Files (x86)\Common Files\Ulead
Systems\MPEG;C:\Program Files\Microsoft Windows Performance
Toolkit\;C:\Program Files (x86)\Windows Live\Shared;C:\Program Files
(x86)\QuickTime\QTSystem\
    PERL_BADLANG (unset)
    SHELL (unset)

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 4, 2011

From @ikegami

0001-Avoid-needless-use-of-deprecated-exists-on-array-ele.patch
From 0aa896e13f78dd949f52e780aa83fd505755b4a1 Mon Sep 17 00:00:00 2001
From: Eric Brine <ikegami@adaelis.com>
Date: Wed, 11 May 2011 22:21:50 -0700
Subject: [PATCH 1/3] Avoid needless use of deprecated exists on array elements

---
 ext/XS-APItest/t/xsub_h.t |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ext/XS-APItest/t/xsub_h.t b/ext/XS-APItest/t/xsub_h.t
index 93742b1..9bf0710 100644
--- a/ext/XS-APItest/t/xsub_h.t
+++ b/ext/XS-APItest/t/xsub_h.t
@@ -18,7 +18,7 @@ sub default {
 
 sub expect_good {
     my $package = $_[0];
-    my $version = exists $_[1] ? ", $_[1]" : '';
+    my $version = @_ >= 2 ? ", $_[1]" : '';
     local $Test::Builder::Level = $Test::Builder::Level + 1;
     is_deeply([XS_VERSION_defined(@_)], [],
 	      "Is good for $package$version");
@@ -37,7 +37,7 @@ sub expect_bad {
     } else {
 	$what = 'bootstrap parameter';
     }
-    if (exists $_[1]) {
+    if (@_ >= 2) {
 	$desc = "$_[0], $_[1]";
     } else {
 	$desc = $_[0];
-- 
1.7.2.5

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 4, 2011

From @ikegami

0002-Add-TODO-tests-to-add-deprecation-warnings-for-exist.patch
From dfaa7e0893d2fcc3e21fbcd78235e82ab24ad0b8 Mon Sep 17 00:00:00 2001
From: Eric Brine <ikegami@adaelis.com>
Date: Wed, 11 May 2011 19:41:29 -0700
Subject: [PATCH 2/3] Add TODO tests to add deprecation warnings for exists and delete on array elements.

---
 t/op/delete.t |   35 +++++++++++++++++++++++++++++++----
 1 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/t/op/delete.t b/t/op/delete.t
index 493717e..52ee914 100644
--- a/t/op/delete.t
+++ b/t/op/delete.t
@@ -6,7 +6,7 @@ BEGIN {
 }
 
 require "test.pl";
-plan( tests => 38 );
+plan( tests => 44 );
 
 # delete() on hash elements
 
@@ -74,16 +74,43 @@ $foo[3] = 'c';
 $foo[4] = 'd';
 $foo[5] = 'e';
 
-$foo = delete $foo[2];
+{
+    my @warns;
+    local $SIG{__WARN__} = sub { push @warns, "@_" };
+
+    $foo = delete $foo[2];
+
+    local $TODO = 'Add deprecation warning for delete on array elements';
+    is(0+@warns, 1);
+    like($warns[0], qr/^Use of delete on array elements is deprecated\b/);
+}
 
 cmp_ok($foo,'eq','b','ary delete 2');
-ok(!(exists $foo[2]),'ary b absent');
+{
+    my @warns;
+    local $SIG{__WARN__} = sub { push @warns, "@_" };
+
+    ok(!(exists $foo[2]),'ary b absent');
+
+    local $TODO = 'Add deprecation warning for exists on array elements';
+    is(0+@warns, 1);
+    like($warns[0], qr/^Use of exists on array elements is deprecated\b/);
+}
 cmp_ok($foo[1],'eq','a','ary a exists');
 cmp_ok($foo[3],'eq','c','ary c exists');
 cmp_ok($foo[4],'eq','d','ary d exists');
 cmp_ok($foo[5],'eq','e','ary e exists');
 
-@bar = delete @foo[4,5];
+{
+    my @warns;
+    local $SIG{__WARN__} = sub { push @warns, "@_" };
+
+    @bar = delete @foo[4,5];
+
+    local $TODO = 'Add deprecation warning for delete on array elements';
+    is(0+@warns, 1);
+    like($warns[0], qr/^Use of delete on array elements is deprecated\b/);
+}
 
 cmp_ok(scalar(@bar),'==',2,'ary deleted slice');
 cmp_ok($bar[0],'eq','d','ary slice 1');
-- 
1.7.2.5

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 4, 2011

From @ikegami

0003-Add-deprecation-message-for-exists-and-delete-on-arr.patch
From e92e87f8ee1a623851fa6a347f9d8758e471baf4 Mon Sep 17 00:00:00 2001
From: Eric Brine <ikegami@adaelis.com>
Date: Wed, 11 May 2011 23:15:40 -0700
Subject: [PATCH 3/3] Add deprecation message for exists and delete on array elements

---
 dist/Dumpvalue/lib/Dumpvalue.pm |    5 +-
 dist/Storable/t/freeze.t        |    4 +-
 lib/Internals.t                 |    4 +-
 lib/Tie/Array.pm                |    4 +-
 lib/dumpvar.pl                  |    1 +
 pod/perldiag.pod                |   12 +++++
 pp.c                            |    9 ++++-
 t/op/args.t                     |    1 +
 t/op/delete.t                   |   18 +++++---
 t/op/local.t                    |   87 +++++++++++++++++++++++++++-----------
 t/op/reverse.t                  |   66 +++++++++++++++++++-----------
 t/op/sub.t                      |    4 +-
 t/op/tiearray.t                 |    3 +
 t/re/pat.t                      |   29 +++++++------
 14 files changed, 168 insertions(+), 79 deletions(-)

diff --git a/dist/Dumpvalue/lib/Dumpvalue.pm b/dist/Dumpvalue/lib/Dumpvalue.pm
index f3cde0b..c45a44d 100644
--- a/dist/Dumpvalue/lib/Dumpvalue.pm
+++ b/dist/Dumpvalue/lib/Dumpvalue.pm
@@ -1,7 +1,7 @@
 use 5.006_001;			# for (defined ref) and $#$v and our
 package Dumpvalue;
 use strict;
-our $VERSION = '1.16';
+our $VERSION = '1.17';
 our(%address, $stab, @stab, %stab, %subs);
 
 # documentation nits, handle complex data structures better by chromatic
@@ -230,7 +230,7 @@ sub unwrap {
       if ($#$v >= 0) {
 	$short = $sp . "0..$#{$v}  " .
 	  join(" ", 
-	       map {exists $v->[$_] ? $self->stringify($v->[$_]) : "empty"} (0..$tArrayDepth)
+	       map {; no warnings 'deprecated'; exists $v->[$_] ? $self->stringify($v->[$_]) : "empty"} (0..$tArrayDepth)
 	      ) . "$shortmore";
       } else {
 	$short = $sp . "empty array";
@@ -240,6 +240,7 @@ sub unwrap {
     for my $num (0 .. $tArrayDepth) {
       return if $DB::signal and $self->{stopDbSignal};
       print "$sp$num  ";
+      no warnings 'deprecated';  # exists $a[]
       if (exists $v->[$num]) {
         $self->DumpElem($v->[$num], $s);
       } else {
diff --git a/dist/Storable/t/freeze.t b/dist/Storable/t/freeze.t
index a02f836..649e7e6 100644
--- a/dist/Storable/t/freeze.t
+++ b/dist/Storable/t/freeze.t
@@ -128,8 +128,8 @@ SKIP: {
     eval '
         $a = []; $#$a = 2; $a->[1] = undef;
         $b = thaw freeze $a;
-        @a = map { ~~ exists $a->[$_] } 0 .. $#$a;
-        @b = map { ~~ exists $b->[$_] } 0 .. $#$b;
+        @a = map {; no warnings "deprecated"; ~~ exists $a->[$_] } 0 .. $#$a;
+        @b = map {; no warnings "deprecated"; ~~ exists $b->[$_] } 0 .. $#$b;
     ';
     is($@, '');
     is("@a", "@b");
diff --git a/lib/Internals.t b/lib/Internals.t
index d3fce9c..8c378ea 100644
--- a/lib/Internals.t
+++ b/lib/Internals.t
@@ -43,7 +43,7 @@ ok(  Internals::SvREADONLY @foo, 1 );
 ok(  Internals::SvREADONLY @foo );
 eval { undef(@foo); };
 like($@, $ro_err, q/Can't undef read-only array/);
-eval { delete($foo[2]); };
+eval { no warnings 'deprecated'; delete($foo[2]); };
 like($@, $ro_err, q/Can't delete from read-only array/);
 eval { shift(@foo); };
 like($@, $ro_err, q/Can't shift read-only array/);
@@ -90,7 +90,7 @@ eval { undef($foo[2]); };
 like($@, $ro_err, q/Can't undef read-only array element/);
 TODO: {
     local $TODO = 'Due to restricted hashes implementation';
-    eval { delete($foo[2]); };
+    eval { no warnings 'deprecated'; delete($foo[2]); };
     like($@, $ro_err, q/Can't delete read-only array element/);
 }
 
diff --git a/lib/Tie/Array.pm b/lib/Tie/Array.pm
index 767cfdd..5da255a 100644
--- a/lib/Tie/Array.pm
+++ b/lib/Tie/Array.pm
@@ -96,8 +96,8 @@ sub POP       { pop(@{$_[0]}) }
 sub PUSH      { my $o = shift; push(@$o,@_) }
 sub SHIFT     { shift(@{$_[0]}) }
 sub UNSHIFT   { my $o = shift; unshift(@$o,@_) }
-sub EXISTS    { exists $_[0]->[$_[1]] }
-sub DELETE    { delete $_[0]->[$_[1]] }
+sub EXISTS    { no warnings 'deprecated'; exists $_[0]->[$_[1]] }
+sub DELETE    { no warnings 'deprecated'; delete $_[0]->[$_[1]] }
 
 sub SPLICE
 {
diff --git a/lib/dumpvar.pl b/lib/dumpvar.pl
index 1c782dd..6b26d29 100644
--- a/lib/dumpvar.pl
+++ b/lib/dumpvar.pl
@@ -264,6 +264,7 @@ sub unwrap {
 	for $num (0 .. $tArrayDepth) {
 	    return if $DB::signal;
 	    print "$sp$num  ";
+	    no warnings 'deprecated';
 	    if (exists $v->[$num]) {
                 if (defined $v->[$num]) {
 	          DumpElem $v->[$num], $s, $m-1;
diff --git a/pod/perldiag.pod b/pod/perldiag.pod
index 05d1365..7927d06 100644
--- a/pod/perldiag.pod
+++ b/pod/perldiag.pod
@@ -5297,6 +5297,18 @@ will be available for use in new operators in the future.  Write C<m?\w?>
 instead, explicitly using the C<m> operator: the question mark delimiter
 still invokes match-once behaviour.
 
+=item Use of delete on array elements is deprecated
+
+(D deprecated) Use L<perlfunc/undef> to make an array element undefined.
+Use L<perlfunc/splice> or L<perlfunc/shift> to remove elements from an array.
+
+=item Use of exists on array elements is deprecated
+
+(D deprecated) The behavior of exists on array elements is not obvious and
+strongly tied to the deprecated use of delete on array elements. To check
+if an index falls within the bounds of an array, compare it against the
+number of elements in the array, like C<<@a >= 2>>.
+
 =item Use of qw(...) as parentheses is deprecated
 
 (D deprecated) You have something like C<foreach $x qw(a b c) {...}>,
diff --git a/pp.c b/pp.c
index 27d6a00..aa42db0 100644
--- a/pp.c
+++ b/pp.c
@@ -4479,6 +4479,8 @@ PP(pp_delete)
 	}
 	else if (hvtype == SVt_PVAV) {                  /* array element */
             if (PL_op->op_flags & OPf_SPECIAL) {
+		Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED),
+		    "Use of delete on array elements is deprecated");
                 while (++MARK <= SP) {
                     SV * const sv = av_delete(MUTABLE_AV(hv), SvIV(*MARK), discard);
                     *MARK = sv ? sv : &PL_sv_undef;
@@ -4505,8 +4507,11 @@ PP(pp_delete)
 	if (SvTYPE(hv) == SVt_PVHV)
 	    sv = hv_delete_ent(hv, keysv, discard, 0);
 	else if (SvTYPE(hv) == SVt_PVAV) {
-	    if (PL_op->op_flags & OPf_SPECIAL)
+	    if (PL_op->op_flags & OPf_SPECIAL) {
+		Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED),
+		    "Use of delete on array elements is deprecated");
 		sv = av_delete(MUTABLE_AV(hv), SvIV(keysv), discard);
+	    }
 	    else
 		DIE(aTHX_ "panic: avhv_delete no longer supported");
 	}
@@ -4545,6 +4550,8 @@ PP(pp_exists)
     }
     else if (SvTYPE(hv) == SVt_PVAV) {
 	if (PL_op->op_flags & OPf_SPECIAL) {		/* array element */
+	    Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED),
+		"Use of exists on array elements is deprecated");
 	    if (av_exists(MUTABLE_AV(hv), SvIV(tmpsv)))
 		RETPUSHYES;
 	}
diff --git a/t/op/args.t b/t/op/args.t
index 02d6352..063733a 100644
--- a/t/op/args.t
+++ b/t/op/args.t
@@ -93,6 +93,7 @@ for (1..3) {
     my $flag = 0;
     sub X::DESTROY { $flag = 1 }
     sub f {
+	no warnings 'deprecated';  # delete $a[]
 	delete $_[0];
 	ok(!$flag, 'delete $_[0] : in f');
     }
diff --git a/t/op/delete.t b/t/op/delete.t
index 52ee914..50906a9 100644
--- a/t/op/delete.t
+++ b/t/op/delete.t
@@ -80,7 +80,6 @@ $foo[5] = 'e';
 
     $foo = delete $foo[2];
 
-    local $TODO = 'Add deprecation warning for delete on array elements';
     is(0+@warns, 1);
     like($warns[0], qr/^Use of delete on array elements is deprecated\b/);
 }
@@ -92,7 +91,6 @@ cmp_ok($foo,'eq','b','ary delete 2');
 
     ok(!(exists $foo[2]),'ary b absent');
 
-    local $TODO = 'Add deprecation warning for exists on array elements';
     is(0+@warns, 1);
     like($warns[0], qr/^Use of exists on array elements is deprecated\b/);
 }
@@ -107,7 +105,6 @@ cmp_ok($foo[5],'eq','e','ary e exists');
 
     @bar = delete @foo[4,5];
 
-    local $TODO = 'Add deprecation warning for delete on array elements';
     is(0+@warns, 1);
     like($warns[0], qr/^Use of delete on array elements is deprecated\b/);
 }
@@ -115,8 +112,11 @@ cmp_ok($foo[5],'eq','e','ary e exists');
 cmp_ok(scalar(@bar),'==',2,'ary deleted slice');
 cmp_ok($bar[0],'eq','d','ary slice 1');
 cmp_ok($bar[1],'eq','e','ary slice 2');
-ok(!(exists $foo[4]),'ary d absent');
-ok(!(exists $foo[5]),'ary e absent');
+{
+    no warnings 'deprecated';  # exists $a[]
+    ok(!(exists $foo[4]),'ary d absent');
+    ok(!(exists $foo[5]),'ary e absent');
+}
 cmp_ok($foo[1],'eq','a','ary a still exists');
 cmp_ok($foo[3],'eq','c','ary c still exists');
 
@@ -125,6 +125,7 @@ cmp_ok($foo,'eq','ac','ary elems');
 cmp_ok(scalar(@foo),'==',4,'four is the number thou shalt count');
 
 foreach $key (0 .. $#foo) {
+    no warnings 'deprecated';  # delete $a[]
     delete $foo[$key];
 }
 
@@ -139,7 +140,10 @@ cmp_ok($foo,'eq','x y','two fresh');
 $refary[0]->[0] = "FOO";
 $refary[0]->[3] = "BAR";
 
-delete $refary[0]->[3];
+{
+    no warnings 'deprecated';  # delete $a[]
+    delete $refary[0]->[3];
+}
 
 cmp_ok( scalar(@{$refary[0]}),'==',1,'one down');
 
@@ -147,6 +151,7 @@ cmp_ok( scalar(@{$refary[0]}),'==',1,'one down');
     my @a = 33;
     my($a) = \(@a);
     my $b = \$a[0];
+    no warnings 'deprecated';  # delete $a[]
     my $c = \delete $a[bar];
 
     ok($a == $b && $b == $c,'a b c also equivalent');
@@ -164,6 +169,7 @@ cmp_ok( scalar(@{$refary[0]}),'==',1,'one down');
     {
 	my @a;
 	$a[0] = bless [], 'X';
+	no warnings 'deprecated';  # delete $a[]
 	my $y = delete $a[0];
     }
     cmp_ok($x,'==',1,q([perl #30733] array delete didn't free returned element));
diff --git a/t/op/local.t b/t/op/local.t
index d7a2975..ec170e7 100644
--- a/t/op/local.t
+++ b/t/op/local.t
@@ -101,8 +101,11 @@ ok(!defined $a[0]);
     is($a[4], 'x');
 }
 is(scalar(@a), 3);
-ok(!exists $a[3]);
-ok(!exists $a[4]);
+{
+    no warnings 'deprecated';  # exists $a[]
+    ok(!exists $a[3]);
+    ok(!exists $a[4]);
+}
 
 @a = ('a', 'b', 'c');
 {
@@ -115,7 +118,10 @@ ok(!exists $a[4]);
 is(scalar(@a), 5);
 ok(!defined $a[3]);
 is($a[4], 'y');
-ok(!exists $a[5]);
+{
+    no warnings 'deprecated';  # exists $a[]
+    ok(!exists $a[5]);
+}
 
 @a = ('a', 'b', 'c');
 {
@@ -126,10 +132,13 @@ ok(!exists $a[5]);
     is($a[6], 'z');
 }
 is(scalar(@a), 3);
-ok(!exists $a[3]);
-ok(!exists $a[4]);
-ok(!exists $a[5]);
-ok(!exists $a[6]);
+{
+    no warnings 'deprecated';  # exists $a[]
+    ok(!exists $a[3]);
+    ok(!exists $a[4]);
+    ok(!exists $a[5]);
+    ok(!exists $a[6]);
+}
 
 @a = ('a', 'b', 'c');
 {
@@ -144,7 +153,10 @@ is(scalar(@a), 6);
 ok(!defined $a[3]);
 ok(!defined $a[4]);
 is($a[5], 'y');
-ok(!exists $a[6]);
+{
+    no warnings 'deprecated';  # exists $a[]
+    ok(!exists $a[6]);
+}
 
 @a = ('a', 'b', 'c');
 {
@@ -161,6 +173,8 @@ is($a[0].$a[1], "Xb");
 @a = ('a', 'b', 'c');
 $a[4] = 'd';
 {
+    no warnings 'deprecated';  # delete $a[],exists $a[]
+
     delete local $a[1];
     is(scalar(@a), 5);
     is($a[0], 'a');
@@ -184,7 +198,10 @@ $a[4] = 'd';
 
     my $c = delete local $a[2];
     is(scalar(@a), 1);
-    ok(!exists($a[2]));
+    {
+	no warnings 'deprecated';  # exists $a[]
+	ok(!exists($a[2]));
+    }
     is($c, 'c');
 
     $a[888] = 'yyy';
@@ -196,9 +213,12 @@ is($a[1], 'b');
 is($a[2], 'c');
 ok(!defined($a[3]));
 is($a[4], 'd');
-ok(!exists($a[5]));
-ok(!exists($a[888]));
-ok(!exists($a[999]));
+{
+    no warnings 'deprecated';  # exists $a[]
+    ok(!exists($a[5]));
+    ok(!exists($a[888]));
+    ok(!exists($a[999]));
+}
 
 %h = (a => 1, b => 2, c => 3, d => 4);
 {
@@ -300,8 +320,8 @@ is($m, 5);
     sub TIEARRAY { bless [], $_[0] }
     sub STORE { print "# STORE [@_]\n"; $_[0]->[$_[1]] = $_[2] }
     sub FETCH { my $v = $_[0]->[$_[1]]; print "# FETCH [@_=$v]\n"; $v }
-    sub EXISTS { print "# EXISTS [@_]\n"; exists $_[0]->[$_[1]]; }
-    sub DELETE { print "# DELETE [@_]\n"; delete $_[0]->[$_[1]]; }
+    sub EXISTS { no warnings 'deprecated'; print "# EXISTS [@_]\n"; exists $_[0]->[$_[1]]; }
+    sub DELETE { no warnings 'deprecated'; print "# DELETE [@_]\n"; delete $_[0]->[$_[1]]; }
     sub CLEAR { print "# CLEAR [@_]\n"; @{$_[0]} = (); }
     sub FETCHSIZE { scalar(@{$_[0]}) }
     sub SHIFT { shift (@{$_[0]}) }
@@ -349,8 +369,11 @@ is("@a", "a b c", '@a should now contain original value');
     is($a[4], 'x');
 }
 is(scalar(@a), 3);
-ok(!exists $a[3]);
-ok(!exists $a[4]);
+{
+    no warnings 'deprecated';  # exists $a[]
+    ok(!exists $a[3]);
+    ok(!exists $a[4]);
+}
 
 @a = ('a', 'b', 'c');
 {
@@ -363,7 +386,10 @@ ok(!exists $a[4]);
 is(scalar(@a), 5);
 ok(!defined $a[3]);
 is($a[4], 'y');
-ok(!exists $a[5]);
+{
+    no warnings 'deprecated';  # exists $a[]
+    ok(!exists $a[5]);
+}
 
 @a = ('a', 'b', 'c');
 {
@@ -374,10 +400,13 @@ ok(!exists $a[5]);
     is($a[6], 'z');
 }
 is(scalar(@a), 3);
-ok(!exists $a[3]);
-ok(!exists $a[4]);
-ok(!exists $a[5]);
-ok(!exists $a[6]);
+{
+    no warnings 'deprecated';  # exists $a[]
+    ok(!exists $a[3]);
+    ok(!exists $a[4]);
+    ok(!exists $a[5]);
+    ok(!exists $a[6]);
+}
 
 @a = ('a', 'b', 'c');
 {
@@ -392,11 +421,16 @@ is(scalar(@a), 6);
 ok(!defined $a[3]);
 ok(!defined $a[4]);
 is($a[5], 'y');
-ok(!exists $a[6]);
+{
+    no warnings 'deprecated';  # exists $a[]
+    ok(!exists $a[6]);
+}
 
 @a = ('a', 'b', 'c');
 $a[4] = 'd';
 {
+    no warnings 'deprecated';  # delete $a[], exists $a[]
+
     delete local $a[1];
     is(scalar(@a), 5);
     is($a[0], 'a');
@@ -432,9 +466,12 @@ is($a[1], 'b');
 is($a[2], 'c');
 ok(!defined($a[3]));
 is($a[4], 'd');
-ok(!exists($a[5]));
-ok(!exists($a[888]));
-ok(!exists($a[999]));
+{
+    no warnings 'deprecated';  # exists $a[]
+    ok(!exists($a[5]));
+    ok(!exists($a[888]));
+    ok(!exists($a[999]));
+}
 
 # see if localization works on tied hashes
 {
diff --git a/t/op/reverse.t b/t/op/reverse.t
index 916724c..1733d9e 100644
--- a/t/op/reverse.t
+++ b/t/op/reverse.t
@@ -26,24 +26,33 @@ is(reverse(), "raboof");
     @a = reverse @a;
     is("@a", "4 3 2 1");
 
-    delete $a[1];
-    @a = reverse @a;
-    ok(!exists $a[2]);
-    is($a[0] . $a[1] . $a[3], '124');
+    {
+	no warnings 'deprecated';  # delete $a[], exists $a[]
+	delete $a[1];
+	@a = reverse @a;
+	ok(!exists $a[2]);
+	is($a[0] . $a[1] . $a[3], '124');
+    }
 
     @a = (5, 6, 7, 8, 9);
     @a = reverse @a;
     is("@a", "9 8 7 6 5");
 
-    delete $a[3];
-    @a = reverse @a;
-    ok(!exists $a[1]);
-    is($a[0] . $a[2] . $a[3] . $a[4], '5789');
+    {
+	no warnings 'deprecated';  # delete $a[], exists $a[]
+	delete $a[3];
+	@a = reverse @a;
+	ok(!exists $a[1]);
+	is($a[0] . $a[2] . $a[3] . $a[4], '5789');
+    }
 
-    delete $a[2];
-    @a = reverse @a;
-    ok(!exists $a[2] && !exists $a[3]);
-    is($a[0] . $a[1] . $a[4], '985');
+    {
+	no warnings 'deprecated';  # delete $a[], exists $a[]
+	delete $a[2];
+	@a = reverse @a;
+	ok(!exists $a[2] && !exists $a[3]);
+	is($a[0] . $a[1] . $a[4], '985');
+    }
 
     my @empty;
     @empty = reverse @empty;
@@ -59,24 +68,33 @@ use Tie::Array;
     @a = reverse @a;
     is("@a", "4 3 2 1");
 
-    delete $a[1];
-    @a = reverse @a;
-    ok(!exists $a[2]);
-    is($a[0] . $a[1] . $a[3], '124');
+    {
+	no warnings 'deprecated';  # delete $a[], exists $a[]
+	delete $a[1];
+	@a = reverse @a;
+	ok(!exists $a[2]);
+	is($a[0] . $a[1] . $a[3], '124');
+    }
 
     @a = (5, 6, 7, 8, 9);
     @a = reverse @a;
     is("@a", "9 8 7 6 5");
 
-    delete $a[3];
-    @a = reverse @a;
-    ok(!exists $a[1]);
-    is($a[0] . $a[2] . $a[3] . $a[4], '5789');
+    {
+	no warnings 'deprecated';  # delete $a[], exists $a[]
+	delete $a[3];
+	@a = reverse @a;
+	ok(!exists $a[1]);
+	is($a[0] . $a[2] . $a[3] . $a[4], '5789');
+    }
 
-    delete $a[2];
-    @a = reverse @a;
-    ok(!exists $a[2] && !exists $a[3]);
-    is($a[0] . $a[1] . $a[4], '985');
+    {
+	no warnings 'deprecated';  # delete $a[], exists $a[]
+	delete $a[2];
+	@a = reverse @a;
+	ok(!exists $a[2] && !exists $a[3]);
+	is($a[0] . $a[1] . $a[4], '985');
+    }
 
     tie my @empty, "Tie::StdArray";
     @empty = reverse @empty;
diff --git a/t/op/sub.t b/t/op/sub.t
index b8e514d..aa665c3 100644
--- a/t/op/sub.t
+++ b/t/op/sub.t
@@ -55,9 +55,9 @@ is(scalar(@test), 0, 'Didnt return anything');
 	   'result of delete(helem) is copied when explicitly returned';
     }
     my $x;
-    isnt \sub { delete $_[0] }->($x), \$x,
+    isnt \sub { no warnings 'deprecated'; delete $_[0] }->($x), \$x,
       'result of delete(aelem) is copied when returned';
-    isnt \sub { return delete $_[0] }->($x), \$x,
+    isnt \sub { no warnings 'deprecated'; return delete $_[0] }->($x), \$x,
       'result of delete(aelem) is copied when explicitly returned';
     isnt \sub { ()=\@_; shift }->($x), \$x,
       'result of shift is copied when returned';
diff --git a/t/op/tiearray.t b/t/op/tiearray.t
index 99b7938..591493b 100644
--- a/t/op/tiearray.t
+++ b/t/op/tiearray.t
@@ -124,6 +124,7 @@ sub DELETE {
   my ($ob,$id) = @_;
 #  print "# DELETE @_\n";
   $id += $offset;
+  no warnings 'deprecated';  # delete $a[]
   delete $ob->[$id];
 }
 
@@ -131,6 +132,7 @@ sub EXISTS {
   my ($ob,$id) = @_;
 #  print "# EXISTS @_\n";
   $id += $offset;
+  no warnings 'deprecated';  # exists $a[]
   exists $ob->[$id];
 }
 
@@ -283,6 +285,7 @@ untie @ary;
 
   # DELETE and EXISTS
   for (-2 .. 2) {
+    no warnings 'deprecated';  # delete $a[], exists $a[]
     ok($n[$_]);
     delete $n[$_];
     is(defined($n[$_]), '');
diff --git a/t/re/pat.t b/t/re/pat.t
index 54d44ac..8bc6f61 100644
--- a/t/re/pat.t
+++ b/t/re/pat.t
@@ -639,19 +639,22 @@ sub run_tests {
         ok(!defined $+ [3] && !defined $- [3] &&
            !defined $+ [4] && !defined $- [4], $message);
 
-        # Exists has a special check for @-/@+ - bug 45147
-        ok(exists $-[0], $message);
-        ok(exists $+[0], $message);
-        ok(exists $-[2], $message);
-        ok(exists $+[2], $message);
-        ok(!exists $-[3], $message);
-        ok(!exists $+[3], $message);
-        ok(exists $-[-1], $message);
-        ok(exists $+[-1], $message);
-        ok(exists $-[-3], $message);
-        ok(exists $+[-3], $message);
-        ok(!exists $-[-4], $message);
-        ok(!exists $+[-4], $message);
+	{
+            # Exists has a special check for @-/@+ - bug 45147
+	    no warnings 'deprecated';  # exists $a[]
+            ok(exists $-[0], $message);
+            ok(exists $+[0], $message);
+            ok(exists $-[2], $message);
+            ok(exists $+[2], $message);
+            ok(!exists $-[3], $message);
+            ok(!exists $+[3], $message);
+            ok(exists $-[-1], $message);
+            ok(exists $+[-1], $message);
+            ok(exists $-[-3], $message);
+            ok(exists $+[-3], $message);
+            ok(!exists $-[-4], $message);
+            ok(!exists $+[-4], $message);
+	}
 
         /.(a)(b)?(a)/;
         is($#+, 3, $message);
-- 
1.7.2.5

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 4, 2011

From @cpansprout

On Sun Dec 04 01​:47​:40 2011, ikegami@​adaelis.com wrote​:

This is a bug report for perl from ikegami@​adaelis.com,
generated with the help of perlbug 1.39 running under perl 5.14.0.

-----------------------------------------------------------------
[Please describe your issue here]

Hi,

exists($a[$i]) and delete($a[0]) are deprecated, yet they don't emit a
deprecation warning. There has been talk about removing them from core
or
changing their functionality, which required the deprecated warning to
be
emitted. The attached patches add a deprecation warning for those
operations.

I have previously gotten most use of exists($a[$i]) and delete($a[0])
removed from the cpan/ directory, but two remain​:

- Object​::Accessor has one remaining use, and there's an open ticket
to
have it fixed.
- Tie​::File needs to have C<< no warnings 'deprecated'; >> added
around its
tests for its EXISTS and DELETE handler. I'll be happy to submit a
patch
once this is committed.

Seeing that I maintain code that would produce literally thousands of
warnings as a result (and there’s nothing wrong with the code), I’m
opposed to this.

--

Father Chrysostomos

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 4, 2011

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 7, 2011

From @ikegami

On Sun, Dec 4, 2011 at 9​:34 AM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

Seeing that I maintain code that would produce literally thousands of
warnings as a result (and there’s nothing wrong with the code), I’m
opposed to this.

That only applies to #3 and maybe #2. #1 simply removes deprecated uses of
exists from XS-APItest.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 7, 2011

From @nwc10

On Tue, Dec 06, 2011 at 09​:53​:35PM -0500, Eric Brine wrote​:

On Sun, Dec 4, 2011 at 9​:34 AM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

Seeing that I maintain code that would produce literally thousands of
warnings as a result (and there?s nothing wrong with the code), I?m
opposed to this.

That only applies to #3 and maybe #2. #1 simply removes deprecated uses of
exists from XS-APItest.

Independent of everything else, I don't think that's a good idea.

Historically we've continued to test deprecated stuff until the commit that
actually removes it, to ensure that it doesn't change behaviour without us
knowing about it.

Nicholas Clark

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 7, 2011

From @ikegami

On Wed, Dec 7, 2011 at 6​:50 AM, Nicholas Clark via RT <
perlbug-followup@​perl.org> wrote​:

On Tue, Dec 06, 2011 at 09​:53​:35PM -0500, Eric Brine wrote​:

On Sun, Dec 4, 2011 at 9​:34 AM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

Seeing that I maintain code that would produce literally thousands of
warnings as a result (and there?s nothing wrong with the code), I?m
opposed to this.

That only applies to #3 and maybe #2. #1 simply removes deprecated uses
of
exists from XS-APItest.

Independent of everything else, I don't think that's a good idea.

Historically we've continued to test deprecated stuff until the commit that
actually removes it, to ensure that it doesn't change behaviour without us
knowing about it.

None of the patches remove any tests.

- Eric

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 16, 2011

From @ikegami

Hi,

The text of the opening post (below) describes patches #2 and #3.

Patch #1 changes two C<< exist($_[1]) >> to C<< @​_ >= 2 >>. Any objections
to that? They aren't part of tests for exists($a[]) or delete($a[]).

- Eric

On Sun, Dec 4, 2011 at 4​:47 AM, Eric Brine <perlbug-followup@​perl.org>wrote​:

# New Ticket Created by "Eric Brine"
# Please include the string​: [perl #105278]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=105278 >

This is a bug report for perl from ikegami@​adaelis.com,
generated with the help of perlbug 1.39 running under perl 5.14.0.

-----------------------------------------------------------------
[Please describe your issue here]

Hi,

exists($a[$i]) and delete($a[0]) are deprecated, yet they don't emit a
deprecation warning. There has been talk about removing them from core or
changing their functionality, which required the deprecated warning to be
emitted. The attached patches add a deprecation warning for those
operations.

I have previously gotten most use of exists($a[$i]) and delete($a[0])
removed from the cpan/ directory, but two remain​:

- Object​::Accessor has one remaining use, and there's an open ticket to
have it fixed.
- Tie​::File needs to have C<< no warnings 'deprecated'; >> added around its
tests for its EXISTS and DELETE handler. I'll be happy to submit a patch
once this is committed.

Thanks,
Eric

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 31, 2011

From @ikegami

On Fri, Dec 16, 2011 at 2​:25 PM, Eric Brine <ikegami@​adaelis.com> wrote​:

Hi,

Patch #1 changes two

unecessary uses of

C<< exist($_[1]) >> to C<< @​_ >= 2 >>. Any objections to that? They aren't
part of tests for exists($a[]) or delete($a[]).

Warnocked.

- Eric

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 31, 2011

From @cpansprout

On Fri Dec 16 11​:26​:15 2011, ikegami@​adaelis.com wrote​:

Hi,

The text of the opening post (below) describes patches #2 and #3.

Patch #1 changes two C<< exist($_[1]) >> to C<< @​_ >= 2 >>. Any
objections
to that? They aren't part of tests for exists($a[]) or delete($a[]).

I actually think the code reads more clearly with the exists() form.

--

Father Chrysostomos

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 31, 2011

From @ikegami

On Sat, Dec 31, 2011 at 12​:53 AM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

On Fri Dec 16 11​:26​:15 2011, ikegami@​adaelis.com wrote​:

Hi,

The text of the opening post (below) describes patches #2 and #3.

Patch #1 changes two C<< exist($_[1]) >> to C<< @​_ >= 2 >>. Any
objections
to that? They aren't part of tests for exists($a[]) or delete($a[]).

I actually think the code reads more clearly with the exists() form.

But it's not valid code according to the documentation. Are you saying the
documentation should be changed?

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 31, 2011

From @doy

On Sat, Dec 31, 2011 at 01​:12​:16AM -0500, Eric Brine wrote​:

On Sat, Dec 31, 2011 at 12​:53 AM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

On Fri Dec 16 11​:26​:15 2011, ikegami@​adaelis.com wrote​:

Hi,

The text of the opening post (below) describes patches #2 and #3.

Patch #1 changes two C<< exist($_[1]) >> to C<< @​_ >= 2 >>. Any
objections
to that? They aren't part of tests for exists($a[]) or delete($a[]).

I actually think the code reads more clearly with the exists() form.

But it's not valid code according to the documentation. Are you saying the
documentation should be changed?

The documentation says that it is deprecated, not invalid. That said, I
think it being deprecated is enough reason to agree with the patch.

-doy

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jul 3, 2012

From @doy

On Sat, Dec 31, 2011 at 04​:17​:44AM -0600, Jesse Luehrs wrote​:

On Sat, Dec 31, 2011 at 01​:12​:16AM -0500, Eric Brine wrote​:

On Sat, Dec 31, 2011 at 12​:53 AM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

On Fri Dec 16 11​:26​:15 2011, ikegami@​adaelis.com wrote​:

Hi,

The text of the opening post (below) describes patches #2 and #3.

Patch #1 changes two C<< exist($_[1]) >> to C<< @​_ >= 2 >>. Any
objections
to that? They aren't part of tests for exists($a[]) or delete($a[]).

I actually think the code reads more clearly with the exists() form.

But it's not valid code according to the documentation. Are you saying the
documentation should be changed?

The documentation says that it is deprecated, not invalid. That said, I
think it being deprecated is enough reason to agree with the patch.

And patch #1 is now applied as 5e5bb7c. Leaving #2 and #3 for now until
we come to a consensus on this issue.

-doy

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jul 24, 2012

From @rjbs

* Jesse Luehrs <doy@​tozt.net> [2012-07-03T02​:16​:05]

And patch #1 is now applied as 5e5bb7c. Leaving #2 and #3 for now until
we come to a consensus on this issue.

"this issue" here is "should using the deprecated exists($a[$i]) and
delete($a[$i])" warn?

Or is it going back to the notion of removing tests, which I believe was
rebutted, and not erroneously?

The use of exists and delete on array elements has been marked as deprecated
since 5.12, and I believe it should be issuing a deprecation warning if we are
ever to follow through and remove it.

--
rjbs

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jul 24, 2012

From @doy

On Mon, Jul 23, 2012 at 11​:11​:33PM -0400, Ricardo Signes wrote​:

* Jesse Luehrs <doy@​tozt.net> [2012-07-03T02​:16​:05]

And patch #1 is now applied as 5e5bb7c. Leaving #2 and #3 for now until
we come to a consensus on this issue.

"this issue" here is "should using the deprecated exists($a[$i]) and
delete($a[$i])" warn?

Or is it going back to the notion of removing tests, which I believe was
rebutted, and not erroneously?

The use of exists and delete on array elements has been marked as deprecated
since 5.12, and I believe it should be issuing a deprecation warning if we are
ever to follow through and remove it.

"This issue" being if whether or not we are planning on actually
removing exists and delete on arrays. I think we should, but it has been
fairly contentious when brought up in the past. I agree that these
warnings need to go in if we want to remove them.

-doy

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 12, 2017

From zefram@fysh.org

exists() and delete() on array elements are no longer officially
deprecated, but merely discouraged. The documentation changed in commit
2fbadc0. That undermines the basis
for the warning proposed in this ticket.

-zefram

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 12, 2017

From @xsawyerx

On Mon, 11 Dec 2017 22​:47​:10 -0800, zefram@​fysh.org wrote​:

exists() and delete() on array elements are no longer officially
deprecated, but merely discouraged. The documentation changed in commit
2fbadc0. That undermines the basis
for the warning proposed in this ticket.

Agreed. Rejecting ticket as it is no longer relevant to have a deprecation warning on something that is not deprecated.

@p5pRT p5pRT closed this as completed Dec 12, 2017
@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 12, 2017

@xsawyerx - Status changed from 'open' to 'rejected'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant