From @andk

This bug was discovered by investigating
http​://matrix.cpantesters.org/?dist=Data-Table-Text%202017.109, it's
an inverse BBC​: a warning went away and made a test pass but it should
not.

Expected behaviour​:
perl -le 'use warnings "syntax"; delete @​a[$x]; delete @​a{$x};print "Bye"'
Scalar value @​a[$x] better written as $a[$x] at -e line 1.
Scalar value @​a{$x} better written as $a{$x} at -e line 1.
Bye

Observed behaviour​:
perl -le 'use warnings "syntax"; delete @​a[$x]; delete @​a{$x};print "Bye"'
Bye

Bisect points to v5.19.3-506-g429a25554a​:

commit 429a255
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Sat Sep 14 17​:23​:11 2013 -0700

  Reduce false positives for @​hsh{$s} and @​ary[$s] warnings

perl -V

Summary of my perl5 (revision 5 version 19 subversion 4) configuration​:
  Derived from​: 429a255
  Platform​:
  osname=linux, osvers=4.8.0-2-amd64, archname=x86_64-linux
  uname='linux k93msid 4.8.0-2-amd64 #1 smp debian 4.8.11-1 (2016-12-02) x86_64 gnulinux '
  config_args='-Dprefix=/home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.19.3-506-g429a25554a/89ad -Dmyhostname=k93msid -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Dlibswanted=cl pthread socket inet nsl gdbm dbm malloc dl ld sun m crypt sec util c cposix posix ucb BSD gdbm_compat -Uuseithreads -Uuselongdouble -DDEBUGGING=-g'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2 -g',
  cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='6.2.1 20161124', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib
  libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.24.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.24'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector'

Characteristics of this binary (from libperl)​:
  Compile-time options​: HAS_TIMES PERLIO_LAYERS PERL_DONT_CREATE_GVSV
  PERL_HASH_FUNC_ONE_AT_A_TIME_HARD PERL_MALLOC_WRAP
  PERL_NEW_COPY_ON_WRITE PERL_PRESERVE_IVUV
  PERL_USE_DEVEL USE_64_BIT_ALL USE_64_BIT_INT
  USE_LARGE_FILES USE_LOCALE USE_LOCALE_COLLATE
  USE_LOCALE_CTYPE USE_LOCALE_NUMERIC USE_PERLIO
  USE_PERL_ATOF
  Locally applied patches​:
  uncommitted-changes
  Built under linux
  Compiled at Jan 12 2017 09​:37​:16
  @​INC​:
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.19.3-506-g429a25554a/89ad/lib/site_perl/5.19.4/x86_64-linux
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.19.3-506-g429a25554a/89ad/lib/site_perl/5.19.4
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.19.3-506-g429a25554a/89ad/lib/5.19.4/x86_64-linux
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.19.3-506-g429a25554a/89ad/lib/5.19.4
  .

--
andreas

From @jkeenan

On Thu, 12 Jan 2017 12​:13​:16 GMT, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

This bug was discovered by investigating
http​://matrix.cpantesters.org/?dist=Data-Table-Text%202017.109, it's
an inverse BBC​: a warning went away and made a test pass but it should
not.

Expected behaviour​:
perl -le 'use warnings "syntax"; delete @​a[$x];
delete @​a{$x};print "Bye"'
Scalar value @​a[$x] better written as $a[$x] at -e line 1.
Scalar value @​a{$x} better written as $a{$x} at -e line 1.
Bye

Observed behaviour​:
perl -le 'use warnings "syntax"; delete @​a[$x]; delete @​a{$x};print
"Bye"'
Bye

Bisect points to v5.19.3-506-g429a25554a​:

commit 429a255
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Sat Sep 14 17​:23​:11 2013 -0700

Reduce false positives for @​hsh{$s} and @​ary[$s] warnings

Confirmed.

#####
$ perl -v | head -2 | tail -1
This is perl 5, version 18, subversion 4 (v5.18.4) built for x86_64-linux

