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

List::Util attaching to the wrong $_ when used inside given/when construct #9798

Closed
p5pRT opened this issue Jul 19, 2009 · 36 comments
Closed

Comments

@p5pRT
Copy link

p5pRT commented Jul 19, 2009

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

Searchable as RT67694$

@p5pRT
Copy link
Author

p5pRT commented Jul 19, 2009

From vparseval@gmail.com

Created by vparseval@gmail.com

Please behold the following​:

  #!/usr/bin/perl -l

  use feature qw/switch/;
  use List​::Util qw/first/;

  my $val = 1;
  given ($val) {

  # my $_
 
  when (1) {
  print +(first { $_ == 10 } 10 .. 15) ? 'works inside when'
  : 'broken inside when';
  }
  }
  print +(first { $_ == 10 } 10 .. 15) ? 'works outside when'
  : 'broken outside when';
  __END__
  broken inside when
  works outside when

Note that when commenting out the lexical declaration of $_, the code behaves
as expected.

At this point, I am not sure if the problem is in List​::Util or the perl core.
List​::MoreUtil uses does the same thing as List​::Util when it comes to
assigning to $_ and dispatching the code-reference hence it's suffering from
the same malaise.

Cheers,
Tassilo

Perl Info

Flags:
    category=library
    severity=low

Site configuration information for perl 5.10.0:

Configured by Debian Project at Sun May  3 21:49:06 UTC 2009.

Summary of my perl5 (revision 5 version 10 subversion 0) configuration:
  Platform:
    osname=linux, osvers=2.6.18-xen-3.1-1-amd64, archname=x86_64-linux-gnu-thread-multi
    uname='linux nautilus 2.6.18-xen-3.1-1-amd64 #1 smp fri oct 19 23:35:59 cest 2007 x86_64 gnulinux '
    config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.10 -Darchlib=/usr/lib/perl/5.10 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.10.0 -Dsitearch=/usr/local/lib/perl/5.10.0 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib -Dlibperl=libperl.so.5.10.0 -Dd_dosuid -des'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2 -g',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include'
    ccversion='', gccversion='4.3.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='cc', ldflags =' -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib /lib64 /usr/lib64
    libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
    perllibs=-ldl -lm -lpthread -lc -lcrypt
    libc=/lib/libc-2.9.so, so=so, useshrplib=true, libperl=libperl.so.5.10.0
    gnulibc_version='2.9'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib'

Locally applied patches:
    


@INC for perl 5.10.0:
    /etc/perl
    /usr/local/lib/perl/5.10.0
    /usr/local/share/perl/5.10.0
    /usr/lib/perl5
    /usr/share/perl5
    /usr/lib/perl/5.10
    /usr/share/perl/5.10
    /usr/local/lib/site_perl
    .


Environment for perl 5.10.0:
    HOME=/home/ethan
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/ethan/bin:/home/ethan/bin:/usr/local/bin:/usr/bin:/bin:/usr/games
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Jul 21, 2009

From p5p@perl.wizbit.be

-----------------------------------------------------------------
[Please enter your report here]

Please behold the following​:

[code snipped]

Note that when commenting out the lexical declaration of $_, the code behaves
as expected.

No it doesn't. Or at least not for me. I suggest you check your test
case again.

This seem to happen because given creates a lexical $_. It also
happens when for/foreach is used with a lexical $_​:

#!/usr/bin/perl -l

use strict;
use warnings;
use feature qw/switch/;

sub s1 (&) {
  my $code = shift;
  for ("a") {
  print "\ts1​:\t \$_ = " . \$_ . " (value​: $_)";
  $code->();
  }
}

print "Given/when block​:";
my $val = 1;
given ($val) {
  when (1) {
  print "\twhen-1​:\t \$_ = " . \$_ . " (value​: $_)";
  s1 { print "\tblock​:\t \$_ = " . \$_ . " (value​: $_)"; };
  print "\twhen-2​:\t \$_ = " . \$_ . " (value​: $_)";
  }
}

print "";
print "For loop";
for (1) {
  print "\tfor-1​:\t \$_ = " . \$_ . " (value​: $_)";
  s1 { print "\tblock​:\t \$_ = " . \$_ . " (value​: $_)"; };
  print "\tfor-2​:\t \$_ = " . \$_ . " (value​: $_)";
}

print "";
print "For loop with lexical \$_";
for my $_ (1) {
  print "\tfor-1​:\t \$_ = " . \$_ . " (value​: $_)";
  s1 { print "\tblock​:\t \$_ = " . \$_ . " (value​: $_)"; };
  print "\tfor-2​:\t \$_ = " . \$_ . " (value​: $_)";
}
__END__

$ perl-5.10.0 rt-67694.pl
Given/when block​:
  when-1​: $_ = SCALAR(0x994f768) (value​: 1)
  s1​: $_ = SCALAR(0x994f478) (value​: a)
  block​: $_ = SCALAR(0x994f768) (value​: 1)
  when-2​: $_ = SCALAR(0x994f768) (value​: 1)

For loop
  for-1​: $_ = SCALAR(0x994ed08) (value​: 1)
  s1​: $_ = SCALAR(0x994f478) (value​: a)
  block​: $_ = SCALAR(0x994f478) (value​: a)
  for-2​: $_ = SCALAR(0x994ed08) (value​: 1)

