Fellow monks,

I just spent a hour trying to find a bug in some code that I was eval-ing. It turns out that there was no bug in the eval code; the bug was in how I checked the result of the eval. I am posting it to the monastery since I've seen this type of bug in LOTS of Perl code before, and I never realized it was a bug until now.

First, some code: 

use strict;
my $num = 1;
print "Surprise!\n" if (!defined(eval { $num++; return;}));
print "$num\r\n";

[download]
As most people know, eval returns undef if it fails, and the value of the last operation if it succeeds. In the code above, the last operation is an argument-less return, so eval returns undef even though it succeeded. Hence, we can't check the return value to determine if an eval succeeded; we need to check $@: 
use strict;
my $num = 1;
eval { $num++; return;};
print "Surprise!\n" if ($@);
print "$num\r\n";

[download]
This gives the desired result.

-Ton
-----
Be bloody, bold, and resolute; laugh to scorn
The power of man...

Replies are listed 'Best First'.
Re: Checking the success of eval
by ncw (Friar) on Apr 21, 2001 at 01:55 UTC
    My favourite gotcha with eval is if you set a die handler then the die handler is called even inside the eval! In fact I consider this a bug in perl - as does the author of "perldoc die"

    Eg 

    $SIG{__DIE__} = sub { print "Caught by die handler: $_[0]"; exit };

eval
{
    die "Oops\n";
};

print "Eval returned error: $@" if $@;

    [download]
    This prints Caught by die handler: Oops.

    The fix goes like this :-

    $SIG{__DIE__} = sub { print "Caught by die handler: $_[0]"; exit };

eval
{
    local $SIG{__DIE__};
    die "Oops\n";
};

print "Eval returned error: $@" if $@;
    Which returns Eval returned error: Oops as anyone would always want.
[reply]
[d/l]

      I consider eval to be an essential feature of Perl and $SIG{__DIE__} to be a mostly bad idea. So if you want to fix the buggy conflict that they have, I really think you should be fixing the die handler and not the eval: 

      $SIG{__DIE__}= sub {
    return   if  $^S;
    print "Caught by die handler: @_";
    exit;
}

      [download]

      Better still is to avoid die handlers altogether and either use eval to catch the exception or use a destructor or END block to do the clean up.

              - tye (but my friends call me "Tye")
[reply]
[d/l]
        Thank you for that $^S nugget tye - I shall faithfully inscribe this into all my $SIG{__DIE__} handlers :-)

        I agree with you that eval is essential, but I disagree with you that $SIG{__DIE__} is a completely bad idea. Just think of if like an eval { } or do_stuff round your entire code and you could come to love it ;-) (If it actually worked that way of course!)

        I like $SIG{__DIE__} for CGI's in the form of CGI::Carp which is really very useful - it would be tiresome to be doing that with eval. I also like $SIG{__DIE__} so I can write scripts which mail me when they've blown up!

        I don't see how you would do it with an END block - perhaps you could enlighten me there? How do you know in an END block whether your program is dieing or just exiting and if you can tell how do you get the value of $@ (presuming that some other destructor/END block used eval)?

[reply]
Re: Checking the success of eval
by satchboost (Scribe) on Apr 21, 2001 at 00:16 UTC
    I hate to say it, but ... Duh! :) $@ was provided for a reson. I personally don't like to assume that a "standard" return value will work if "The Powers That Be"(tm) have provided another method of checking success. Things like eval and $@ are a perfect example.

    It's good of you to post it, though. I never knew about eval returning undef if it failed. :)

[reply]
Re: Checking the success of eval
by cLive ;-) (Prior) on Apr 21, 2001 at 04:17 UTC
    But you answered your own question (in a way :) 
    print "Surprise!\n" if (!defined(eval { $num++; return 1;}));

#or (equivalent)
print "Surprise!\n" if (!defined(eval { $num++; 1;}));

    [download]

    all you need to do is return a value and you're ok.

    cLive ;-)

[reply]
[d/l]

Back to Meditations