$ perl -le 'use warnings "syntax"; delete @​a[$x]; delete @​a{$x};print "Bye"'
Scalar value @​a[$x] better written as $a[$x] at -e line 1.
Scalar value @​a{$x} better written as $a{$x} at -e line 1.
Bye

#####

$ perl -v | head -2 | tail -1
This is perl 5, version 24, subversion 0 (v5.24.0) built for x86_64-linux

$ perl -le 'use warnings "syntax"; delete @​a[$x]; delete @​a{$x};print "Bye"'
Bye

#####

--
James E Keenan (jkeenan@​cpan.org)

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

From @hvds

On Thu, 12 Jan 2017 04​:13​:16 -0800, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

This bug was discovered by investigating
http​://matrix.cpantesters.org/?dist=Data-Table-Text%202017.109, it's
an inverse BBC​: a warning went away and made a test pass but it should
not.

Expected behaviour​:
perl -le 'use warnings "syntax"; delete @​a[$x];
delete @​a{$x};print "Bye"'
Scalar value @​a[$x] better written as $a[$x] at -e line 1.
Scalar value @​a{$x} better written as $a{$x} at -e line 1.
Bye

Observed behaviour​:
perl -le 'use warnings "syntax"; delete @​a[$x]; delete @​a{$x};print
"Bye"'
Bye

Bisect points to v5.19.3-506-g429a25554a​:

commit 429a255
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Sat Sep 14 17​:23​:11 2013 -0700

Reduce false positives for @​hsh{$s} and @​ary[$s] warnings

Hi Andreas, why do you think it was wrong for the warning to go away in this case? The intent of that commit was to reduce false positives by (among other things) not giving it when the context is list mode, as in your example.

As it happens, for the case of the offending source line​:
  delete @​parms{$_} for keys %$permitted;
.. embracing the slice is actually a better improvement than removing it.

Hugo

From @andk

On Fri, 13 Jan 2017 02​:36​:54 -0800, "Hugo van der Sanden via RT" <perlbug-followup@​perl.org> said​:

  > On Thu, 12 Jan 2017 04​:13​:16 -0800, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

This bug was discovered by investigating
http​://matrix.cpantesters.org/?dist=Data-Table-Text%202017.109, it's
an inverse BBC​: a warning went away and made a test pass but it should
not.

Expected behaviour​:
perl -le 'use warnings "syntax"; delete @​a[$x];
delete @​a{$x};print "Bye"'
Scalar value @​a[$x] better written as $a[$x] at -e line 1.
Scalar value @​a{$x} better written as $a{$x} at -e line 1.
Bye

Observed behaviour​:
perl -le 'use warnings "syntax"; delete @​a[$x]; delete @​a{$x};print
"Bye"'
Bye

Bisect points to v5.19.3-506-g429a25554a​:

commit 429a255
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Sat Sep 14 17​:23​:11 2013 -0700

Reduce false positives for @​hsh{$s} and @​ary[$s] warnings

  > Hi Andreas, why do you think it was wrong for the warning to go away
  > in this case? The intent of that commit was to reduce false positives
  > by (among other things) not giving it when the context is list mode,
  > as in your example.

  > As it happens, for the case of the offending source line​:
  > delete @​parms{$_} for keys %$permitted;
  > .. embracing the slice is actually a better improvement than removing it.

Oh, your point of view is interesting. But I do not believe removing the
warning was the intent of the commit in question.

Look for a comparison at other list ops C<print> or C<utime>. Warning
for those still there​:

: perl -le 'use warnings "syntax"; print @​a[$x]; print @​a{$x};print "Bye"'
: Scalar value @​a[...] better written as $a[...] at -e line 1.
: Scalar value @​a{...} better written as $a{...} at -e line 1.
:
:
: Bye

: perl -le 'use warnings "syntax"; utime 0,0,@​a[$x]; utime 0,0, @​a{$x};print "Bye"'
: Scalar value @​a[...] better written as $a[...] at -e line 1.
: Scalar value @​a{...} better written as $a{...} at -e line 1.
: Bye