For loop with lexical $_
  for-1​: $_ = SCALAR(0x994e848) (value​: 1)
  s1​: $_ = SCALAR(0x994f478) (value​: a)
  block​: $_ = SCALAR(0x994e848) (value​: 1)
  for-2​: $_ = SCALAR(0x994e848) (value​: 1)

Best regards,

Bram

@p5pRT
Copy link
Author

p5pRT commented Jul 21, 2009

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

@p5pRT
Copy link
Author

p5pRT commented Jul 21, 2009

From @rgs

2009/7/21 Bram <p5p@​perl.wizbit.be>​:

This seem to happen because given creates a lexical $_. It also happens when
for/foreach is used with a lexical $_​:

Hmm what ? No, given does not create a lexical $_ : from pp_entergiven :

  if (PL_op->op_targ == 0) {
  SV ** const defsv_p = &GvSV(PL_defgv);
  *defsv_p = newSVsv(POPs);
  SAVECLEARSV(*defsv_p);
  }
  else
  sv_setsv(PAD_SV(PL_op->op_targ), POPs);

(rewriting that with the proper macros will be nice) (not sure if
there is absolutely no bug there)

What happens, at first glance, it that List​::Util always uses the
global $_. (an "our $_" in the "first" block should solve the pb).

We provide UNDERBAR and dUNDERBAR macros to access the current $_, be
it lexical or global.

However we don't have (yet) an API to localize the current $_ as
needed in grep-like functions.

I suppose that code from pp_grepstart could be copied in List​::Util to
solve the bug.

@p5pRT
Copy link
Author

p5pRT commented Jul 21, 2009

From p5p@perl.wizbit.be

Citeren Rafael Garcia-Suarez <rgarciasuarez@​gmail.com>​:

2009/7/21 Bram <p5p@​perl.wizbit.be>​:

This seem to happen because given creates a lexical $_. It also happens when
for/foreach is used with a lexical $_​:

Hmm what ? No, given does not create a lexical $_ :

I didn't look at the code only at the docs and the output :/

from perldoc perlsyn​:
'given(EXPR) will assign the value of EXPR to $_ within the lexical
scope of the block, so it's similar to do { my $_ = EXPR; ... }'

the output shows a different reference for $_ in the when block and in
the s1 sub

from pp_entergiven :

if \(PL\_op\->op\_targ == 0\) \{
    SV \*\* const defsv\_p = &GvSV\(PL\_defgv\);
    \*defsv\_p = newSVsv\(POPs\);
    SAVECLEARSV\(\*defsv\_p\);
\}
else
    sv\_setsv\(PAD\_SV\(PL\_op\->op\_targ\)\, POPs\);

What happens, at first glance, it that List​::Util always uses the
global $_. (an "our $_" in the "first" block should solve the pb).

Then what does in create in pp_entergiven?
Doesn't it modify PL_defgv?
(I have to admit being not familiar enough with the code to really
read/understand it :( )

Adding an our $_ does solve the problem​:
With 's1 { our $_; print "\tblock​:\t \$_ = " . \$_ . " (value​: $_)";
};' the output becomes​:

Given/when block​:
  when-1​: $_ = SCALAR(0x942d7f0) (value​: 1)
  s1​: $_ = SCALAR(0x942d500) (value​: a)
  block​: $_ = SCALAR(0x942d500) (value​: a)
  when-2​: $_ = SCALAR(0x942d7f0) (value​: 1)

But why is this be nessesary?
If given() does not create a lexical $_ then shouldn't the $_ in the
block be the same as the global $_?

We provide UNDERBAR and dUNDERBAR macros to access the current $_, be
it lexical or global.

However we don't have (yet) an API to localize the current $_ as
needed in grep-like functions.

I suppose that code from pp_grepstart could be copied in List​::Util to
solve the bug.

That would only work for the XS version right?
(There is also a Pure Perl version...)

Best regards,

Bram

@p5pRT
Copy link
Author

p5pRT commented Jul 21, 2009

From @rgs

2009/7/21 Bram <p5p@​perl.wizbit.be>​:

Citeren Rafael Garcia-Suarez <rgarciasuarez@​gmail.com>​:

2009/7/21 Bram <p5p@​perl.wizbit.be>​:

This seem to happen because given creates a lexical $_. It also happens
when
for/foreach is used with a lexical $_​:

Hmm what ? No, given does not create a lexical $_ :

I didn't look at the code only at the docs and the output :/

from perldoc perlsyn​:
'given(EXPR) will assign the value of EXPR to $_ within the lexical scope of
the block, so it's similar to do { my $_ = EXPR; ... }'

the output shows a different reference for $_ in the when block and in the
s1 sub

from pp_entergiven :

   if (PL_op->op_targ == 0) {
       SV ** const defsv_p = &GvSV(PL_defgv);
       *defsv_p = newSVsv(POPs);
       SAVECLEARSV(*defsv_p);
   }
   else
       sv_setsv(PAD_SV(PL_op->op_targ), POPs);

What happens, at first glance, it that List​::Util always uses the
global $_. (an "our $_" in the "first" block should solve the pb).

