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

while each %$ref clobbers @_ in some circumstances #13734

Closed
p5pRT opened this issue Apr 13, 2014 · 12 comments
Closed

while each %$ref clobbers @_ in some circumstances #13734

p5pRT opened this issue Apr 13, 2014 · 12 comments

Comments

@p5pRT
Copy link

p5pRT commented Apr 13, 2014

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

Searchable as RT121646$

@p5pRT
Copy link
Author

p5pRT commented Apr 13, 2014

From @demerphq

Created by @demerphq

The following should print "ok". The triggers seems to be the
condition clause in C< 1 while each %$r; >. Changing this
in simple ways, such as removing the while just calling each,
or other simple changes seem to eliminate the problem.

perl -le'
t(["foo"],
sub {
  my $got= $_[0];
  my $r={};
  1 while each %$r;
  print $_[0] ne $got ? "not " : "", "ok"
});
sub t{my ($ary,$sub)=@​_; $sub->($_) for @​$ary; }
'
not ok

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl 5.19.11:

Configured by yorton at Sun Apr 13 18:56:14 CEST 2014.

Summary of my perl5 (revision 5 version 19 subversion 11) configuration:
  Snapshot of: 0c5ea01913265b717b8615a704acd13ddde5b078
  Platform:
    osname=linux, osvers=3.8.0-19-generic, archname=x86_64-linux
    uname='linux shire 3.8.0-19-generic #30-ubuntu smp wed may 1
16:35:23 utc 2013 x86_64 x86_64 x86_64 gnulinux '
    config_args='-de -Dcc=ccache gcc -Dld=gcc
-Dprefix=/home/yorton/perl5/perlbrew/perls/perl-blead -Dusedevel
-Aeval:scriptdir=/home/yorton/perl5/perlbrew/perls/perl-blead/bin'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='ccache gcc', ccflags ='-fwrapv -fno-strict-aliasing -pipe
-fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE
-D_FILE_OFFSET_BITS=64',
    optimize='-O2',
    cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include'
    ccversion='', gccversion='4.7.3', 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='gcc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib
/usr/lib/gcc/x86_64-linux-gnu/4.7/include-fixed
/usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu
/lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
    libs=-lnsl -ldl -lm -lcrypt -lutil -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.17.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.17'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib
-fstack-protector'



@INC for perl 5.19.11:
    /home/yorton/perl5/perlbrew/perls/perl-blead/lib/site_perl/5.19.11/x86_64-linux
    /home/yorton/perl5/perlbrew/perls/perl-blead/lib/site_perl/5.19.11
    /home/yorton/perl5/perlbrew/perls/perl-blead/lib/5.19.11/x86_64-linux
    /home/yorton/perl5/perlbrew/perls/perl-blead/lib/5.19.11
    .


Environment for perl 5.19.11:
    HOME=/home/yorton
    LANG=en_GB.utf8
    LANGUAGE (unset)
    LC_ADDRESS=en_CA.UTF-8
    LC_IDENTIFICATION=en_CA.UTF-8
    LC_MEASUREMENT=en_CA.UTF-8
    LC_MONETARY=en_CA.UTF-8
    LC_NAME=en_CA.UTF-8
    LC_NUMERIC=en_CA.UTF-8
    LC_PAPER=en_CA.UTF-8
    LC_TELEPHONE=en_CA.UTF-8
    LC_TIME=en_CA.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/yorton/perl5/perlbrew/bin:/home/yorton/perl5/perlbrew/perls/perl-blead/bin:/home/yorton/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/home/yorton/bin
    PERLBREW_BASHRC_VERSION=0.67
    PERLBREW_HOME=/home/yorton/.perlbrew
    PERLBREW_MANPATH=/home/yorton/perl5/perlbrew/perls/perl-blead/man
    PERLBREW_PATH=/home/yorton/perl5/perlbrew/bin:/home/yorton/perl5/perlbrew/perls/perl-blead/bin
    PERLBREW_PERL=perl-blead
    PERLBREW_ROOT=/home/yorton/perl5/perlbrew
    PERLBREW_VERSION=0.67
    PERL_BADLANG (unset)
    SHELL=/bin/bash