--
andreas

From @hvds

On Fri, 13 Jan 2017 04​:05​:43 -0800, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

On Fri, 13 Jan 2017 02​:36​:54 -0800, "Hugo van der Sanden via RT"
<perlbug-followup@​perl.org> said​:

On Thu, 12 Jan 2017 04​:13​:16 -0800,
andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

This bug was discovered by investigating
http​://matrix.cpantesters.org/?dist=Data-Table-Text%202017.109, it's
an inverse BBC​: a warning went away and made a test pass but it
should
not.

Expected behaviour​:
perl -le 'use warnings "syntax"; delete @​a[$x];
delete @​a{$x};print "Bye"'
Scalar value @​a[$x] better written as $a[$x] at -e line 1.
Scalar value @​a{$x} better written as $a{$x} at -e line 1.
Bye

Observed behaviour​:
perl -le 'use warnings "syntax"; delete @​a[$x]; delete @​a{$x};print
"Bye"'
Bye

Bisect points to v5.19.3-506-g429a25554a​:

commit 429a255
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Sat Sep 14 17​:23​:11 2013 -0700

Reduce false positives for @​hsh{$s} and @​ary[$s] warnings

Hi Andreas, why do you think it was wrong for the warning to go away
in this case? The intent of that commit was to reduce false positives
by (among other things) not giving it when the context is list mode,
as in your example.

As it happens, for the case of the offending source line​:
delete @​parms{$_} for keys %$permitted;
.. embracing the slice is actually a better improvement than removing
it.

Oh, your point of view is interesting. But I do not believe removing
the
warning was the intent of the commit in question.

Look for a comparison at other list ops C<print> or C<utime>. Warning
for those still there​:

: perl -le 'use warnings "syntax"; print @​a[$x]; print @​a{$x};print
"Bye"'
: Scalar value @​a[...] better written as $a[...] at -e line 1.
: Scalar value @​a{...} better written as $a{...} at -e line 1.
:
:
: Bye

: perl -le 'use warnings "syntax"; utime 0,0,@​a[$x]; utime 0,0,
@​a{$x};print "Bye"'
: Scalar value @​a[...] better written as $a[...] at -e line 1.
: Scalar value @​a{...} better written as $a{...} at -e line 1.
: Bye

Thanks, that helps motivate where to look.

This appears to happen because Perl_ck_delete() nulls out the ASLICE (or HSLICE) op before it gets as far as checking for this warning during S_finalize_op().

I see we can use o->op_targ to check the original type of a nulled op. I've no idea if this is appropriate, but the approach below does reenable the warnings​:

% ./miniperl -Ilib -Mwarnings=syntax -e 'delete @​a[$b]; delete @​a{$b}'
Scalar value @​a[...] better written as $a[...] at -e line 1.
Scalar value @​a{...} better written as $a{...} at -e line 1.
%

Father C., does this look sane to you?