Then what does in create in pp_entergiven?
Doesn't it modify PL_defgv?
(I have to admit being not familiar enough with the code to really
read/understand it :( )

Ah, you're right. The parser has this line :
switch : label GIVEN '(' remember mydefsv mexpr ')' mblock
where mydefsv is an empty rule that lexicalizes $_.
But then the code I pasted above could probably be simplified.

Adding an our $_ does solve the problem​:
With 's1 { our $_; print "\tblock​:\t \$_ = " . \$_ . " (value​: $_)"; };' the
output becomes​:

Given/when block​:
       when-1​:  $_ = SCALAR(0x942d7f0) (value​: 1)
       s1​:      $_ = SCALAR(0x942d500) (value​: a)
       block​:   $_ = SCALAR(0x942d500) (value​: a)
       when-2​:  $_ = SCALAR(0x942d7f0) (value​: 1)

But why is this be nessesary?
If given() does not create a lexical $_ then shouldn't the $_ in the block
be the same as the global $_?

We provide UNDERBAR and dUNDERBAR macros to access the current $_, be
it lexical or global.

However we don't have (yet) an API to localize the current $_ as
needed in grep-like functions.

I suppose that code from pp_grepstart could be copied in List​::Util to
solve the bug.

That would only work for the XS version right?
(There is also a Pure Perl version...)

Yes.

@p5pRT
Copy link
Author

p5pRT commented Jul 24, 2009

From @iabyn

On Tue, Jul 21, 2009 at 12​:45​:04PM +0200, Rafael Garcia-Suarez wrote​:

Ah, you're right. The parser has this line :
switch : label GIVEN '(' remember mydefsv mexpr ')' mblock
where mydefsv is an empty rule that lexicalizes $_.
But then the code I pasted above could probably be simplified.

So if I've understood this correctly, given() adds a lexical $_ to the
scope, and there's a bug in List​::Util​::first in that it doesn't work with
a lexical $_? So not a bug in given/smartmatch?

  use List​::Util qw/first/;
  print "ok outside\n" if first { $_ } 1;
  {
  my $_ = 0;
  print "ok inside\n" if first { $_ } 1;
  }

outputs​:

  ok outside

--
Technology is dominated by two types of people​: those who understand what
they do not manage, and those who manage what they do not understand.

@p5pRT
Copy link
Author

p5pRT commented Jul 24, 2009

From p5p@perl.wizbit.be

Citeren Dave Mitchell <davem@​iabyn.com>​:

On Tue, Jul 21, 2009 at 12​:45​:04PM +0200, Rafael Garcia-Suarez wrote​:

Ah, you're right. The parser has this line :
switch : label GIVEN '(' remember mydefsv mexpr ')' mblock
where mydefsv is an empty rule that lexicalizes $_.
But then the code I pasted above could probably be simplified.

So if I've understood this correctly, given() adds a lexical $_ to the
scope, and there's a bug in List​::Util​::first in that it doesn't work with
a lexical $_? So not a bug in given/smartmatch?

I'm not sure if 'bug' is the correct word.

What happens​: (example with for instead of given)

#!/usr/bin/perl -l

for my $_ (1) {
  my $c = sub { print $_ };
  $c->();
}

sub foo {
  my $code = shift;
  $_ = "abc";
  $code->();
}
__END__

$c is a closure and closes over $_.
It then passes the code reference to the function foo which modifies
the global $_ and calls the closure.

But​: the closure closes over $_ so it will always uses the value 1...

There are, as fas as I can see, two solutions​:

1) modify given() so it doesn't use a lexical $_.

2) update the documentation for given() and make it more explicit that
it uses a lexical $_ and that every block/code reference inside it
needs our $_; before using $_.

Best regards,

Bram

@p5pRT
Copy link
Author

p5pRT commented Jul 24, 2009

From @iabyn

On Fri, Jul 24, 2009 at 07​:16​:17PM +0200, Bram wrote​:

Citeren Dave Mitchell <davem@​iabyn.com>​:

On Tue, Jul 21, 2009 at 12​:45​:04PM +0200, Rafael Garcia-Suarez wrote​:

Ah, you're right. The parser has this line :
switch : label GIVEN '(' remember mydefsv mexpr ')' mblock
where mydefsv is an empty rule that lexicalizes $_.
But then the code I pasted above could probably be simplified.

So if I've understood this correctly, given() adds a lexical $_ to the
scope, and there's a bug in List​::Util​::first in that it doesn't work with
a lexical $_? So not a bug in given/smartmatch?

I'm not sure if 'bug' is the correct word.

Well,

  use List​::Util qw/first/;
  {
  my $_ = 0;
  print "ok grep \n" if grep { $_ } 1;
  print "ok first\n" if first { $_ } 1;
  }

outputs

  ok grep

So, the behaviour of first() could be described as unexpected, to say the
least. I think the correct behaviour of first() would be to to modify the
$_ in the caller's scope rather than what's currently in scope.

--
Nothing ventured, nothing lost.

@p5pRT
Copy link
Author

p5pRT commented Jul 24, 2009

From @rgs

2009/7/24 Dave Mitchell wrote​:

So if I've understood this correctly, given() adds a lexical $_ to the
scope, and there's a bug in List​::Util​::first in that it doesn't work with
a lexical $_? So not a bug in given/smartmatch?

Yes, although I'm not 100% sure that pp_given does exactly the right
thing with the pad. List​::Util​::first needs to assign to the lexical $_
in the pad if there is one.

--
The world is independent of my will.
  -- Wittgenstein, Tractatus Logico-Philosophicus, 6.373

@p5pRT
Copy link
Author

p5pRT commented Jul 24, 2009

From @rgs

