Just got my butt chomped by this one, so I thought I'd share.

What's wrong with this function?

sub foo {
  return eval { bar( shift ) } || 0;
}

[download]
The answer does not depend on the definition of bar. E.g. let bar be 
sub bar { return shift; }

[download]
To see what's wrong, try this: 
print foo( 3 ), "\n";
$@ = 3;
print foo( $@ ), "\n";
__END__
3
0

[download]
The moral of the story: never use @_ inside an eval.

the lowliest monk

Replies are listed 'Best First'.
Re: Spot the bug!
by ikegami (Patriarch) on Oct 25, 2006 at 21:13 UTC

    What's wrong with the function? Nothing!

    The only bug I see is in the code under the spoiler tag. You can't rely on a function not modifying $@, $!, $1, etc. If you need to pass these variables to a function, pass them by copy. 

    print foo( 3 ), "\n";     # 3

$@ = 3;
print foo( $@ ), "\n";    # 0  Bad. Passing special var.

$@ = 3;
print foo( "$@" ), "\n";  # 3  There we go.

$@ = 3;
my $e = $@;
print foo( $e ), "\n";    # 3
print $e, "\n";           # 3  Still avilable to us.

    [download]

    In theory, this problem exists whenever a global is passed to a function which modifies that global. $@ is a global modified by eval.

    Rethrowing with die $@ considered harmful demonstrates a similar problem.

    Update: Major rephrasing — I wasn't clear — but no new content except for the link.

[reply]
[d/l]
[select]
Re: Spot the bug!
by Errto (Vicar) on Oct 25, 2006 at 23:00 UTC
    I see your point but I disagree with your conclusion:
    The moral seems to me to be: don't call a function that uses eval and expect the value of $@ to be preserved. Using @_ inside the eval causes no issues that I can see.
[reply]
[d/l]
Re: Spot the bug!
by Anonymous Monk on Oct 26, 2006 at 07:13 UTC
    Red herrings, and a wrong conclusion. It has nothing to do with either @_, or eval. It all has to do with Perl using pass by alias. 
    #!/usr/bin/perl

use strict;
use warnings;

my $n;

sub foo {
    $n = 0;
    return $_[0];
}

print foo(3), "\n";
$n = 3;
print foo($n), "\n";

__END__
3
0

    [download]
    $_[0] is just a different name for $n, which is being modified in foo, just like your function uses eval, which sets $@.
[reply]
[d/l]
[select]
Re: Spot the bug!
by tlm (Prior) on Oct 26, 2006 at 13:18 UTC

    Actually, after thinking about it some more, I think that the argument below (which I generally agree with in the abstract), does not apply to the example I posted, because, as the repliers pointed out in the first place, avoiding @_ inside an eval won't prevent $@ from being clobbered by the eval. To follow the prescription I proposed below would require localizing the eval within the function... I'm coming around to my detractors' point of view. I stand corrected.

    It was interesting to read the replies, if nothing else just to see how perspectives on something like this can differ. Maybe this is one of those "six tomaytoes/half-a-dozen tomahtos"-type arguments...

    The example I posted illustrates a collision of expectations: the function caller's, which is that "any reasonable, well-behaved function" won't modify the arguments passed to it; and the function writer's, which is that "any reasonable, halfway-intelligent programmer" won't pass as argument to a function anything that shouldn't modified. Clearly one (at least) of these two expectations needs to be corrected. Take your pick.

    In my humble opinion it's the function caller's expectation above that seems the most reasonable of the two, and therefore I proposed what amounts to an adjustment in the function writer's expectations, but all the repliers to my post think it is exactly the other way around. Verrry interesting...

    Of course, one could argue that the best (i.e. most defensive) practice would be to change both expectations...

    the lowliest monk

[reply]
[d/l]
[select]

      Localizing $@ would have protected your caller against your clobbering $@. You'd still have the alias/copy bug so now shift() would always return undef because $@ now is always blank but at least you'd be safer. When using $@, always, always, always copy it out first. The smallest piece of code can go clobber $@ so you should always copy it out before examining it.

      sub foo {
    local $@;
    return eval { bar( shift ) } || 0;
}

      [download]

      ⠤⠤ ⠙⠊⠕⠞⠁⠇⠑⠧⠊

[reply]
[d/l]
[select]

        Agreed. But what about just using @_ = @_ instead of local @_? Assignment to @_ is what the docs prescribe for breaking the aliasing. Any reason for preferring one approach over the other?

        the lowliest monk

[reply]
[d/l]
[select]

Back to Meditations