Hugo
--- a/op.c
+++ b/op.c
@​@​ -1670,14 +1670,17 @​@​ static void
S_scalar_slice_warning(pTHX_ const OP *o)
{
  OP *kid;
- const char lbrack =
- o->op_type == OP_HSLICE ? '{' : '[';
- const char rbrack =
- o->op_type == OP_HSLICE ? '}' : ']';
+ OPCODE type = o->op_type;
+ char lbrack, rbrack;
  SV *name;
  SV *keysv = NULL; /* just to silence compiler warnings */
  const char *key = NULL;

+ if (type == OP_NULL)
+ type = (OPCODE)o->op_targ;
+ lbrack = (type == OP_HSLICE) ? '{' : '[';
+ rbrack = (type == OP_HSLICE) ? '}' : ']';
+
  if (!(o->op_private & OPpSLICEWARNING))
  return;
  if (PL_parser && PL_parser->error_count)
@​@​ -2599,6 +2602,11 @​@​ S_finalize_op(pTHX_ OP* o)
  case OP_ASLICE​:
  S_scalar_slice_warning(aTHX_ o);
  break;
+ case OP_NULL​:
+ /* ck_delete() may have nulled out an op we should still warn on */
+ if (o->op_targ == OP_ASLICE || o->op_targ == OP_HSLICE)
+ S_scalar_slice_warning(aTHX_ o);
+ break;

  case OP_SUBST​: {
  if (cPMOPo->op_pmreplrootu.op_pmreplroot)

From @cpansprout

On Thu, 12 Jan 2017 04​:13​:16 -0800, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

This bug was discovered by investigating
http​://matrix.cpantesters.org/?dist=Data-Table-Text%202017.109, it's
an inverse BBC​: a warning went away and made a test pass but it should
not.

Expected behaviour​:
perl -le 'use warnings "syntax"; delete @​a[$x];
delete @​a{$x};print "Bye"'
Scalar value @​a[$x] better written as $a[$x] at -e line 1.
Scalar value @​a{$x} better written as $a{$x} at -e line 1.
Bye

Observed behaviour​:
perl -le 'use warnings "syntax"; delete @​a[$x]; delete @​a{$x};print
"Bye"'
Bye

Bisect points to v5.19.3-506-g429a25554a​:

commit 429a255
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Sat Sep 14 17​:23​:11 2013 -0700

Reduce false positives for @​hsh{$s} and @​ary[$s] warnings

This is now fixed in commit fe7df09.

--

Father Chrysostomos

@cpansprout - Status changed from 'open' to 'pending release'

From @cpansprout

On Fri, 13 Jan 2017 05​:47​:20 -0800, hv wrote​:

Father C., does this look sane to you?

Sorry, I totally missed your discussion and did not realise you were already working on it. (I started working on it before you sent your first message, and then I did not bother reading p5p in the mean time.)

Hugo
--- a/op.c
+++ b/op.c
@​@​ -1670,14 +1670,17 @​@​ static void
S_scalar_slice_warning(pTHX_ const OP *o)
{
OP *kid;
- const char lbrack =
- o->op_type == OP_HSLICE ? '{' : '[';
- const char rbrack =
- o->op_type == OP_HSLICE ? '}' : ']';
+ OPCODE type = o->op_type;
+ char lbrack, rbrack;
SV *name;
SV *keysv = NULL; /* just to silence compiler warnings */
const char *key = NULL;

+ if (type == OP_NULL)
+ type = (OPCODE)o->op_targ;
+ lbrack = (type == OP_HSLICE) ? '{' : '[';
+ rbrack = (type == OP_HSLICE) ? '}' : ']';
+
if (!(o->op_private & OPpSLICEWARNING))
return;
if (PL_parser && PL_parser->error_count)
@​@​ -2599,6 +2602,11 @​@​ S_finalize_op(pTHX_ OP* o)
case OP_ASLICE​:
S_scalar_slice_warning(aTHX_ o);
break;
+ case OP_NULL​:
+ /* ck_delete() may have nulled out an op we should still warn
on */
+ if (o->op_targ == OP_ASLICE || o->op_targ == OP_HSLICE)
+ S_scalar_slice_warning(aTHX_ o);
+ break;

case OP_SUBST​: {
if (cPMOPo->op_pmreplrootu.op_pmreplroot)

Pretty much equivalent to what I committed yesterday.

Sorry about the duplicate effort.

--

Father Chrysostomos

From @hvds

On Sat, 14 Jan 2017 10​:01​:36 -0800, sprout wrote​:

On Fri, 13 Jan 2017 05​:47​:20 -0800, hv wrote​:

Father C., does this look sane to you?

Sorry, I totally missed your discussion and did not realise you were
already working on it. (I started working on it before you sent your
first message, and then I did not bother reading p5p in the mean
time.)

No worries, happy to have it fixed.

Hugo

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release today of Perl 5.26.0, this and 210 other issues have been
resolved.

Perl 5.26.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.26.0

If you find that the problem persists, feel free to reopen this ticket.

@khwilliamson - Status changed from 'pending release' to 'resolved'