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

Wrong warning 'Variable "$_" is not available' in closure #11499

Closed
p5pRT opened this issue Jul 13, 2011 · 7 comments
Closed

Wrong warning 'Variable "$_" is not available' in closure #11499

p5pRT opened this issue Jul 13, 2011 · 7 comments

Comments

@p5pRT
Copy link

@p5pRT p5pRT commented Jul 13, 2011

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

Searchable as RT94682$

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jul 13, 2011

From qallpaqa@gmail.com

Created by qallpaqa@gmail.com

I wonder if it is not valid to give the warning of 'Variable "$_" is
not available' when running the following code.

I can't figure out whether it's a proper warning or just a bug even
after reading the perldiag.
Let me know the reason if it is proper.

It doesn't take place on Perl 5.12.
My platform is Win Vista x64 ActivePerl 5.14.1.1401 x64.

#!perl
use 5.14.0;
use warnings;

sub main { # take place just in main()
  given(1){ # take place just in "given"
  default {
  func(sub{ my $s='foo';$s =~ m/./; say $s });
  # doesn't take place without "m//". Also does with "s///" and "say"
  # my $_; doesn't with this declaration
  # func(sub{ my $_; my $s='foo';$s =~ m/./; say $s });
  }
  }
}

sub func {
  shift->();
}

main();
__END__

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.14.1:

Configured by sshd_server at Thu Jun 16 17:23:59 2011.

Summary of my perl5 (revision 5 version 14 subversion 1) configuration:

  Platform:
    osname=MSWin32, osvers=5.2, archname=MSWin32-x64-multi-thread
    uname=''
    config_args='undef'
    hint=recommended, useposix=true, d_sigaction=undef
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cl', ccflags ='-nologo -GF -W3 -MD -Zi -DNDEBUG -Ox -GL
-fp:precise -DWIN32 -D_CONSOLE -DNO_STRICT -DWIN64 -DCONSERVATIVE
-DPERL_TEXTMODE_SCRIPTS -DUSE_SITECUSTOMIZE -DPERL_IMPLICIT_CONTEXT
-DPERL_IMPLICIT_SYS -DUSE_PERLIO',
    optimize='-MD -Zi -DNDEBUG -Ox -GL -fp:precise',
    cppflags='-DWIN32'
    ccversion='14.00.40310.41', gccversion='', gccosandvers=''
    intsize=4, longsize=4, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=8
    ivtype='__int64', ivsize=8, nvtype='double', nvsize=8,
Off_t='__int64', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='link', ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf
-ltcg  -libpath:"C:\Perl\x64\514\lib\CORE"  -machine:AMD64'
    libpth=\lib
    libs=oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib
comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib
netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib  version.lib
odbc32.lib odbccp32.lib comctl32.lib bufferoverflowU.lib msvcrt.lib
    perllibs=oldnames.lib kernel32.lib user32.lib gdi32.lib
winspool.lib  comdlg32.lib advapi32.lib shell32.lib ole32.lib
oleaut32.lib  netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib
version.lib odbc32.lib odbccp32.lib comctl32.lib bufferoverflowU.lib
msvcrt.lib
    libc=msvcrt.lib, so=dll, useshrplib=true, libperl=perl514.lib
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
    cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug
-opt:ref,icf -ltcg  -libpath:"C:\Perl\x64\514\lib\CORE"
-machine:AMD64'

Locally applied patches:
    ACTIVEPERL_LOCAL_PATCHES_ENTRY


@INC for perl 5.14.1:
    C:/Perl/x64/514/site/lib
    C:/Perl/x64/514/lib
    .


Environment for perl 5.14.1:
    HOME (unset)
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=C:\Perl\x64\514\site\bin;C:\Perl\x64\514\bin;
    PERL_BADLANG (unset)
    SHELL (unset)

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jul 13, 2011

From @cpansprout

On Tue Jul 12 21​:24​:27 2011, qallpaqa@​gmail.com wrote​:

I wonder if it is not valid to give the warning of 'Variable "$_" is
not available' when running the following code.

I can't figure out whether it's a proper warning or just a bug even
after reading the perldiag.
Let me know the reason if it is proper.

It doesn't take place on Perl 5.12.
My platform is Win Vista x64 ActivePerl 5.14.1.1401 x64.

#!perl
use 5.14.0;
use warnings;

sub main { # take place just in main()
given(1){ # take place just in "given"
default {
func(sub{ my $s='foo';$s =~ m/./; say $s });
# doesn't take place without "m//". Also does with "s///"
and "say"
# my $_; doesn't with this declaration
# func(sub{ my $_; my $s='foo';$s =~ m/./; say $s });
}
}
}

sub func {
shift->();
}

main();
__END__

I believe thatâ��s a genuine bug. It was caused by this commit​:

commit 05d04d9
Author​: Nicholas Clark <nick@​ccl4.org>
Date​: Thu Feb 25 21​:35​:39 2010 +0000

  Don't clone the contents of lexicals in pads.
 
  This stops the values of lexicals in active stack frames in the
parent leaking
  into the lexicals in the child thread.
 
  With an exception for lexicals with a reference count of > 1, to
cope with the
  implementation of ?{{ ... }} blocks in regexps. :-(

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jul 13, 2011

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

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Sep 6, 2011

From @iabyn

On Tue, Jul 12, 2011 at 10​:58​:59PM -0700, Father Chrysostomos via RT wrote​:

On Tue Jul 12 21​:24​:27 2011, qallpaqa@​gmail.com wrote​:

I wonder if it is not valid to give the warning of 'Variable "$_" is
not available' when running the following code.

I can't figure out whether it's a proper warning or just a bug even
after reading the perldiag.
Let me know the reason if it is proper.

It doesn't take place on Perl 5.12.
My platform is Win Vista x64 ActivePerl 5.14.1.1401 x64.

#!perl
use 5.14.0;
use warnings;

sub main { # take place just in main()
given(1){ # take place just in "given"
default {
func(sub{ my $s='foo';$s =~ m/./; say $s });
# doesn't take place without "m//". Also does with "s///"
and "say"
# my $_; doesn't with this declaration
# func(sub{ my $_; my $s='foo';$s =~ m/./; say $s });
}
}
}

sub func {
shift->();
}

main();
__END__

I believe thatâ��s a genuine bug. It was caused by this commit​:

commit 05d04d9
Author​: Nicholas Clark <nick@​ccl4.org>
Date​: Thu Feb 25 21​:35​:39 2010 +0000

Don't clone the contents of lexicals in pads\.

This stops the values of lexicals in active stack frames in the

parent leaking
into the lexicals in the child thread.

With an exception for lexicals with a reference count of > 1\, to

cope with the
implementation of ?{{ ... }} blocks in regexps. :-(

Odd, that commit only affected the pad duping code, which would only be
called on thread creation, which isn't happening here. So I think it
may be a red herring.

I think I've fixed the real issue with the following commit​:

commit 87e4a53
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Tue Sep 6 12​:21​:21 2011 +0100

  RT #4682​: given() didn't scope $_ correctly
 
  given(expr) {...} behaves similarly to { my $_ = expr; ...},
  except that, prior to this commit, it wasn't doing the SAVECLEARSV()
  that pp_padsv would do. This meant that $_ was still marked as stale
  while in scope, and wasn't getting cleared at the end of scope.

There is a second issue that is brought up by the original bug report,
and that is that $a =~ /foo/ closes over any outer lexical $_, even though
it doesn't use $_. This is because when /foo/ is compiled, it initially
assumes that it wants to use $_ and so closes over it (if there's a lexical
version in scope); only later does it see the =~ bind operator, and
undo that assumption (by setting OPf_STACKED). By then the closure
has already been set up (a FAKE entry in the pad).

I don't see a simple way to fix that.

--
I before E. Except when it isn't.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Sep 6, 2011

@iabyn - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this Sep 6, 2011
@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 10, 2012

From @nwc10

On Tue, Sep 06, 2011 at 12​:29​:00PM +0100, Dave Mitchell wrote​:

There is a second issue that is brought up by the original bug report,
and that is that $a =~ /foo/ closes over any outer lexical $_, even though
it doesn't use $_. This is because when /foo/ is compiled, it initially
assumes that it wants to use $_ and so closes over it (if there's a lexical
version in scope); only later does it see the =~ bind operator, and
undo that assumption (by setting OPf_STACKED). By then the closure
has already been set up (a FAKE entry in the pad).

I don't see a simple way to fix that.

Meaning that the third section of this code is buggy?

$ cat ../test/94682.pl
#!/usr/bin/perl -w

sub DESTROY {
  print "DESTROY for $_[0]\n";
}

sub should_close {
  my $_ = shift;
  return sub {
  print "I'm really in a closure with $_\n";
  }
}

sub should_not_close {
  my $_ = shift;
  return sub {
  print "I'm not a closure\n";
  }
}

sub buggy {
  my $_ = shift;
  return sub {
  my $m = "I'm not to be a closure\n";
  $m =~ s/not/not meant/;
  print $m;
  }
}

print "Should close\n";
my $s = should_close(bless []);
print "(object should live)\n";
$s->();
undef $s;
print "\n";

print "Should not close\n";
$s = should_not_close(bless []);
print "(object should be gone)\n";
$s->();
undef $s;
print "\n";

print "Should not close either\n";
$s = buggy(bless []);
print "(object should be gone)\n";
$s->();
undef $s;

__END__
$ ./perl -Ilib ../test/94682.pl
Should close
(object should live)
I'm really in a closure with main=ARRAY(0x100802eb8)
DESTROY for main=ARRAY(0x100802eb8)

Should not close
DESTROY for main=ARRAY(0x1008114a8)
(object should be gone)
I'm not a closure

Should not close either
(object should be gone)
I'm not meant to be a closure
DESTROY for main=ARRAY(0x1008114c0)

In that the sub buggy() should not return a closure, hence the third
paragraph of output should be the same as the second, not the first, as
currently is?

(and this should become a TODO test somewhere?)

Nicholas Clark

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 10, 2012

From @iabyn

On Thu, May 10, 2012 at 02​:27​:05PM +0100, Nicholas Clark wrote​:

On Tue, Sep 06, 2011 at 12​:29​:00PM +0100, Dave Mitchell wrote​:

There is a second issue that is brought up by the original bug report,
and that is that $a =~ /foo/ closes over any outer lexical $_, even though
it doesn't use $_. This is because when /foo/ is compiled, it initially
assumes that it wants to use $_ and so closes over it (if there's a lexical
version in scope); only later does it see the =~ bind operator, and
undo that assumption (by setting OPf_STACKED). By then the closure
has already been set up (a FAKE entry in the pad).

I don't see a simple way to fix that.

Meaning that the third section of this code is buggy?
[snip]
sub buggy {
my $_ = shift;
return sub {
my $m = "I'm not to be a closure\n";
$m =~ s/not/not meant/;
print $m;
}
}
[snip]
print "Should not close either\n";
$s = buggy(bless []);
print "(object should be gone)\n";
$s->();
undef $s;

__END__
$ ./perl -Ilib ../test/94682.pl
[snip]
Should not close either
(object should be gone)
I'm not meant to be a closure
DESTROY for main=ARRAY(0x1008114c0)

In that the sub buggy() should not return a closure, hence the third
paragraph of output should be the same as the second, not the first, as
currently is?

Correct.

(and this should become a TODO test somewhere?)

Yes. op/closure.t ???

--
"I do not resent criticism, even when, for the sake of emphasis,
it parts for the time with reality".
  -- Winston Churchill, House of Commons, 22nd Jan 1941.

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

Successfully merging a pull request may close this issue.

None yet
1 participant