Skip to content

Commit

Permalink
Close over stale vars in active subs
Browse files Browse the repository at this point in the history
\$x and sub { $x }->() should never produce different values.  But
this used to be possible because sub cloning (which happens with
sub{...}) was written to avoid closing over variables that are not
active.  Not closing over inactive variables makes sense in cases like
this (because the variable doesn’t really exist yet):

sub {
    my $x;
    sub foo {
        $x
    }
}
foo;

but the logic breaks down in cases like this (which was printing 3
only on the first print):

sub foo {
    my $x;
    sub bar {
        $x = 3;
        print $x, "\n";
        sub { print $x, "\n" }->()
    }
}
bar();

If bar can see a scalar named $x (even if it is questionable),
sub { $x }->() should jolly well see the same scalar as the immedi-
ately enclosing sub.

The only case where a run-time cloning of a CV should refuse to close
over the same scalar that the outer sub sees is when the outer sub is
not running.  That only happens with formats:

sub f {
    my $x;
    format =
@
$x
.
}
write STDOUT;

As of this commit, it does only happen with formats.

The actual cases of subs refusing to close over stale variables in
active parents have changed twice since 5.10.0.  See the comments in
the tests.
  • Loading branch information
Father Chrysostomos committed Aug 4, 2012
1 parent 3207fc6 commit cae5dbb
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 9 deletions.
3 changes: 2 additions & 1 deletion pad.c
Expand Up @@ -2020,7 +2020,8 @@ Perl_cv_clone(pTHX_ CV *proto)
while my $x if $false can leave an active var marked as
stale. And state vars are always available */
if (!outpad || !(sv = outpad[PARENT_PAD_INDEX(namesv)])
|| (SvPADSTALE(sv) && !SvPAD_STATE(namesv))) {
|| ( SvPADSTALE(sv) && !SvPAD_STATE(namesv)
&& !CvDEPTH(outside)) ) {
Perl_ck_warner(aTHX_ packWARN(WARN_CLOSURE),
"Variable \"%"SVf"\" is not available", namesv);
sv = NULL;
Expand Down
43 changes: 35 additions & 8 deletions t/op/closure.t
Expand Up @@ -654,17 +654,19 @@ __EOF__
}

sub f {
my $x if $_[0];
sub { \$x }
my $x;
format ff =
@
$r = \$x
.
}

{
f(1);
my $c1= f(0);
my $c2= f(0);

my $r1 = $c1->();
my $r2 = $c2->();
fileno ff;
write ff;
my $r1 = $r;
write ff;
my $r2 = $r;
isnt($r1, $r2,
"don't copy a stale lexical; crate a fresh undef one instead");
}
Expand Down Expand Up @@ -750,5 +752,30 @@ is $closure_test::s2->()(), '10 cubes',
::is $s2->()(), 3, 'cloning closure proto whose CvOUTSIDE has changed';
}

# This should never emit two different values:
# print $x, "\n";
# print sub { $x }->(), "\n";
# This test case started to do just that in commit 33894c1aa3e
# (5.10.1/5.12.0):
sub mosquito {
my $x if @_;
return if @_;

$x = 17;
is sub { $x }->(), $x, 'closing over stale var in 2nd sub call';
}
mosquito(1);
mosquito;
# And this case in commit adf8f095c588 (5.14):
sub anything {
my $x;
sub gnat {
$x = 3;
is sub { $x }->(), $x,
'closing over stale var before 1st sub call';
}
}
gnat();


done_testing();

0 comments on commit cae5dbb

Please sign in to comment.