2009/7/24 Rafael Garcia-Suarez <rgarciasuarez@​gmail.com>​:

2009/7/24 Dave Mitchell wrote​:

So if I've understood this correctly, given() adds a lexical $_ to the
scope, and there's a bug in List​::Util​::first in that it doesn't work with
a lexical $_? So not a bug in given/smartmatch?

Yes, although I'm not 100% sure that pp_given does exactly the right
thing with the pad. List​::Util​::first needs to assign to the lexical $_
in the pad if there is one.

Yes, the patch below solves your reduced test case, the one with grep
and first, but doesn't solve the original problem, which denotes
probably another bug in pp_given (that constructs a lexical $_ without
a padmy op.)

Graham, would you consider an improved version of the patch below for
List​::Util, to make first work with lexical $_ ? I don't think there
are other areas in List​::Util that need a similar modification.

--- a/ext/List-Util/ListUtil.xs
+++ b/ext/List-Util/ListUtil.xs
@​@​ -329,16 +329,25 @​@​ CODE​:
  I32 gimme = G_SCALAR;
  SV **args = &PL_stack_base[ax];
  CV *cv;
+ PADOFFSET padoff_du = find_rundefsvoffset();
+ bool has_global_underbar = padoff_du == NOT_IN_PAD
+ || PAD_COMPNAME_FLAGS_isOUR(padoff_du);

  if(items <= 1) {
  XSRETURN_UNDEF;
  }
  cv = sv_2cv(block, &stash, &gv, 0);
  PUSH_MULTICALL(cv);
