I'd call both cases a bug. local's restore needs to be done before die sets $@. Yes, I understand that this makes the implementation slightly more difficult since it requires storing what will end up in $@ somewhere while the stack is unwound. But it also allows for fixing the long-standing bug of DESTROY methods being able to clobber $@.

Given what I perceive as the perversity of p5p's decision making, I'll note that at the very least BEGIN handling needs to be fixed to check the return value of eval rather than checking $@. Yes, this might mean forcing something like "; 1" onto the end of the BEGIN block's code before it is string-eval'd. But a better fix is discussed below.

To reduce the impact of the change, I'd still set $@ early but also have eval save that value and set $@ to it right before it returns.

(Update: Actually, eval isn't in the picture until the stack unwinding runs into it, so die needs to save $@ somewhere for eval to copy back into $@. And the stack unwinding can involve other cases of eval and die so we really need a stack of these saved values. So die should set $@ then push @@, $@;. Just before eval returns, it should $@= pop @@;?) (Update2: Perhaps better to have eval push a blank value onto @@ up front and have die set both $@ and $@[-1] then $@= pop @@; when eval finishes will always be safe. @@ would start out with one entry in it that perl itself would use so that DESTROY/local can't clobber the error message that caused perl to die. This means that you can use 'die' inside of DESTROY or such to change the error message but you can't just change $@ to do that.)

I didn't mind the DESTROY bug in eval so much because local($@); fixes it. But this bug really sucks (in that I can't see a reasonable way to work around it -- forcing BEGIN { my $pe= $@; ...; $@= $pe } isn't reasonable, IMHO).

This appears to mean that it is best to use local $@; to prevent $@ from being clobbered in some conditions, but also that any local $@; in a scope that gets unwound will break a surrounding eval. So the fix for one aspect of the bug makes the other aspect of the bug more likely. Time to just fix it.

Looking into this also provided the following surprise:

$@= "Before";
my $end= "None";
eval q{
    {
        local $@;
        eval 'die "DoNotLeak\n"';
    }
    $end= $@;
};
print "($end)\n";
[download]

prints "()" when I thought it should print "(Before)". I expect $@ to be set to the empty string when the eval returns successfully. But it appears that instead, $@ is set to the empty string when eval starts. Further testing shows that it is also set to the empty string at the end so I don't see the value in setting it at the start so I'd consider that a low-priority bug.

Here is a little test script:

use strict;
use warnings;
use Test qw( plan ok );

plan( tests => 12 );

sub Test {
    my $first= "None";
    my $end= "None";
    eval q{
        {
            local $@
                if  $_[0];
            eval 'die "DoNotLeak\n"';
            die "DoNotHide\n"
                if  $_[1];
            $first= $@;
        }
        $end= $@;
    };
    chomp
        for $first, $end;
    return( $first, $end );
}

while(  <DATA>  ) {
    my( $local, $die, $one, $two, $three )= split /\s*,/, $_;
    $@= "Before";
    my( $first, $end )= Test( $local, $die );
    ok( $first, $one,   "local:$local die:$die 1" );
    ok( $end,   $two,   "local:$local die:$die 2" );
    ok( $@,     $three, "local:$local die:$die 3" );
}
__END__
0,0,DoNotLeak,DoNotLeak,,
1,0,DoNotLeak,Before   ,,
0,1,None     ,None     ,DoNotHide
1,1,None     ,None     ,DoNotHide
[download]

And my results:

1..12
# Running under perl version 5.008008 for MSWin32
# Using Test.pm version 1.25
ok 1
ok 2
ok 3
ok 4
not ok 5
# Test 5 got: "" (- at line 31 fail #2)
#   Expected: "Before" (local:1 die:0 2)
ok 6
ok 7
ok 8
ok 9
ok 10
ok 11
not ok 12
# Test 12 got: "" (- at line 32 fail #4)
#    Expected: "DoNotHide\n" (local:1 die:1 3)
[download]