faiz has asked for the wisdom of the Perl Monks concerning the following question:

Dear Monks,

I have just encountered the following strangeness:
A subroutine appears to 'remember' a my variable from it's previous invocation:

The variable here is a code ref: 
# A 'wannabe' foreach, which:
#   - takes a CODE ref as an optional first argument
#   - applies it to the rest of the args
#   - If no CODE ref was passed, then it uses a closure of its 
#     own that simply shoves all the args into an accumulator list
#     which it just dumps to STDERR
sub wannabe_foreach {
    my $func = shift if ref $_[0] eq 'CODE'; # optional code-ref
    # Funkiness:  $func has a value if:
    #                 (a) the user gave it one; 
    #                     (User thinks closures are funky)
    #                 (b) it mysteriously has a value.
    #                     (many people would call that 'funky')
    warn "my func is funky!\n" if $func;
    my @list = @_;

    my @accumulator = ();
    $func = sub { push @accumulator, shift } unless $func;

    $func->($_) for @list;

    warn join ",", @accumulator if @accumulator;

}

wannabe_foreach(1..3); # Works. Warns: '1,2,3 at foo.pl line ...'
wannabe_foreach(1..4); # Gets funky.

# Still funky, but at least the passed code ref is invoked.
# wannabe_foreach(sub { print shift }, 1..7); print "\n";
# Plain funky again. 
# wannabe_foreach(1..7);

[download]

A cursory debugging shows that the line 

my $func = shift if ref $_[0] eq 'CODE'; # optional code-ref

[download]
Gets funny - $func retains a reference to the closure created in the previous invocation. Looks like the compiler is too eager to optimize the closure instances?

Well whatever the reason is, the fix was simply: 

#my $func = shift if ref $_[0] eq 'CODE';
my $func  = ref $_[0] eq 'CODE' ? shift() : undef;

[download]

Strange. I mean, one would expect that $func end up as 'undef' in any case right? And even then, why does it end up as a reference to the old $func?

So - I suspect some glaring 'overlooking the obvious' mistake on my part, but not having found a reason that I understand yet, I thought I'd ask you all.

Replies are listed 'Best First'.
Re: Unforgettable Closures?
by FunkyMonk (Bishop) on Dec 09, 2007 at 11:36 UTC
    Quoting perlsyn:

    NOTE: The behaviour of a my statement modified with a statement modifier conditional or loop construct (e.g. my $x if ... ) is undefined. The value of the my variable may be undef, any previously assigned value, or possibly anything else. Don't rely on it. Future versions of perl might do something different from the version of perl you try it out on. Here be dragons.
    I thought this "feature" had been removed in the upcoming 5.10 release of perl, but I can't find any reference to it now :(

    update

    Gah! I was reading perl595delta instead of perl5100delta:

    New or Changed Diagnostics

    ...

    Deprecated use of my() in false conditional
    A new deprecation warning, Deprecated use of my() in false conditional, has been added, to warn against the use of the dubious and deprecated construct

        my $x if 0;

    See perldiag. Use state variables instead.

[reply]
[d/l]
[select]
      Gah! I was reading perl595delta instead of perl5100delta

      Is perl5100delta relevant here ?

      I mean ... the code provided by the op produces the same output on both 5.8.8 and 5.10.0 (for me, anyway): 
      1,2,3 at try.pl line 22.
my func is funky!

      [download]
      Cheers,
      Rob
[reply]
[d/l]
        I hadn't tried it earlier, but I get the same here with 5.10.0, with & without warnings. It appears that Deprecated use of my() in false conditional applies only in limited cases:

        my $x if 0;     #warns
my $x if "";    #warns
my $x if undef; #OK
my $x = 0 if 0; #OK

        [download]
        Test program used:

        use warnings;
sub a { my $x = 0 if undef;  print ++$x }
a for 1 .. 3;

        [download]

[reply]
[d/l]
[select]
      Oh boy, Occam's razor. Thanks, that clears it up, I guess. I've not got anything newer than Perl 5.8.8, and grepping perlsyn for the said gotcha finds nothing, so I'm assuming I need a newer Perl?
[reply]

        No, you don't need a newer version of Perl (it wouldn't help you, for several reasons1).

        You need to change your code that looks like: 

        my $var= ... if ...;

        [download]

        to instead look like 

        my $var;
$var= ... if ...;

        [download]

        You don't want to skip the run-time effect "my" when your conditional is false. That run-time effect is what resets your variable so that you don't get a previous value.

        - tye        

        1 A) The newer version just adds warning; it doesn't change how anything works. B) The warning doesn't even apply to what you've done; it appears this new warning is pretty stupid, in fact. C) Just fix your code and it works fine on old versions of Perl.

[reply]
[d/l]
[select]
Re: Unforgettable Closures?
by webfiend (Vicar) on Dec 10, 2007 at 18:46 UTC

    Folks have already pointed out the weirdness result of mixing my and if in the same statement, but I wanted to point out another stylistic issue. You define $func as a subroutine reference whether it's provided by the user or defined by wannabe_foreach, but you've got it going in two distinct places. Why not mush them together so folks can easily see what $func's ultimate fate is?

    use warnings;
use strict;

# A 'wannabe' foreach, which:
#   - takes a CODE ref as an optional first argument
#   - applies it to the rest of the args
#   - If no CODE ref was passed, then it uses a closure of its
#     own that simply shoves all the args into an accumulator list
#     which it just dumps to STDERR
sub wannabe_foreach {
    my ($func, @list, @accumulator);

    # Use provided code-ref if there is one,
    # otherwise create a simple subref to push onto @accumulator
    if ( ref $_[0] eq 'CODE' ) {
        $func = shift; # optional code-ref
    }
    else {
        $func = sub { push @accumulator, shift };
    }

    # If you absolutely gotta be terse there's always
    # $func = ( ref $_[0] eq 'CODE' )
    #       ? shift
    #       : sub { push @accumulator, shift }
    #       ;

    @list = @_;

    $func->($_) for @list;

    warn join ",", @accumulator if @accumulator;

}

wannabe_foreach(1..3); # Works. Warns: '1,2,3 at foo.pl line ...'
wannabe_foreach(1..4); # No funkiness here

    [download]
[reply]
[d/l]

Back to Seekers of Perl Wisdom