- SAVESPTR(GvSV(PL_defgv));
+ if (has_global_underbar)
+ SAVESPTR(GvSV(PL_defgv));
+ else
+ SAVESPTR(PAD_SVl(padoff_du));

  for(index = 1 ; index < items ; index++) {
- GvSV(PL_defgv) = args[index];
+ if (has_global_underbar)
+ GvSV(PL_defgv) = args[index];
+ else
+ PAD_SVl(padoff_du) = args[index];
  MULTICALL;
  if (SvTRUE(*PL_stack_sp)) {
  POP_MULTICALL;

@p5pRT
Copy link
Author

p5pRT commented Jul 24, 2009

From @ikegami

On Fri, Jul 24, 2009 at 4​:42 PM, Rafael Garcia-Suarez <
rgarciasuarez@​gmail.com> wrote​:

2009/7/24 Dave Mitchell wrote​:

So if I've understood this correctly, given() adds a lexical $_ to the
scope, and there's a bug in List​::Util​::first in that it doesn't work
with
a lexical $_? So not a bug in given/smartmatch?

Yes, although I'm not 100% sure that pp_given does exactly the right
thing with the pad. List​::Util​::first needs to assign to the lexical $_
in the pad if there is one.

How are the Pure Perl implementation of List​::Util and countless other Perl
(&) subs suppose to do that?

@p5pRT
Copy link
Author

p5pRT commented Jul 25, 2009

From @ysth

On Fri, July 24, 2009 3​:44 pm, Eric Brine wrote​:

On Fri, Jul 24, 2009 at 4​:42 PM, Rafael Garcia-Suarez <
rgarciasuarez@​gmail.com> wrote​:

2009/7/24 Dave Mitchell wrote​:

So if I've understood this correctly, given() adds a lexical $_ to
the scope, and there's a bug in List​::Util​::first in that it doesn't
work with
a lexical $_? So not a bug in given/smartmatch?

Yes, although I'm not 100% sure that pp_given does exactly the right
thing with the pad. List​::Util​::first needs to assign to the lexical $_
in the pad if there is one.

How are the Pure Perl implementation of List​::Util and countless other
Perl (&) subs suppose to do that?

Add a reference to the proper $_ to the caller() return list?

@p5pRT
Copy link
Author

p5pRT commented Jul 25, 2009

From @gbarr

On Jul 24, 2009, at 4​:10 PM, Rafael Garcia-Suarez wrote​:

Graham, would you consider an improved version of the patch below for
List​::Util, to make first work with lexical $_ ? I don't think there
are other areas in List​::Util that need a similar modification.

Sure. first is the only place in List​::Util where $_ is used. This
patch would work for the XS version, but what about the pure Perl ?

Although should the code passed to first call a sub, that code will
not see $_, but I suspect grep has the same issue there

Graham.

--- a/ext/List-Util/ListUtil.xs
+++ b/ext/List-Util/ListUtil.xs
@​@​ -329,16 +329,25 @​@​ CODE​:
I32 gimme = G_SCALAR;
SV **args = &PL_stack_base[ax];
CV *cv;
+ PADOFFSET padoff_du = find_rundefsvoffset();
+ bool has_global_underbar = padoff_du == NOT_IN_PAD
+ || PAD_COMPNAME_FLAGS_isOUR(padoff_du);

if\(items \<= 1\) \{
   XSRETURN\_UNDEF;
\}
cv = sv\_2cv\(block\, &stash\, &gv\, 0\);
PUSH\_MULTICALL\(cv\);

- SAVESPTR(GvSV(PL_defgv));
+ if (has_global_underbar)
+ SAVESPTR(GvSV(PL_defgv));
+ else
+ SAVESPTR(PAD_SVl(padoff_du));

for\(index = 1 ; index \< items ; index\+\+\) \{

- GvSV(PL_defgv) = args[index];
+ if (has_global_underbar)
+ GvSV(PL_defgv) = args[index];
+ else
+ PAD_SVl(padoff_du) = args[index];
MULTICALL;
if (SvTRUE(*PL_stack_sp)) {
POP_MULTICALL;

@p5pRT
Copy link
Author

p5pRT commented Jul 25, 2009

From @ysth

On Fri, July 24, 2009 5​:28 pm, Yitzchak Scott-Thoennes wrote​:

On Fri, July 24, 2009 3​:44 pm, Eric Brine wrote​:

On Fri, Jul 24, 2009 at 4​:42 PM, Rafael Garcia-Suarez wrote​:

Yes, although I'm not 100% sure that pp_given does exactly the right
thing with the pad. List​::Util​::first needs to assign to the lexical
$_ in the pad if there is one.

How are the Pure Perl implementation of List​::Util and countless other
Perl (&) subs suppose to do that?

Add a reference to the proper $_ to the caller() return list?

Err, never mind, that wouldn't tell you what $_ was in scope of the sub if
the sub wasn't in the caller's scope.

@p5pRT
Copy link
Author

p5pRT commented Dec 10, 2011

From fany@cpan.org

Created by fany@cpan.org

When I use a function which expects a codeblock as first
argument, like List​::Util​::first(), within a "given" block,
then $_ within that codeblock does not work properly, but
keeps the value of the $_ from the "given" block.

Or, in other words, the test script below will output the
following​:

  1 - 2 - 3 - grep OK!
  1 - first OK!
  1 - fpp OK!
  1 - 2 - 3 - grep within given OK!
  0 - 0 - 0 -
  0 - 0 - 0 -

Kind regards,
fany

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

#!/usr/bin/perl -w

use 5.01;
use strict;
use List​::Util qw(first);

sub fpp(&@​) {
  my $sub = shift;
  $sub->($_) and return 1 for @​_;
  '';
}

my @​array = 1 .. 3;

my $where = '';
print "grep$where OK!" if grep { print "$_ - "; $_ } @​array;
print "\n";
print "first$where OK!" if first { print "$_ - "; $_ } @​array;
print "\n";
print "fpp$where OK!" if fpp { print "$_ - "; $_ } @​array;
print "\n";

$where = ' within given';
given (0) {
  default {
  print "grep$where OK!" if grep { print "$_ - "; $_ } @​array;
  print "\n";
  print "first$where OK!" if first { print "$_ - "; $_ } @​array;
  print "\n";
  print "fpp$where OK!" if fpp { print "$_ - "; $_ } @​array;
  print "\n";
  }
}

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl 5.14.2:

Configured by fany at Mon Oct  3 11:31:06 CEST 2011.

Summary of my perl5 (revision 5 version 14 subversion 2) configuration:
   
  Platform:
    osname=linux, osvers=2.6.37.6-0.5-desktop, archname=x86_64-linux
    uname='linux homer 2.6.37.6-0.5-desktop #1 smp preempt 2011-04-25 21:48:33 +0200 x86_64 x86_64 x86_64 gnulinux '
    config_args=''
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2',
    cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.5.1 20101208 [gcc-4_5-branch revision 167585]', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib/../lib64 /usr/lib/../lib64 /lib /usr/lib /lib64 /usr/lib64 /usr/local/lib64
    libs=-lnsl -ldb -ldl -lm -lcrypt -lutil -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
    libc=/lib/libc-2.11.3.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.11.3'
  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'

Locally applied patches:
    


@INC for perl 5.14.2:
    /usr/local/lib/perl5/site_perl/5.14.2/x86_64-linux
    /usr/local/lib/perl5/site_perl/5.14.2
    /usr/local/lib/perl5/5.14.2/x86_64-linux
    /usr/local/lib/perl5/5.14.2
    /etc/perl
    .


Environment for perl 5.14.2:
    HOME=/home/fany
    LANG=de_DE.UTF-8
    LANGUAGE=
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/fany/bin:/usr/local/bin:/usr/bin:/bin:/usr/bin/X11:/usr/X11R6/bin:/usr/games:/usr/lib64/jvm/jre/bin:/usr/sbin:/usr/sbin:/usr/sbin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Dec 10, 2011

From @cpansprout

On Sat Dec 10 09​:55​:36 2011, fany@​cpan.org wrote​:

This is a bug report for perl from fany@​cpan.org,
generated with the help of perlbug 1.39 running under perl 5.14.2.

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

When I use a function which expects a codeblock as first
argument, like List​::Util​::first(), within a "given" block,
then $_ within that codeblock does not work properly, but
keeps the value of the $_ from the "given" block.

Or, in other words, the test script below will output the
following​:

1 \- 2 \- 3 \- grep OK\!
1 \- first OK\!
1 \- fpp OK\!
1 \- 2 \- 3 \- grep within given OK\!
0 \- 0 \- 0 \-
0 \- 0 \- 0 \-

Kind regards,
fany

This is a known issue, #67694. given does an implicit ‘my $_’. See
also ticket #90018 and ticket #53186 (marked as resolved, even though it
isn’t).

RJBS​: Would you be adverse to changing ‘given’ to alias $_ properly, or
is it too close to 5.16 at this stage?

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Dec 10, 2011

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

@p5pRT
Copy link
Author

p5pRT commented Dec 10, 2011

From @cpansprout

On Sat Dec 10 11​:51​:52 2011, sprout wrote​:

On Sat Dec 10 09​:55​:36 2011, fany@​cpan.org wrote​:

This is a bug report for perl from fany@​cpan.org,
generated with the help of perlbug 1.39 running under perl 5.14.2.

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

When I use a function which expects a codeblock as first
argument, like List​::Util​::first(), within a "given" block,
then $_ within that codeblock does not work properly, but
keeps the value of the $_ from the "given" block.

Or, in other words, the test script below will output the
following​:

1 \- 2 \- 3 \- grep OK\!
1 \- first OK\!
1 \- fpp OK\!
1 \- 2 \- 3 \- grep within given OK\!
0 \- 0 \- 0 \-
0 \- 0 \- 0 \-

Kind regards,
fany

This is a known issue, #67694. given does an implicit ‘my $_’. See
also ticket #90018 and ticket #53186 (marked as resolved, even though it
isn’t).

BTW, the easiest workaround is to use ‘for’ instead of ‘given’. You can
still use ‘when’ inside ‘for’.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Dec 10, 2011

From [Unknown Contact. See original ticket]

On Sat Dec 10 11​:51​:52 2011, sprout wrote​:

On Sat Dec 10 09​:55​:36 2011, fany@​cpan.org wrote​:

This is a bug report for perl from fany@​cpan.org,
generated with the help of perlbug 1.39 running under perl 5.14.2.

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

When I use a function which expects a codeblock as first
argument, like List​::Util​::first(), within a "given" block,
then $_ within that codeblock does not work properly, but
keeps the value of the $_ from the "given" block.

Or, in other words, the test script below will output the
following​:

1 \- 2 \- 3 \- grep OK\!
1 \- first OK\!
1 \- fpp OK\!
1 \- 2 \- 3 \- grep within given OK\!
0 \- 0 \- 0 \-
0 \- 0 \- 0 \-

Kind regards,
fany

This is a known issue, #67694. given does an implicit ‘my $_’. See
also ticket #90018 and ticket #53186 (marked as resolved, even though it
isn’t).

BTW, the easiest workaround is to use ‘for’ instead of ‘given’. You can
still use ‘when’ inside ‘for’.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Dec 10, 2011

From tchrist@perl.com

"Father Chrysostomos via RT" <perlbug-followup@​perl.org> wrote
  on Sat, 10 Dec 2011 11​:51​:53 PST​:

This is a known issue, #67694. given does an implicit ‘my $_’. See
also ticket #90018 and ticket #53186 (marked as resolved, even though
it isn’t).