-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Apr 13, 2014

From @wolfsage

Bisect (seems plausible)​:

bad - non-zero exit from ./perl -Ilib -e t(["foo"],sub { my $got=
$_[0]; my $r={1..10}; my $count; $count++ while each %$r; if ( $_[0]
eq $got ) { exit 0; } else { exit 1; }}); sub t{my ($ary,$sub)=@​_;
$sub->($_) for @​$ary; }
8ae39f6 is the first bad commit
commit 8ae39f6
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Fri May 11 20​:13​:01 2012 -0700

  Make while(each ...) imply defined($_ = ...)

  This came up in ticket #108286.

  ...

-- Matthew Horsfall (alh)

@p5pRT
Copy link
Author

p5pRT commented Apr 13, 2014

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

@p5pRT
Copy link
Author

p5pRT commented Apr 13, 2014

From @demerphq

On 13 April 2014 21​:41, Matthew Horsfall (alh) <wolfsage@​gmail.com> wrote​:

Bisect (seems plausible)​:

bad - non-zero exit from ./perl -Ilib -e t(["foo"],sub { my $got=
$_[0]; my $r={1..10}; my $count; $count++ while each %$r; if ( $_[0]
eq $got ) { exit 0; } else { exit 1; }}); sub t{my ($ary,$sub)=@​_;
$sub->($_) for @​$ary; }
8ae39f6 is the first bad commit
commit 8ae39f6
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Fri May 11 20​:13​:01 2012 -0700

Make while\(each \.\.\.\) imply defined\($\_ = \.\.\.\)

This came up in ticket \#108286\.

\.\.\.

Yeah, reverting this patch fixes the problem.

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Apr 13, 2014

From @demerphq

On 13 April 2014 22​:27, demerphq <demerphq@​gmail.com> wrote​:

On 13 April 2014 21​:41, Matthew Horsfall (alh) <wolfsage@​gmail.com> wrote​:

Bisect (seems plausible)​:

bad - non-zero exit from ./perl -Ilib -e t(["foo"],sub { my $got=
$_[0]; my $r={1..10}; my $count; $count++ while each %$r; if ( $_[0]
eq $got ) { exit 0; } else { exit 1; }}); sub t{my ($ary,$sub)=@​_;
$sub->($_) for @​$ary; }
8ae39f6 is the first bad commit
commit 8ae39f6
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Fri May 11 20​:13​:01 2012 -0700

Make while\(each \.\.\.\) imply defined\($\_ = \.\.\.\)

This came up in ticket \#108286\.

\.\.\.

Yeah, reverting this patch fixes the problem.

With that patch reverted but then rewriting the while loop to apply
the same thing​:

$ ./perl -le'
t(["foo"],
sub {
  my $got= $_[0];
  my $r={};
  1 while ($_=defined(each %$r));
  print $_[0] ne $got ? "not " : "", "ok"
});
sub t{my ($ary,$sub)=@​_; $sub->($_) for @​$ary; }
'
not ok

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Apr 13, 2014

From zefram@fysh.org

demerphq wrote​:

Yeah, reverting this patch fixes the problem.

The bug is not in that commit. All that does is introduce a
transformation that affects the test code​: "while each %$r" implicitly
becomes "while defined($_ = each %$r)". If you write out the result of
the transformation explicitly, it turns out that that code exhibits the
problem independent of the transformation process​:

$ perl5.19.10 -le 'sub p { 1 while defined($_ = each %r); print $_[0] eq "foo" ? "ok" : "BUG"; } p($_) for @​{["foo"]};'
BUG
$ perl5.16.3 -le 'sub p { 1 while defined($_ = each %r); print $_[0] eq "foo" ? "ok" : "BUG"; } p($_) for @​{["foo"]};'
BUG
$ perl5.10.1 -le 'sub p { 1 while defined($_ = each %r); print $_[0] eq "foo" ? "ok" : "BUG"; } p($_) for @​{["foo"]};'
BUG
$ perl5.6.0 -le 'sub p { 1 while defined($_ = each %r); print $_[0] eq "foo" ? "ok" : "BUG"; } p($_) for @​{["foo"]};'
BUG
$ perl5.005_03 -le 'sub p { 1 while defined($_ = each %r); print $_[0] eq "foo" ? "ok" : "BUG"; } p($_) for @​{["foo"]};'
BUG

