Perl / perl5

đŸȘ The Perl programming language
https://dev.perl.org/perl5/
Other
1.98k stars 559 forks source link

Subroutine doesn't create elements. #5310

Closed p5pRT closed 6 years ago

p5pRT commented 22 years ago

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

Searchable as RT8910$

p5pRT commented 22 years ago

From @abigail

Created by @abigail

man perlsub states​:

  Any arguments passed in show up in the array "@​_". There-   fore\, if you called a function with two arguments\, those   would be stored in "$_[0]" and "$_[1]". The array "@​_" is   a local array\, but its elements are aliases for the actual   scalar parameters. In particular\, if an element "$_[0]"   is updated\, the corresponding argument is updated (or an   error occurs if it is not updatable). If an argument is   an array or hash element which did not exist when the   function was called\, that element is created only when   (and if) it is modified or a reference to it is taken.

Now\, consider this program​:

  #!/usr/bin/perl -w

  use strict;

  sub mysub;

  $\, = " ";   $\ = "\n";

  # 0 1 2 3   my @​arr = ("cent"\, "nickel"\, "dime"\, undef);   $arr [5] = "dollar"; # Auto-vivifies $arr [4] to be 'undef'.

  print map {defined $_ ? $_ : "^^"} @​arr;   mysub @​arr;   print map {defined $_ ? $_ : "^^"} @​arr;

  sub mysub {   $_ [0] = 0.01;   $_ [1] = 0.05;   $_ [2] = 0.10;   $_ [3] = 0.25;   $_ [4] = 0.50;   $_ [5] = 1.00;   }   __END__

If we run this\, we get​:   cent nickel dime ^^ ^^ dollar   0.01 0.05 0.1 0.25 ^^ 1

That is\, $arr [4] isn't 0.50 after the function mysub was called\, while the documentation specifies it should.

bleadperl gives the same behaviour.

Abigail

Perl Info ``` Flags: category=core severity=low Site configuration information for perl v5.6.1: Configured by abigail at Tue Jul 10 12:17:49 CEST 2001. Summary of my perl5 (revision 5.0 version 6 subversion 1) configuration: Platform: osname=linux, osvers=2.4.5, archname=i686-linux-64int-ld uname='linux hermione 2.4.5 #6 fri jun 22 01:38:20 pdt 2001 i686 unknown ' config_args='-d -Dprefix=/opt/perl -Doptimize=-g -Dusemorebits' 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=define use64bitall=undef uselongdouble=define Compiler: cc='cc', ccflags ='-DDEBUGGING -fno-strict-aliasing -I/usr/local/include -I/opt/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-g', cppflags='-DDEBUGGING -fno-strict-aliasing -I/usr/local/include -I/opt/local/include' ccversion='', gccversion='2.95.3 20010315 (release)', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 ivtype='long long', ivsize=8, nvtype='long double', nvsize=12, Off_t='off_t', lseeksize=8 alignbytes=4, usemymalloc=n, prototype=define Linker and Libraries: ld='cc', ldflags =' -L/usr/local/lib -L/opt/local/lib' libpth=/usr/local/lib /opt/local/lib /lib /usr/lib libs=-lnsl -lndbm -lgdbm -ldl -lm -lc -lcrypt -lutil perllibs=-lnsl -ldl -lm -lc -lcrypt -lutil libc=/lib/libc-2.2.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 -L/opt/local/lib' Locally applied patches: @INC for perl v5.6.1: /home/abigail/Perl /home/abigail/Sybase /opt/perl/lib/5.6.1/i686-linux-64int-ld /opt/perl/lib/5.6.1 /opt/perl/lib/site_perl/5.6.1/i686-linux-64int-ld /opt/perl/lib/site_perl/5.6.1 /opt/perl/lib/site_perl . Environment for perl v5.6.1: HOME=/home/abigail LANG (unset) LANGUAGE (unset) LC_ALL=POSIX LD_LIBRARY_PATH=/home/abigail/Lib:/usr/local/lib:/usr/lib:/lib:/usr/X11R6/lib:/opt/gnome/lib LOGDIR (unset) PATH=/home/abigail/Bin:/opt/perl/bin:/usr/local/bin:/usr/local/X11/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/usr/X11R6/bin:/usr/games:/opt/povray/bin:/opt/gnome/bin:/opt/opera/bin:/usr/share/texmf/bin:/opt/Acrobat4/bin:/opt/java/blackdown/j2sdk1.3.1/bin:/usr/local/games/bin:/opt/gnuplot/bin:/opt/mysql/bin PERL5LIB=/home/abigail/Perl:/home/abigail/Sybase PERLDIR=/opt/perl PERL_BADLANG (unset) SHELL=/usr/bin/bash ```
p5pRT commented 22 years ago

From @schwern

On Fri\, Apr 05\, 2002 at 02​:50​:30PM -0000\, abigail@​foad.org wrote​: \

\#             0        1         2      3
my @​arr  = \("cent"\, "nickel"\, "dime"\, undef\);
$arr \[5\] = "dollar";     \# Auto\-vivifies $arr \[4\] to be 'undef'\.

print map \{defined $\_ ? $\_ : "^^"\} @​arr;
mysub @​arr;
print map \{defined $\_ ? $\_ : "^^"\} @​arr;

sub mysub \{
    $\_ \[0\] = 0\.01;
    $\_ \[1\] = 0\.05;
    $\_ \[2\] = 0\.10;
    $\_ \[3\] = 0\.25;
    $\_ \[4\] = 0\.50;
    $\_ \[5\] = 1\.00;
\}
\_\_END\_\_

