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

Subroutine doesn't create elements. #5310

Closed
p5pRT opened this issue Apr 5, 2002 · 31 comments
Closed

Subroutine doesn't create elements. #5310

p5pRT opened this issue Apr 5, 2002 · 31 comments

Comments

@p5pRT
Copy link

p5pRT commented Apr 5, 2002

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

Searchable as RT8910$

@p5pRT
Copy link
Author

p5pRT commented Apr 5, 2002

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
Copy link
Author

p5pRT commented Apr 5, 2002

From @schwern

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

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

print map \{defined $\_ ? $\_ : "^^"\} @&#8203;arr;
mysub @&#8203;arr;
print map \{defined $\_ ? $\_ : "^^"\} @&#8203;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
Copy link
Author

p5pRT commented Apr 5, 2002

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​:
<snip>

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

print map \{defined $\_ ? $\_ : "^^"\} @&#8203;arr;
mysub @&#8203;arr;
print map \{defined $\_ ? $\_ : "^^"\} @&#8203;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
Copy link
Author

p5pRT commented Apr 5, 2002

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
Copy link
Author

p5pRT commented Apr 5, 2002

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 "@&#8203;\_"\.  There\-
   fore\, if you called a function with two arguments\, those
   would be stored in "$\_\[0\]" and "$\_\[1\]"\.  The array "@&#8203;\_" 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
Copy link
Author

p5pRT commented Apr 5, 2002

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 "@&#8203;\_"\.  There\-
   fore\, if you called a function with two arguments\, those
   would be stored in "$\_\[0\]" and "$\_\[1\]"\.  The array "@&#8203;\_" 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
Copy link
Author

p5pRT commented Apr 5, 2002

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
Copy link
Author

p5pRT commented Apr 5, 2002

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
Copy link
Author

p5pRT commented Apr 19, 2002

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
Copy link
Author

p5pRT commented Apr 19, 2002

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
Copy link
Author

p5pRT commented Jun 11, 2008

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
Copy link
Author

p5pRT commented Jun 11, 2008

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
Copy link
Author

p5pRT commented Jul 5, 2016

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

  ce0d59f is the first bad commit
  commit ce0d59f
  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
Copy link
Author

p5pRT commented Jul 5, 2016

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
Copy link
Author

p5pRT commented Jul 5, 2016

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

ce0d59f is the first bad commit
commit ce0d59f
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
Copy link
Author

p5pRT commented Jan 16, 2018

From zefram@fysh.org

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

-zefram

@p5pRT
Copy link
Author

p5pRT commented Jan 17, 2018

From @dur-randir

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

Fixed in commit 6661956. 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
Copy link
Author

p5pRT commented Jan 17, 2018

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 6661956. 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
Copy link
Author

p5pRT commented Jan 17, 2018

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 6661956. 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
Copy link
Author

p5pRT commented Jan 17, 2018

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
Copy link
Author

p5pRT commented Jan 17, 2018

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
Copy link
Author

p5pRT commented Jan 17, 2018

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
Copy link
Author

p5pRT commented Jan 20, 2018

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
fd77b29 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
Copy link
Author

p5pRT commented Jan 22, 2018

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
fd77b29 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
Copy link
Author

p5pRT commented Jan 22, 2018

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 4ad10a0
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
Copy link
Author

p5pRT commented Jan 22, 2018

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
Copy link
Author

p5pRT commented Feb 19, 2018

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
Copy link
Author

p5pRT commented Feb 19, 2018

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

@p5pRT
Copy link
Author

p5pRT commented Feb 20, 2018

From @dur-randir

Here's some comparison between a97021b (representing point right before this change) and 8c11620 (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 6661956 (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
Copy link
Author

p5pRT commented Jun 23, 2018

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
Copy link
Author

p5pRT commented Jun 23, 2018

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

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