RJBS​: Would you be adverse to changing ‘given’ to alias $_ properly,
or is it too close to 5.16 at this stage?

I've always felt this to be a bug, and would like to see some way
to get the more expected/normal behavior. I just don't know how
to make the omelette without breaking any eggs (read, existing code).

--tom

@p5pRT
Copy link
Author

p5pRT commented Dec 10, 2011

From @Leont

On Sat, Dec 10, 2011 at 9​:04 PM, Tom Christiansen <tchrist@​perl.com> wrote​:

I've always felt this to be a bug, and would like to see some way
to get the more expected/normal behavior.  I just don't know how
to make the omelette without breaking any eggs (read, existing code).

I'm afraid the only solution is «use 5.016;» :-|

Leon

@p5pRT
Copy link
Author

p5pRT commented Dec 10, 2011

From @cpansprout

On Sat Dec 10 12​:04​:57 2011, tom christiansen wrote​:

"Father Chrysostomos via RT" <perlbug-followup@​perl.org> wrote
on Sat, 10 Dec 2011 11​:51​:53 PST​:

This is a known issue, #67694. given does an implicit ‘my $_’. See
also ticket #90018 and ticket #53186 (marked as resolved, even though
it isn’t).

RJBS​: Would you be adverse to changing ‘given’ to alias $_ properly,
or is it too close to 5.16 at this stage?

I've always felt this to be a bug, and would like to see some way
to get the more expected/normal behavior. I just don't know how
to make the omelette without breaking any eggs (read, existing code).

Use powdered eggs. :-)

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Dec 10, 2011

From @cpansprout

On Sat Dec 10 12​:10​:19 2011, LeonT wrote​:

On Sat, Dec 10, 2011 at 9​:04 PM, Tom Christiansen <tchrist@​perl.com>
wrote​:

I've always felt this to be a bug, and would like to see some way
to get the more expected/normal behavior. I just don't know how
to make the omelette without breaking any eggs (read, existing code).

I'm afraid the only solution is «use 5.016;» :-|

There is this comment in feature.pm​:

# TODO​:
# - think about versioned features (use feature switch => 2)

I think that approach would cause too much confusion. feature.pm deals
with aspects of the perl core itself, so having multiple orthogonal sets
of version numbers for the perl core (as opposed to modules included
with the core) would be undesirable. I propose we tack the perl version
number on the end and make it a new named feature​: ‘switch5.16’. It may
not be pretty, but it follows the same format of the number that version
bundles use, and the meaning is immediately apparent.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Dec 10, 2011

From @rjbs

* Father Chrysostomos via RT <perlbug-followup@​perl.org> [2011-12-10T14​:51​:53]

RJBS​: Would you be adverse to changing ‘given’ to alias $_ properly, or
is it too close to 5.16 at this stage?

I agree, *strenuously* that 'given' is a big problem on several fronts, and the
lexical $_ is certainly one of the more widely-felt pains.