If we run this\, we get​: cent nickel dime ^^ ^^ dollar 0.01 0.05 0.1 0.25 ^^ 1

That is\, $arr [4] isn't 0.50 after the function mysub was called\, while the documentation specifies it should.

Likely related to this feature​:

$ perl -wle '@​a = ("foo "\, "bar "\, "woof "\, undef); $a[5] = "wibble "; print map {defined $_ ? $_ : "^^ "} @​a; print map {exists $a[$_] ? "exists " : "^^ "} 0..$#a;' foo bar woof ^^ ^^ wibble exists exists exists exists ^^ exists

$a[3] is not defined\, but it exists. $a[4] exists but is not defined.

Internally\, $a[3] would contain an SV with the value of undef and $a[4] contains PL_sv_undef.

but since this feature was added in 5.6.0 (or there abouts) and your bug goes back to at least 5.004\, I could be wrong.

Devel​::Peek seems to be confused by @​arr. It doesn't see past element 3.

SV = RV(0x10138350) at 0x1012ef44   REFCNT = 1   FLAGS = (TEMP\,ROK)   RV = 0x1012a2f8   SV = PVAV(0x1011f5e8) at 0x1012a2f8   REFCNT = 2   FLAGS = (PADBUSY\,PADMY)   IV = 0   NV = 0   ARRAY = 0x1012c8f8   FILL = 5   MAX = 11   ARYLEN = 0x0   FLAGS = (REAL)   Elt No. 0   SV = PVNV(0x1013fe38) at 0x1011e1d0   REFCNT = 1   FLAGS = (NOK\,pNOK)   IV = 0   NV = 0.01   PV = 0x10130260 "cent"\0   CUR = 4   LEN = 5   Elt No. 1   SV = PVNV(0x1013fe58) at 0x1011e20c   REFCNT = 1   FLAGS = (NOK\,pNOK)   IV = 0   NV = 0.05   PV = 0x10126028 "nickel"\0   CUR = 6   LEN = 7   Elt No. 2   SV = PVNV(0x1013fe78) at 0x1012a28c   REFCNT = 1   FLAGS = (NOK\,pNOK)   IV = 0   NV = 0.1   PV = 0x10126038 "dime"\0   CUR = 4   LEN = 5   Elt No. 3   SV = NV(0x10143500) at 0x1012e064   REFCNT = 1   FLAGS = (NOK\,pNOK)   NV = 0.25

--

Michael G. Schwern \schwern@​pobox\.com http​://www.pobox.com/~schwern/ Perl Quality Assurance \perl\-qa@​perl\.org Kwalitee Is Job One It's Absinthe time!

p5pRT commented 22 years ago

From @abigail

On Fri\, Apr 05\, 2002 at 10​:51​:44AM -0500\, Michael G Schwern wrote​:

On Fri\, Apr 05\, 2002 at 02​:50​:30PM -0000\, abigail@​foad.org wrote​: \

\#             0        1         2      3
my @​arr  = \("cent"\, "nickel"\, "dime"\, undef\);
$arr \[5\] = "dollar";     \# Auto\-vivifies $arr \[4\] to be 'undef'\.

print map \{defined $\_ ? $\_ : "^^"\} @​arr;
mysub @​arr;
print map \{defined $\_ ? $\_ : "^^"\} @​arr;

sub mysub \{
    $\_ \[0\] = 0\.01;
    $\_ \[1\] = 0\.05;
    $\_ \[2\] = 0\.10;
    $\_ \[3\] = 0\.25;
    $\_ \[4\] = 0\.50;
    $\_ \[5\] = 1\.00;
\}
\_\_END\_\_

If we run this\, we get​: cent nickel dime ^^ ^^ dollar 0.01 0.05 0.1 0.25 ^^ 1

That is\, $arr [4] isn't 0.50 after the function mysub was called\, while the documentation specifies it should.

Likely related to this feature​:

$ perl -wle '@​a = ("foo "\, "bar "\, "woof "\, undef); $a[5] = "wibble "; print map {defined $_ ? $_ : "^^ "} @​a; print map {exists $a[$_] ? "exists " : "^^ "} 0..$#a;' foo bar woof ^^ ^^ wibble exists exists exists exists ^^ exists

$a[3] is not defined\, but it exists. $a[4] exists but is not defined.

I think you mean that $a [4] neither exists\, nor is defined.

But that's exactly my point. The documentation says​:

  If an argument is an array or hash element which DID NOT EXIST when   the function was called\, that element is created only when (and if)   it is modified or a reference to it is taken.

The element doesn't exist (as exists shows)\, is modified\, yet isn't created.

Abigail

p5pRT commented 22 years ago

From @schwern

On Fri\, Apr 05\, 2002 at 06​:26​:17PM +0200\, abigail@​foad.org wrote​:

$a[3] is not defined\, but it exists. $a[4] exists but is not defined.

I think you mean that $a [4] neither exists\, nor is defined.

But that's exactly my point. The documentation says​:

Yes\, I'm not disagreeing with you (despite my brain-o\, it's early for me). Just noting it's likely a the result of $a[4]'s internally having the special value of PL_sv_undef and the @​_ magic not aliasing it because of this. Devel​::Peek not taking this special case into account either is an omen.

--

Michael G. Schwern \schwern@​pobox\.com http​://www.pobox.com/~schwern/ Perl Quality Assurance \perl\-qa@​perl\.org Kwalitee Is Job One It should indeed be said that notwithstanding the fact that I make ambulatory progress through the umbragious inter-hill mortality slot\, terror sensations will no be initiated in me\, due to para-etical phenomena.