And it turns out it's not specific to each() at all. The essential part
of the construct is the assignment to $_​:

$ perl5.19.10 -le 'sub p { $_ = 123; print $_[0] eq "foo" ? "ok" : "BUG"; } p($_) for @​{["foo"]};'
BUG

With different print statements, we can reveal the real problem​:

$ perl5.19.10 -le 'sub p { print \$_[0], " ", $_[0]; $_ = 123; print \$_[0], " ", $_[0]; } p($_) for @​{["foo"]};'
SCALAR(0x1a2e028) foo
SCALAR(0x1a2e028) 123

The problem is that the test code is using $_ in conflicting ways.
This is not a bug in Perl, just a surprising result of implicit assignment
to $_.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Apr 13, 2014

From @demerphq

On 13 April 2014 22​:56, Zefram via RT <perlbug-followup@​perl.org> wrote​:

demerphq wrote​:

Yeah, reverting this patch fixes the problem.

The bug is not in that commit. All that does is introduce a
transformation that affects the test code​: "while each %$r" implicitly
becomes "while defined($_ = each %$r)". If you write out the result of
the transformation explicitly, it turns out that that code exhibits the
problem independent of the transformation process​:

$ perl5.19.10 -le 'sub p { 1 while defined($_ = each %r); print $_[0] eq "foo" ? "ok" : "BUG"; } p($_) for @​{["foo"]};'
BUG
$ perl5.16.3 -le 'sub p { 1 while defined($_ = each %r); print $_[0] eq "foo" ? "ok" : "BUG"; } p($_) for @​{["foo"]};'
BUG
$ perl5.10.1 -le 'sub p { 1 while defined($_ = each %r); print $_[0] eq "foo" ? "ok" : "BUG"; } p($_) for @​{["foo"]};'
BUG
$ perl5.6.0 -le 'sub p { 1 while defined($_ = each %r); print $_[0] eq "foo" ? "ok" : "BUG"; } p($_) for @​{["foo"]};'
BUG
$ perl5.005_03 -le 'sub p { 1 while defined($_ = each %r); print $_[0] eq "foo" ? "ok" : "BUG"; } p($_) for @​{["foo"]};'
BUG

And it turns out it's not specific to each() at all. The essential part
of the construct is the assignment to $_​:

$ perl5.19.10 -le 'sub p { $_ = 123; print $_[0] eq "foo" ? "ok" : "BUG"; } p($_) for @​{["foo"]};'
BUG

With different print statements, we can reveal the real problem​:

$ perl5.19.10 -le 'sub p { print \$_[0], " ", $_[0]; $_ = 123; print \$_[0], " ", $_[0]; } p($_) for @​{["foo"]};'
SCALAR(0x1a2e028) foo
SCALAR(0x1a2e028) 123

The problem is that the test code is using $_ in conflicting ways.
This is not a bug in Perl, just a surprising result of implicit assignment
to $_.

That code was fine until that commit. I claim that commit was a
regression. I guess I wont go so far as to say a regression that
should be reverted, if only because it muddies the waters even more,
but it seems clear to me that had we caught this earlier in the change
cycle we would have reverted it. it worries me that the mere passing
of time makes a regression somehow "ok".

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Apr 14, 2014

From @ikegami

On Sun, Apr 13, 2014 at 5​:16 PM, demerphq <demerphq@​gmail.com> wrote​:

That code was fine until that commit. I claim that commit was a
regression.

Yes, but was it an intentional or unintentional regression? I'm having
problems finding the discussion that lead to the patch, but one could
imagine it was considered that "while (each %$r)" would not be found in
code before the commit.

@p5pRT
Copy link
Author

p5pRT commented Apr 14, 2014

From @ap