I think fixing it via something like :5.16 containing given5_16 is not nuts,
but I've got to say​: I think we should be considering deprecating given for
straight-up removal.

In the meantime, would we want to consider warning on closing over lexical $_
inside a given? Does anybody ever do this on purpose?

This email did not get a lot of deep thought. I composed it during a brief
break in tree-trimming.

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented Dec 10, 2011

From @doy

On Sat, Dec 10, 2011 at 06​:41​:41PM -0500, Ricardo Signes wrote​:

* Father Chrysostomos via RT <perlbug-followup@​perl.org> [2011-12-10T14​:51​:53]

RJBS​: Would you be adverse to changing ‘given’ to alias $_ properly, or
is it too close to 5.16 at this stage?

I agree, *strenuously* that 'given' is a big problem on several fronts, and the
lexical $_ is certainly one of the more widely-felt pains.

I think fixing it via something like :5.16 containing given5_16 is not nuts,
but I've got to say​: I think we should be considering deprecating given for
straight-up removal.

+1. I also don't think it's worth trying to reintroduce new semantics
for given without also fixing the other issues with it (like
smartmatch).

In the meantime, would we want to consider warning on closing over lexical $_
inside a given? Does anybody ever do this on purpose?

Probably +1, although I haven't given it much thought yet.

-doy

@p5pRT
Copy link
Author

p5pRT commented Dec 11, 2011

From @cpansprout

On Sat Dec 10 15​:42​:18 2011, perl.p5p@​rjbs.manxome.org wrote​:

* Father Chrysostomos via RT <perlbug-followup@​perl.org> [2011-12-
10T14​:51​:53]

RJBS​: Would you be adverse to changing ‘given’ to alias $_ properly,
or
is it too close to 5.16 at this stage?

I agree, *strenuously* that 'given' is a big problem on several
fronts, and the
lexical $_ is certainly one of the more widely-felt pains.

I think fixing it via something like :5.16 containing given5_16 is not
nuts,
but I've got to say​: I think we should be considering deprecating
given for
straight-up removal.

From a maintenance standpoint, +1000! But I am loath to break anyone’s
code. On the other hand, it has only been a short while since it was
introduced, and almost everyone who has tried it has run into problems.
So I won’t get in the way of its removal.

In the meantime, would we want to consider warning on closing over
lexical $_
inside a given?

Absolutely.

Does anybody ever do this on purpose?

I doubt it.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Dec 11, 2011

From @xdg

On Sat, Dec 10, 2011 at 6​:41 PM, Ricardo Signes
<perl.p5p@​rjbs.manxome.org> wrote​:

I agree, *strenuously* that 'given' is a big problem on several fronts, and the
lexical $_ is certainly one of the more widely-felt pains.

I think fixing it via something like :5.16 containing given5_16 is not nuts,
but I've got to say​: I think we should be considering deprecating given for
straight-up removal.

I'm supportive only if we replaced it with a decent switch construct
or deprecated just the broken parts of it. It doesn't have to be a
"smart" switch -- just a switch against scalars would be fine. (Nit
-- I suppose the challenge is numeric or string matching -- maybe a
"numeric $scalar" or "string $scalar" keyword like "scalar" for that
context?) For anything "smart", we can/should use if/elsif.

In the meantime, would we want to consider warning on closing over lexical $_
inside a given?  Does anybody ever do this on purpose?

A warning sounds reasonable and anyone doing it on purpose can turn
off the warning.

-- David

@p5pRT
Copy link
Author

p5pRT commented Dec 11, 2011

From chris@prather.org

I missed the list (my apologies David).

-Chris

---------- Forwarded message ----------
From​: Chris Prather <chris@​prather.org>
Date​: Sat, Dec 10, 2011 at 10​:13 PM
Subject​: Re​: [perl #105850] $_ does not work as expected within "given" block
To​: David Golden <xdaveg@​gmail.com>

On Sat, Dec 10, 2011 at 7​:52 PM, David Golden <xdaveg@​gmail.com> wrote​:

On Sat, Dec 10, 2011 at 6​:41 PM, Ricardo Signes
<perl.p5p@​rjbs.manxome.org> wrote​:

I agree, *strenuously* that 'given' is a big problem on several fronts, and the
lexical $_ is certainly one of the more widely-felt pains.

I think fixing it via something like :5.16 containing given5_16 is not nuts,
but I've got to say​: I think we should be considering deprecating given for
straight-up removal.

I'm supportive only if we replaced it with a decent switch construct
or deprecated just the broken parts of it.  It doesn't have to be a
"smart" switch -- just a switch against scalars would be fine.  (Nit
-- I suppose the challenge is numeric or string matching -- maybe a
"numeric $scalar" or "string $scalar" keyword like "scalar" for that
context?)  For anything "smart", we can/should use if/elsif.

Switch and Smart Matching are not equivalent.

I'm guessing (hoping?) that Ric is simply talking about deprecating
`given` and not `when`. Which would leave the more Perl5-ish​:

for ($var) {
  when (defined) { ... }
  when (m/foo/) { ... }
  when ($_ eq 'bar') { ... }
}

That is all you need for switch is a topicalizer (for) and a test that
exits the block on success (when). These cover 99% of the use cases
I've seen in the wild for given/when.

Fact is that I try very hard to only use given/when in cases where I
know it fits within one of the special cases (see above). Removing
given() wouldn't be that painful to most of the code I'm responsible
for, nor honestly would removing SmartMatch. Removing when() however
would make me cry.

In the meantime, would we want to consider warning on closing over lexical $_
inside a given?  Does anybody ever do this on purpose?

A warning sounds reasonable and anyone doing it on purpose can turn
off the warning.

no warnings "given"; # none 'expected';

:)