p5pRT commented 22 years ago

From @tamias

On Fri\, Apr 05\, 2002 at 02​:50​:30PM -0000\, abigail@​foad.org wrote​:

man perlsub states​:

   Any arguments passed in show up in the array "@​\_"\.  There\-
   fore\, if you called a function with two arguments\, those
   would be stored in "$\_\[0\]" and "$\_\[1\]"\.  The array "@​\_" is
   a local array\, but its elements are aliases for the actual
   scalar parameters\.  In particular\, if an element "$\_\[0\]"
   is updated\, the corresponding argument is updated \(or an
   error occurs if it is not updatable\)\.  If an argument is
   an array or hash element which did not exist when the
   function was called\, that element is created only when
   \(and if\) it is modified or a reference to it is taken\.

I believe you are misreading the documentation. It's referring to a situation like this​:

#!perl -w

sub mysub1 {   my $x = $_[0]; }

sub mysub2 {   $_[0] = 1; }

mysub1($arr[0]);

print "$arr[0]\n";

mysub2($arr[0]);

print "$arr[0]\n"; __END__ Use of uninitialized value at tmp.pl - line 13.

1

Note that the subs are being called with "an array ... element which did not exist when the function was called"\, and that "that element is created only when ... it is modified" in mysub2.

Keep in mind that arrays passed to subroutines are flattened\, so​:

@​arr = (0\, 1); mysub(@​arr);

is equivalent to​:

@​arr = (0\, 1); mysub($arr[0]\, $arr[1]);

mysub has no notion that it was called with @​arr as an argument. The elements of @​_ are aliased individually to the elements of @​arr; @​_ as a whole is not aliased to @​arr as a whole. So\, adding elements off the end of @​_ does not add elements to @​arr.

Ronald

p5pRT commented 22 years ago

From @tamias

On Fri\, Apr 05\, 2002 at 11​:58​:58AM -0500\, Ronald J Kimball wrote​:

On Fri\, Apr 05\, 2002 at 02​:50​:30PM -0000\, abigail@​foad.org wrote​:

man perlsub states​:

   Any arguments passed in show up in the array "@​\_"\.  There\-
   fore\, if you called a function with two arguments\, those
   would be stored in "$\_\[0\]" and "$\_\[1\]"\.  The array "@​\_" is
   a local array\, but its elements are aliases for the actual
   scalar parameters\.  In particular\, if an element "$\_\[0\]"
   is updated\, the corresponding argument is updated \(or an
   error occurs if it is not updatable\)\.  If an argument is
   an array or hash element which did not exist when the
   function was called\, that element is created only when
   \(and if\) it is modified or a reference to it is taken\.

I believe you are misreading the documentation. It's referring to a situation like this​:

On second thought\, I think I misread your bug report. Apologies. I see now what the problem is\, and it does look like misbehavior.

Ronald

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

On Fri\, 5 Apr 2002 12​:05​:40 -0500\, rjk@​linguist.Thayer.dartmouth.edu (Ronald J Kimball) wrote​:

On second thought\, I think I misread your bug report. Apologies. I see now what the problem is\, and it does look like misbehavior.

On the third hand\, if $arr[4] is &PL_sv_undef\, then assigning to $_[4] is basically the same as assigning "undef = 0.50;". Perl can't easily put something into $arr[4] because the &PL_sv_undef that got pushed on the stack could be "anything".