* Eric Brine <ikegami@​adaelis.com> [2014-04-14 03​:20]​:

On Sun, Apr 13, 2014 at 5​:16 PM, demerphq <demerphq@​gmail.com> wrote​:

That code was fine until that commit. I claim that commit was
a regression.

Yes, but was it an intentional or unintentional regression? I'm having
problems finding the discussion that lead to the patch, but one could
imagine it was considered that "while (each %$r)" would not be found
in code before the commit.

In RT and in the archives, respectively​:
https://rt.perl.org/Public/Bug/Display.html?id=108286#txn-1109238
http​://www.nntp.perl.org/group/perl.perl5.porters/2012/04/msg185820.html

* demerphq <demerphq@​gmail.com> [2014-04-13 23​:20]​:

That code was fine until that commit.

Until that commit, the meaning of `while (each %foo) {...}` was “execute
the loop body without telling it anything about the iteration, and do
this as many times as there are keys in the hash”.

Is that *actually* what you *wanted* the code to do?

Do you consider that way of writing it the clearest way to do it?

while each %$ref clobbers @​_ in some circumstances

It clobbers @​_ when $_ is aliased to a variable within @​_, which you did
explicitly, by putting $_ in the arguments in `$sub->($_)`. @​_ is a red
herring. If you wrote the following code in a file and ran it, you would
find instead that `readline` “clobbers” @​_​:

  use 5.010;
  t(["foo"],
  sub {
  my $got= $_[0];
  1 while <DATA>;
  say $_[0] ne $got ? "not " : "", "ok"
  });
  sub t{my ($ary,$sub)=@​_; $sub->($_) for @​$ary; }
  __END__

This is a standard trap of the pass-as-alias nature of @​_.

--
Aristotle Pagaltzis // <http​://plasmasturm.org/>

@p5pRT
Copy link
Author

p5pRT commented Apr 14, 2014

From @demerphq

On 14 April 2014 13​:29, Aristotle Pagaltzis <pagaltzis@​gmx.de> wrote​:

* Eric Brine <ikegami@​adaelis.com> [2014-04-14 03​:20]​:

On Sun, Apr 13, 2014 at 5​:16 PM, demerphq <demerphq@​gmail.com> wrote​:

That code was fine until that commit. I claim that commit was
a regression.

Yes, but was it an intentional or unintentional regression? I'm having
problems finding the discussion that lead to the patch, but one could
imagine it was considered that "while (each %$r)" would not be found
in code before the commit.

In RT and in the archives, respectively​:
https://rt.perl.org/Public/Bug/Display.html?id=108286#txn-1109238
http​://www.nntp.perl.org/group/perl.perl5.porters/2012/04/msg185820.html

* demerphq <demerphq@​gmail.com> [2014-04-13 23​:20]​:

That code was fine until that commit.

Until that commit, the meaning of `while (each %foo) {...}` was “execute
the loop body without telling it anything about the iteration, and do
this as many times as there are keys in the hash”.

Is that *actually* what you *wanted* the code to do?

Do you consider that way of writing it the clearest way to do it?

while each %$ref clobbers @​_ in some circumstances

It clobbers @​_ when $_ is aliased to a variable within @​_, which you did
explicitly, by putting $_ in the arguments in `$sub->($_)`. @​_ is a red
herring. If you wrote the following code in a file and ran it, you would
find instead that `readline` “clobbers” @​_​:

use 5\.010;
t\(\["foo"\]\,
sub \{
    my $got= $\_\[0\];
    1 while \<DATA>;
    say $\_\[0\] ne $got ? "not " : ""\, "ok"
\}\);
sub t\{my \($ary\,$sub\)=@&#8203;\_; $sub\->\($\_\) for @&#8203;$ary; \}
\_\_END\_\_

This is a standard trap of the pass-as-alias nature of @​_.

Actually no, its not. Its the exact opposite. The problem comes
because we push $_ onto the stack and not the thing it aliases.

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Dec 16, 2017

From zefram@fysh.org

This has been determined not to be a bug. This ticket should be closed.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Dec 17, 2017

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

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

No branches or pull requests

1 participant