-Chris

@p5pRT
Copy link
Author

p5pRT commented Dec 11, 2011

From @Leont

On Sun, Dec 11, 2011 at 12​:41 AM, Ricardo Signes
<perl.p5p@​rjbs.manxome.org> wrote​:

I agree, *strenuously* that 'given' is a big problem on several fronts, and the
lexical $_ is certainly one of the more widely-felt pains.

I think fixing it via something like :5.16 containing given5_16 is not nuts,
but I've got to say​: I think we should be considering deprecating given for
straight-up removal.

In the meantime, would we want to consider warning on closing over lexical $_
inside a given?  Does anybody ever do this on purpose?

It's possible to do the right thing AFAIK, it's just that no one does
it. Ironically, it's quite easy and sane to do in XS, but really hard
in pure perl. I'd like a way to flag a function as being able to
handle lexical $_ if we are going to make this warn.

Leon

@p5pRT
Copy link
Author

p5pRT commented Dec 11, 2011

From @cpansprout

On Sun Dec 11 06​:05​:24 2011, LeonT wrote​:

On Sun, Dec 11, 2011 at 12​:41 AM, Ricardo Signes
<perl.p5p@​rjbs.manxome.org> wrote​:

I agree, *strenuously* that 'given' is a big problem on several
fronts, and the
lexical $_ is certainly one of the more widely-felt pains.

I think fixing it via something like :5.16 containing given5_16 is
not nuts,
but I've got to say​: I think we should be considering deprecating
given for
straight-up removal.

In the meantime, would we want to consider warning on closing over
lexical $_
inside a given? �Does anybody ever do this on purpose?

It's possible to do the right thing AFAIK, it's just that no one does
it. Ironically, it's quite easy and sane to do in XS, but really hard
in pure perl. I'd like a way to flag a function as being able to
handle lexical $_ if we are going to make this warn.

What’s so ironic is that, if called functions can see it, $_ isn’t
really lexical any more, is it?

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Dec 12, 2011

From @rjbs

* Father Chrysostomos via RT <perlbug-followup@​perl.org> [2011-12-11T09​:48​:24]

On Sun Dec 11 06​:05​:24 2011, LeonT wrote​:

It's possible to do the right thing AFAIK, it's just that no one does
it. Ironically, it's quite easy and sane to do in XS, but really hard
in pure perl. I'd like a way to flag a function as being able to
handle lexical $_ if we are going to make this warn.

  given ($x) {
  no warnings 'closure';
  my $sub = sub { warn $_ };
  }

Something along those lines?

What’s so ironic is that, if called functions can see it, $_ isn’t
really lexical any more, is it?

Well, they're seeing it because they closed over it, right? That's still
lexical.

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented Dec 12, 2011

From @cpansprout

On Mon Dec 12 12​:20​:53 2011, perl.p5p@​rjbs.manxome.org wrote​:

* Father Chrysostomos via RT <perlbug-followup@​perl.org> [2011-12-
11T09​:48​:24]

On Sun Dec 11 06​:05​:24 2011, LeonT wrote​:

It's possible to do the right thing AFAIK, it's just that no one
does
it. Ironically, it's quite easy and sane to do in XS, but really
hard
in pure perl. I'd like a way to flag a function as being able to
handle lexical $_ if we are going to make this warn.

given ($x) {
no warnings 'closure';
my $sub = sub { warn $_ };
}

Something along those lines?

I think Leon Timmermans meant that functions like Scalar​::Util​::first
should be able to suppress that warning in their *arguments*.

given ($x) {
  when(first { ... $_ ... }); # no warning
  when(other_func { ... $_ ... }); # warning
}

What’s so ironic is that, if called functions can see it, $_ isn’t
really lexical any more, is it?

Well, they're seeing it because they closed over it, right? That's
still
lexical.

I’m referring to the ability of XS modules (like Scalar​::Util​::first) to
access the lexical $_ of the caller. Scalar​::Util​::first doesn’t
actually do that, so it’s not a good example, but there is an API for it.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Dec 18, 2011

From @Leont

On Mon, Dec 12, 2011 at 10​:03 PM, Father Chrysostomos via RT
<perlbug-followup@​perl.org> wrote​:

I think Leon Timmermans meant that functions like Scalar​::Util​::first
should be able to suppress that warning in their *arguments*.

given ($x) {
   when(first { ... $_ ... }); # no warning
   when(other_func { ... $_ ... }); # warning
}

Exactly.

I’m referring to the ability of XS modules (like Scalar​::Util​::first) to
access the lexical $_ of the caller.  Scalar​::Util​::first doesn’t
actually do that, so it’s not a good example, but there is an API for it.

Though I'm not sure any module uses it currently TBH, ppport and some
unicode symbols generate too much noise for grep.cpan.me to give clear
results but I didn't see any positives so far.

Leon

@p5pRT
Copy link
Author

p5pRT commented Aug 19, 2012

From @cpansprout

Fixed by b5a6481.

@p5pRT
Copy link
Author

p5pRT commented Aug 19, 2012

@cpansprout - Status changed from 'open' 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