On the fourth hand\, it does mean that the documentation is incorrect (or insufficient). Not sure whether the documentation or the code should be changed\, though. (I think the docs should change to reflect the code\, but that's probably biassed by perceived difficulty of making the code reflect the documentation in this aspect.)

Cheers\, Philip

p5pRT commented 22 years ago

From @schwern

On Fri\, Apr 05\, 2002 at 09​:26​:41PM +0200\, Philip Newton wrote​:

On the third hand\, if $arr[4] is &PL_sv_undef\, then assigning to $_[4] is basically the same as assigning "undef = 0.50;". Perl can't easily put something into $arr[4] because the &PL_sv_undef that got pushed on the stack could be "anything".

$arr[4] containing &PL_sv_undef is correct.

@​_ aliasing is definately doing something Evil and probably not going through the AV API and likely doesn't know how to deal with an array element which is PL_sv_undef.

On the fourth hand\, it does mean that the documentation is incorrect (or insufficient). Not sure whether the documentation or the code should be changed\, though. (I think the docs should change to reflect the code\, but that's probably biassed by perceived difficulty of making the code reflect the documentation in this aspect.)

Don't document bugs as features. Then you can never fix it.

--

Michael G. Schwern \schwern@​pobox\.com http​://www.pobox.com/~schwern/ Perl Quality Assurance \perl\-qa@​perl\.org Kwalitee Is Job One We're talkin' to you\, weaselnuts.   http​://www.goats.com/archive/000831.html

p5pRT commented 22 years ago

From @abigail

On Fri\, Apr 05\, 2002 at 09​:26​:41PM +0200\, Philip Newton wrote​:

On Fri\, 5 Apr 2002 12​:05​:40 -0500\, rjk@​linguist.Thayer.dartmouth.edu (Ronald J Kimball) wrote​:

On second thought\, I think I misread your bug report. Apologies. I see now what the problem is\, and it does look like misbehavior.

On the third hand\, if $arr[4] is &PL_sv_undef\, then assigning to $_[4] is basically the same as assigning "undef = 0.50;". Perl can't easily put something into $arr[4] because the &PL_sv_undef that got pushed on the stack could be "anything".

On the fourth hand\, it does mean that the documentation is incorrect (or insufficient). Not sure whether the documentation or the code should be changed\, though. (I think the docs should change to reflect the code\, but that's probably biassed by perceived difficulty of making the code reflect the documentation in this aspect.)

Changing the documentation means you are extending the language. From a language perspective\, there is just one kind of "undef"\, &PL_sv_undef might make sense to many people on this list\, but the language Perl doesn't know any PL_sv_undef.

You'd have a hard time to explain why the following doesn't print twice the same line​:

  #!/usr/bin/perl -w

  use strict;

  sub mysub;

  $\, = " ";   $\ = "\n";

  my @​arr1 = ("cent"\, "nickel"\, "dime"\, undef);   $arr1 [5] = "dollar"; # Auto-vivifies $arr1 [4];

  my @​arr2 = @​arr1;

  mysub @​arr1;   mysub @​arr2;   print map {defined $_ ? $_ : "^^^"} @​arr1;   print map {defined $_ ? $_ : "^^^"} @​arr2;

  sub mysub {   $_ [0] = 0.01;   $_ [1] = 0.05;   $_ [2] = 0.10;   $_ [3] = 0.25;   $_ [4] = 0.50;   $_ [5] = 1.00;   }

  __END__

This will print​:

  0.01 0.05 0.1 0.25 ^^^ 1   0.01 0.05 0.1 0.25 0.5 1

Abigail

p5pRT commented 22 years ago

From @mjdominus

Changing the documentation means you are extending the language.

From a language perspective\, there is just one kind of "undef"\, &PL_sv_undef might make sense to many people on this list\, but the language Perl doesn't know any PL_sv_undef.

Unfortunately\, it does.

  print exists($a[3]) ? "1\n" : "0\n";   undef $a[3];   print exists($a[3]) ? "1\n" : "0\n";

The output is

  0   1

I still think this was a bad idea\, for exactly the reason you cite\, but there it is.

p5pRT commented 16 years ago

From renee.baecker@smart-websolutions.de

The auto-vivification doesn't create SV's at all​:

rbaecker@​www-devel-rbaecker ~/perl510/bugs $ cat 8910.pl #!/usr/bin/perl

use strict; use warnings; use Devel​::Peek;

my @​b = (undef\,undef\,undef\,undef); Dump \@​b\, 10; test(@​b); print join " -> "\, map {defined $_ ? $_ : "^^"} @​b;

print "\n\n";

my @​c; $#c = 3; Dump \@​c\, 10; test(@​c); print join " -> "\, map {defined $_ ? $_ : "^^"} @​c;

print "\n";

sub test {   $_[0] = 0;   $_[1] = 1;   $_[2] = 2;   $_[3] = 3; } rbaecker@​www-devel-rbaecker ~/perl510/bugs $ perl 8910.pl SV = RV(0x8189260) at 0x815deac   REFCNT = 1   FLAGS = (TEMP\,ROK)   RV = 0x8142c78   SV = PVAV(0x8147284) at 0x8142c78   REFCNT = 2   FLAGS = (PADBUSY\,PADMY)   IV = 0   NV = 0   ARRAY = 0x81514c0   FILL = 3   MAX = 3   ARYLEN = 0x0   FLAGS = (REAL)   Elt No. 0   SV = NULL(0x0) at 0x8142180   REFCNT = 1   FLAGS = ()   Elt No. 1   SV = NULL(0x0) at 0x814239c   REFCNT = 1   FLAGS = ()   Elt No. 2   SV = NULL(0x0) at 0x8142c48   REFCNT = 1   FLAGS = ()   Elt No. 3   SV = NULL(0x0) at 0x814209c   REFCNT = 1   FLAGS = () 0 -> 1 -> 2 -> 3

SV = RV(0x8189260) at 0x815de10   REFCNT = 1   FLAGS = (TEMP\,ROK)   RV = 0x8175b74   SV = PVAV(0x81472b0) at 0x8175b74   REFCNT = 2   FLAGS = (PADBUSY\,PADMY)   IV = 0   NV = 0   ARRAY = 0x8168798   FILL = 3   MAX = 3   ARYLEN = 0x815deac   FLAGS = (REAL)   Elt No. 0   Elt No. 1   Elt No. 2   Elt No. 3 ^^ -> ^^ -> ^^ -> ^^ rbaecker@​www-devel-rbaecker ~/perl510/bugs $

--

RenĂ©e BĂ€cker renee.baecker@​smart-websolutions.de

XING​: http​://www.xing.com/profile/Renee_Baecker Foo-Magazin​: http​://foo-magazin.de

p5pRT commented 16 years ago

From @davidnicol

On 6/11/08\, RenĂ©e BĂ€cker \renee\.baecker@​smart\-websolutions\.de wrote​:

The auto-vivification doesn't create SV's at all​: not in 5.8.8 either

p5pRT commented 8 years ago

From @dcollinsn

In blead\, it throws an error now.

  Porting/bisect.pl --start v5.10.0 --end v5.22.0 --target miniperl -Dcc='ccache gcc-6' -- ./miniperl ../8910.pl

  ce0d59fdd1c7d145efdf6bf8da56a259fed483e4 is the first bad commit   commit ce0d59fdd1c7d145efdf6bf8da56a259fed483e4   Author​: Father Chrysostomos \sprout@​cpan\.org   Date​: Tue Jul 2 13​:07​:45 2013 -0700

  [perl #7508] Use NULL for nonexistent array elems

  This commit fixes bug #7508 and provides the groundwork for fixing   several other bugs.

  Elements of @​_ are aliased to the arguments\, so that \$_[0] within   sub foo will reference the same scalar as \$x if the sub is called   as foo($x).

  &PL_sv_undef (the global read-only undef scalar returned by the   ‘undef’ operator itself) was being used to represent nonexistent   array elements. So the pattern would be broken for foo(undef)\, where   \$_[0] would vivify a new $_[0] element\, treating it as having been   nonexistent.

  This also causes other problems with constants under ithreads   (#105906) and causes a pending fix for another bug (#118691) to trig-   ger this bug.

  This commit changes the internals to use a null pointer to represent a   nonexistent element.

  This requires that Storable be changed to account for it. Also\,   IPC​::Open3 was relying on the bug. So this commit patches   both modules.

  :100644 100644 b15f6ff7b60ea28323b7bba85b7bdaae7132782f aae70bf8acfcce9fe5f71143d6039efeb8c8e2a9 M av.c   :040000 040000 fa838b22ec5547df4feb63110c10681d950bb500 384bbf0307eb46120bb7a1dee84c27f3ce5e2a4d M dist   :040000 040000 b2024904fd1708fbd395a2f5778f892a7955668c dc1480bd633862776023c8a1bee98c1238d5722a M ext   :100644 100644 d8d9322c6055073b532f540ef5466980c93181c2 92765f0f792591f1ba8e9275f80747a1651c6e3f M pad.c   :100644 100644 fd404536577cfe67f40e3effb2283e0f33e428a2 17e0f3bb5142db10ec4bb965c01e785ccbad84ce M pp.c   :100644 100644 b08643fcc14028c8f319ff9a9a3da8e064b3d144 58a30831d803eeb02cf0928c5a8b73cbda3b393f M pp_hot.c   :100644 100644 5f142a0332404b6689c4322a58207337044ec607 d207d0d951de3dddc27b0ebc61b1e0fe7b48c4b6 M regexec.c   bisect run success   That took 1492 seconds.

p5pRT commented 8 years ago

From @dcollinsn

I probably should mention\, it segfaults in that commit\, it doesn't segfault in blead. Here's what it does now​:

  dcollins@​nightshade64​:\~/toolchain/perl$ ./perl ../8910.pl   cent nickel dime ^^ ^^ dollar   Modification of a read-only value attempted at ../8910.pl line 19.

p5pRT commented 8 years ago

From @cpansprout

On Mon Jul 04 18​:02​:26 2016\, dcollinsn@​gmail.com wrote​:

In blead\, it throws an error now.

Porting/bisect.pl --start v5.10.0 --end v5.22.0 --target miniperl -Dcc='ccache gcc-6' -- ./miniperl ../8910.pl

ce0d59fdd1c7d145efdf6bf8da56a259fed483e4 is the first bad commit commit ce0d59fdd1c7d145efdf6bf8da56a259fed483e4 Author​: Father Chrysostomos \sprout@​cpan\.org Date​: Tue Jul 2 13​:07​:45 2013 -0700

[perl #7508] Use NULL for nonexistent array elems

The solution is probably to have S_pushav in pp_hot.c push a defelem rather than &PL_sv_undef if it is in flattening lvalue context. We have to check for nulls anyway.

(I have been told that defelems are too slow. This may not be the best solution.)

--

Father Chrysostomos

p5pRT commented 6 years ago

From zefram@fysh.org

Fixed in commit 6661956a23de82b41adc406200054293d6d7aded. Fix caused some CPAN breakage\, tracked as [perl #132727].

-zefram

p5pRT commented 6 years ago

From @dur-randir

On Tue\, 16 Jan 2018 11​:19​:20 -0800\, zefram@​fysh.org wrote​:

Fixed in commit 6661956a23de82b41adc406200054293d6d7aded. Fix caused some CPAN breakage\, tracked as [perl #132727].

I'd very cautiously monitor effects of this commit - as it can result in very hard to detect but measurable fall down in execution speed and increased memory consumption. And that's not mentioning new bugs. Off the top of my head\, I was able to produce the following​:

5.26.0 perl -e '{local $a[3] = 12; @​foo=@​a}; warn scalar @​a' 0 at -e line 1.

blead ./perl -e '{local $a[3] = 12; @​foo=@​a}; warn scalar @​a' 3 at -e line 1.

p5pRT commented 6 years ago

From @cpansprout

On Tue\, 16 Jan 2018 16​:22​:44 -0800\, randir wrote​:

On Tue\, 16 Jan 2018 11​:19​:20 -0800\, zefram@​fysh.org wrote​:

Fixed in commit 6661956a23de82b41adc406200054293d6d7aded. Fix caused some CPAN breakage\, tracked as [perl #132727].

I'd very cautiously monitor effects of this commit - as it can result in very hard to detect but measurable fall down in execution speed and increased memory consumption. And that's not mentioning new bugs. Off the top of my head\, I was able to produce the following​:

5.26.0 perl -e '{local $a[3] = 12; @​foo=@​a}; warn scalar @​a' 0 at -e line 1.

blead ./perl -e '{local $a[3] = 12; @​foo=@​a}; warn scalar @​a' 3 at -e line 1.

Not only that\, it also breaks use of exists and delete on arrays. I think it should be reverted.

--

Father Chrysostomos

p5pRT commented 6 years ago

From @xenu

On Tue\, 16 Jan 2018 17​:49​:18 -0800 "Father Chrysostomos via RT" \perlbug\-followup@​perl\.org wrote​:

On Tue\, 16 Jan 2018 16​:22​:44 -0800\, randir wrote​:

On Tue\, 16 Jan 2018 11​:19​:20 -0800\, zefram@​fysh.org wrote​:

Fixed in commit 6661956a23de82b41adc406200054293d6d7aded. Fix caused some CPAN breakage\, tracked as [perl #132727].

I'd very cautiously monitor effects of this commit - as it can result in very hard to detect but measurable fall down in execution speed and increased memory consumption. And that's not mentioning new bugs. Off the top of my head\, I was able to produce the following​:

5.26.0 perl -e '{local $a[3] = 12; @​foo=@​a}; warn scalar @​a' 0 at -e line 1.

blead ./perl -e '{local $a[3] = 12; @​foo=@​a}; warn scalar @​a' 3 at -e line 1.

Not only that\, it also breaks use of exists and delete on arrays. I think it should be reverted.

It's a bit off-topic\, but what's the point of exists and delete on arrays? I think they're even weirder than defined(@​array) which was deprecated and removed.

p5pRT commented 6 years ago

From zefram@fysh.org

Father Chrysostomos via RT wrote​:

Not only that\, it also breaks use of exists and delete on arrays.

Not really. delete() and exists() on array elements still work as documented; the `non-existent' status of an array element can still occur and be manipulated. The documentation for those operators doesn't claim that that state isn't affected by other operations. Indeed\, since the array enumeration that one gets from list-context @​a is an operation from the consecutive-indices model of arrays\, one should not be surprised by such side effects arising from mixing the two models.

I think it should be reverted.

We should not compromise the coherent consecutive-indices model of arrays in order to support the "strongly discouraged" sparse-index model. Indeed\, we came close to deprecating the latter\, and it only escaped that fate on the grounds that it's not in the way of anything. If it is now impeding bugfixing of the consecutive-indices mechanism\, exists() on array elements is the thing that should be sacrificed.

-zefram

p5pRT commented 6 years ago

From zefram@fysh.org

Tomasz Konojacki wrote​:

It's a bit off-topic\, but what's the point of exists and delete on arrays?

It was an attempt to make arrays serve two distinct purposes simultaneously. They allow one to use a Perl array as an associative array with natural-number keys. Each possible key is present or absent independent of all others; presence is detected with exists(). The more conventional view of arrays\, storing a sequence of items\, with indices always being an initial subsequence of the natural numbers\, always predominated\, and of course Perl has a perfectly good data type whose primary purpose is to handle sparse sets of keys. So the associative view of Perl arrays has always had only partial support\, and the consecutive-indices model has taken precedence where conflicts arise.

-zefram

p5pRT commented 6 years ago

From @cpansprout

On Tue\, 16 Jan 2018 18​:20​:42 -0800\, zefram@​fysh.org wrote​:

Father Chrysostomos via RT wrote​:

Not only that\, it also breaks use of exists and delete on arrays.

Not really. delete() and exists() on array elements still work as documented; the `non-existent' status of an array element can still occur and be manipulated. The documentation for those operators doesn't claim that that state isn't affected by other operations. Indeed\, since the array enumeration that one gets from list-context @​a is an operation from the consecutive-indices model of arrays\, one should not be surprised by such side effects arising from mixing the two models.

I think it should be reverted.

We should not compromise the coherent consecutive-indices model of arrays in order to support the "strongly discouraged" sparse-index model. Indeed\, we came close to deprecating the latter\, and it only escaped that fate on the grounds that it's not in the way of anything.

It escaped that fate on the ground that there are regular users of the feature who would have to tweak thousands of lines of code if it were removed. (I still disagree with the ‘strongly discouraged’ label.)

If it is now impeding bugfixing

If a pure rvalue operation starts having observable side effects\, then that is a bug. Introducing a new bug for the sake of fixing an existing one is not a reasonable compromise\, considering that Perl favours backward-compatibility. You also have not responded to the localization bug that Mr. Aleynikov brought up.

Flagging the op in \(@​a) is easy enough to do. Surely there aren’t too many lvalue cases that are not currently flagged as lvalues?

of the consecutive-indices mechanism\, exists() on array elements is the thing that should be sacrificed.

--

Father Chrysostomos

p5pRT commented 6 years ago

From zefram@fysh.org

Father Chrysostomos via RT wrote​:

You also have not responded to the localization bug that Mr. Aleynikov brought up.

Is it a bug? It's a change in behaviour\, but the semantics of "local" don't make clear that one behaviour or the other is correct. The effect that escapes the localisation\, namely the existence of some array elements\, isn't part of the object that is ostensibly being localised\, a higher array element. We have a problem here that an implied side effect means that the structure of the "local" expression isn't rich enough to fully describe the extent of what should be localised. It's rather akin to this preexisting behaviour that might be considered a bug​:

$ perl5.27.7 -lwe '$#a = 3; print $#a; { local $a[6] = 1; print $#a; } print $#a' 3 6 -1

I'm more inclined to call this one a bug than what my edit introduced\, because this one means that an empty localisation (with no operations performed in the scope of the localisation) has a side effect.

I see you've edited the code in commit fd77b29b3be4108adb4a74b9c274599eaf228cb3 to use magical deferred elements. There's a defect in your edit\, which you'll want to rectify​: only the part for RMAGICAL arrays is effective. Your new tests only use such arrays\, because you make your non-existent elements by assigning to $#a\, which makes the array permanently RMAGICAL.

$ perl5.27.8 -lwe '$#a = 3; print exists($a[0])||0; my @​b = @​a; print exists($a[0])||0' 0 0 $ perl5.27.8 -lwe '$a[3] = 3; print exists($a[0])||0; my @​b = @​a; print exists($a[0])||0' 0 1

I'm not a fan of the deferred-element approach. Firstly\, it means that these uses of arrays in list context suffer from [perl #132729]\, which they didn't under my version of the code. It's introducing a bug to fix another one\, though to be fair [perl #132729] needs to be addressed more generally anyway.

But even if it worked\, it means that the deferred elements get created *every* time the array is used in this kind of context\, causing a bigger performance drain than just creating the element once. This mechanism only being required in order to support the sparse array view\, it means that supporting sparse arrays is imposing a performance penalty on code that doesn't use that feature. Further reason to deprecate it.

-zefram

p5pRT commented 6 years ago

From @cpansprout

On Sat\, 20 Jan 2018 14​:44​:17 -0800\, zefram@​fysh.org wrote​:

Father Chrysostomos via RT wrote​:

You also have not responded to the localization bug that Mr. Aleynikov brought up.

Is it a bug? It's a change in behaviour\, but the semantics of "local" don't make clear that one behaviour or the other is correct. The effect that escapes the localisation\, namely the existence of some array elements\, isn't part of the object that is ostensibly being localised\, a higher array element. We have a problem here that an implied side effect means that the structure of the "local" expression isn't rich enough to fully describe the extent of what should be localised. It's rather akin to this preexisting behaviour that might be considered a bug​:

$ perl5.27.7 -lwe '$#a = 3; print $#a; { local $a[6] = 1; print $#a; } print $#a' 3 6 -1

I'm more inclined to call this one a bug than what my edit introduced\, because this one means that an empty localisation (with no operations performed in the scope of the localisation) has a side effect.

The existing behaviour is longstanding. Yes\, it could be called a bug. One could argue it either way\, so I don’t think it should be changed without a good reason. I have a good reason\, which is off-topic for this ticket\, but please let this wait until after 5.28\, because my as-yet unwritten proposal changes more than just this\, for the sake of consistency\, and may be controversial.

I see you've edited the code in commit fd77b29b3be4108adb4a74b9c274599eaf228cb3 to use magical deferred elements. There's a defect in your edit\, which you'll want to rectify​: only the part for RMAGICAL arrays is effective.

Thank you for pointing that out. I have rectified it in commit 0f3591f.

Your new tests only use such arrays\, because you make your non-existent elements by assigning to $#a\, which makes the array permanently RMAGICAL.

Hmm\, does that mean that support for tied arrays slows down all @​a access once $#a has been mentioned? Maybe we should fix that. (Again\, off-topic for this ticket.)

I'm not a fan of the deferred-element approach. Firstly\, it means that these uses of arrays in list context suffer from [perl #132729]\, which they didn't under my version of the code. It's introducing a bug to fix another one\, though to be fair [perl #132729] needs to be addressed more generally anyway.

But even if it worked\, it means that the deferred elements get created *every* time the array is used in this kind of context\, causing a bigger performance drain than just creating the element once. This mechanism only being required in order to support the sparse array view\, it means that supporting sparse arrays is imposing a performance penalty on code that doesn't use that feature. Further reason to deprecate it.

I have a fix in mind that will make it a one-time penalty and fix #132729\, at least for elephants within the array. Namely\, we could store the deferred elements in the array and change the way the magic works. I think that can be done for 5.28.

--

Father Chrysostomos

p5pRT commented 6 years ago

From @dur-randir

On Sat\, 20 Jan 2018 14​:44​:17 -0800\, zefram@​fysh.org wrote​:

Is it a bug? It's a change in behaviour\, but the semantics of "local" don't make clear that one behaviour or the other is correct.

Ten years ago this was considered a bug​:

commit 4ad10a0b60fb728d1be0a9eeb1970166a3846d38 Author​: Vincent Pit \perl@​profvince\.com Date​: Sun Dec 28 15​:08​:05 2008 +0100

  On scope end\, delete localized array elements that should not exist anymore\, so that the array recovers its previous length. Honour EXISTS and DELETE for tied arrays.

Has any implications changed since then?

p5pRT commented 6 years ago

From @cpansprout

On Sun\, 21 Jan 2018 16​:24​:55 -0800\, sprout wrote​:

On Sat\, 20 Jan 2018 14​:44​:17 -0800\, zefram@​fysh.org wrote​:

I'm not a fan of the deferred-element approach. Firstly\, it means that these uses of arrays in list context suffer from [perl #132729]\, which they didn't under my version of the code. It's introducing a bug to fix another one\, though to be fair [perl #132729] needs to be addressed more generally anyway.

But even if it worked\, it means that the deferred elements get created *every* time the array is used in this kind of context\, causing a bigger performance drain than just creating the element once. This mechanism only being required in order to support the sparse array view\, it means that supporting sparse arrays is imposing a performance penalty on code that doesn't use that feature. Further reason to deprecate it.

I have a fix in mind that will make it a one-time penalty and fix #132729\, at least for elephants within the array. Namely\, we could store the deferred elements in the array and change the way the magic works. I think that can be done for 5.28.

Please review the ‘Nonelem’ patch in the sprout/nonelem branch. It currently fixes #132729 only for the cases covered by this ticket (S_pushav)\, but it could be extended to other uses of deferred elements. However\, it would only be a partial fix for #132729\, not applying to elephants beyond the end of the array. Fixing *that* would be harder\, but doable (but probably too intrusive for 5.28). It is worth applying a partial fix?

The commit does not yet include tests for #132729 or for the memory leaks it fixes. I also have not had time yet to benchmark it.

--

Father Chrysostomos

p5pRT commented 6 years ago

From @cpansprout

On Sun\, 21 Jan 2018 21​:59​:24 -0800\, sprout wrote​:

Please review the ‘Nonelem’ patch in the sprout/nonelem branch. It currently fixes #132729 only for the cases covered by this ticket (S_pushav)\, but it could be extended to other uses of deferred elements. However\, it would only be a partial fix for #132729\, not applying to elephants beyond the end of the array. Fixing *that* would be harder\, but doable (but probably too intrusive for 5.28). It is worth applying a partial fix?

Replying to myself\, I think it is worth it\, because most Perl programmers probably do not realize that arrays can have holes in them.

I have applied my fix in the branch merged as 2a05854.

The commit does not yet include tests for #132729 or for the memory leaks it fixes.

The branch includes that.

I also have not had time yet to benchmark it.

I know my method is primitive\, but it should not matter considering the stark difference​:

$ time ./miniperl -e '$#a++; for (1..5000000) { map 1\, @​a }'

real 0m7.991s user 0m7.272s sys 0m0.703s

$ time ./newminiperl -e '$#a++; for (1..5000000) { map 1\, @​a }'

real 0m4.503s user 0m4.490s sys 0m0.006s

The first is blead before the merge (with my previous\, inefficient defelem fix). The second is after the merge.

--

Father Chrysostomos

p5pRT commented 6 years ago

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

p5pRT commented 6 years ago

From @dur-randir

Here's some comparison between a97021b9d2 (representing point right before this change) and 8c11620dca (representing blead)​:

% perf stat -r 5 -e instructions\,branches\,branch-misses\,cache-misses ./miniperl_a97021b9d2 -e 'sub foo{} $a[5] = 1; foo(@​a) for (1..100_000_000)'

Performance counter stats for './miniperl_a97021b9d2 -e sub foo{} $a[5] = 1; foo(@​a) for (1..100_000_000)' (5 runs)​:

  68\,408\,669\,959 instructions ( +- 0.00% )   13\,001\,633\,581 branches ( +- 0.00% )   49\,824 branch-misses # 0.00% of all branches ( +- 2.59% )   17\,808 cache-misses ( +- 35.84% )

  7.886012337 seconds time elapsed ( +- 0.45% )

% perf stat -r 5 -e instructions\,branches\,branch-misses\,cache-misses ./miniperl_8c11620dca -e 'sub foo{} $a[5] = 1; foo(@​a) for (1..100_000_000)'

Performance counter stats for './miniperl_8c11620dca -e sub foo{} $a[5] = 1; foo(@​a) for (1..100_000_000)' (5 runs)​:

  68\,208\,661\,954 instructions ( +- 0.00% )   13\,601\,633\,162 branches ( +- 0.00% )   50\,091 branch-misses # 0.00% of all branches ( +- 1.05% )   23\,055 cache-misses ( +- 25.61% )

  7.753697959 seconds time elapsed ( +- 0.43% )

Calling subs with empty slots takes now ~4% more branches.

% perf stat -r 5 -e instructions\,branches\,branch-misses\,cache-misses ./miniperl_a97021b9d2 -e 'sub foo{} @​a=(1)x5; foo(@​a) for (1..100_000_000)'

Performance counter stats for './miniperl_a97021b9d2 -e sub foo{} @​a=(1)x5; foo(@​a) for (1..100_000_000)' (5 runs)​:

  66\,508\,170\,242 instructions ( +- 0.00% )   12\,601\,545\,986 branches ( +- 0.00% )   55\,329 branch-misses # 0.00% of all branches ( +- 2.02% )   20\,607 cache-misses ( +- 31.11% )

  7.426998822 seconds time elapsed ( +- 0.09% )

% perf stat -r 5 -e instructions\,branches\,branch-misses\,cache-misses ./miniperl_8c11620dca -e 'sub foo{} @​a=(1)x5; foo(@​a) for (1..100_000_000)'

Performance counter stats for './miniperl_8c11620dca -e sub foo{} @​a=(1)x5; foo(@​a) for (1..100_000_000)' (5 runs)​:

  66\,308\,249\,789 instructions ( +- 0.00% )   13\,101\,560\,547 branches ( +- 0.00% )   69\,581 branch-misses # 0.00% of all branches ( +- 3.59% )   27\,929 cache-misses ( +- 17.41% )

  7.480210740 seconds time elapsed ( +- 0.26% )

Calling subs with dense array is\, again\, taking 4% more branches.

Blead is chosen here to demonstrate that nothing fundamentally changed after recent patches from Father Chrysostomos\, the main hit was done by 6661956a23 (though instructions improved a bit\, but I didn't pinpoint that to specific commit/change)​:

% perf stat -r 5 -e instructions\,branches\,branch-misses\,cache-misses ./miniperl_6661956a23 -e 'sub foo{} $a[5] = 1; foo(@​a) for (1..100_000_000)'

Performance counter stats for './miniperl_6661956a23 -e sub foo{} $a[5] = 1; foo(@​a) for (1..100_000_000)' (5 runs)​:

  68\,708\,344\,437 instructions ( +- 0.00% )   13\,601\,578\,019 branches ( +- 0.00% )   56\,758 branch-misses # 0.00% of all branches ( +- 8.07% )   17\,868 cache-misses ( +- 35.83% )

  7.620128855 seconds time elapsed ( +- 0.52% )

p5pRT commented 6 years ago

From @khwilliamson

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

With the release yesterday of Perl 5.28.0\, this and 185 other issues have been resolved.

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

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

p5pRT commented 6 years ago

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