From: Vincent Pit <vince@profvince.com>
Date: Fri, 25 Jun 2010 18:34:14 +0000 (+0200)
Subject: Exception propagation fixes
X-Git-Tag: v0.43~6
X-Git-Url: http://git.vpit.fr/?p=perl%2Fmodules%2FVariable-Magic.git;a=commitdiff_plain;h=a5a7e1159e5dc96c5046b990a7a23316ab5d5889

Exception propagation fixes

First, make sure that exceptions thrown in a free callback at the end of a
regular block are propagated.

Second, prevent duplication of error messages when dieing in a free callback
at BEGIN time.
---

diff --git a/Magic.xs b/Magic.xs
index ccce441..0cfce8c 100644
--- a/Magic.xs
+++ b/Magic.xs
@@ -1038,13 +1038,12 @@ STATIC int vmg_svt_clear(pTHX_ SV *sv, MAGIC *mg) {
 
 STATIC int vmg_svt_free(pTHX_ SV *sv, MAGIC *mg) {
  const MGWIZ *w;
+ I32 cxix = 0, in_eval = 0;
 #if VMG_HAS_PERL(5, 9, 5)
  PERL_CONTEXT saved_cx;
- I32 cxix;
 #endif
- I32 had_err, has_err, flags = G_SCALAR | G_EVAL;
  int ret = 0;
- SV *svr;
+ SV *svr, *old_err = NULL;
 
  dSP;
 
@@ -1076,37 +1075,55 @@ STATIC int vmg_svt_free(pTHX_ SV *sv, MAGIC *mg) {
   XPUSHs(vmg_op_info(w->opinfo));
  PUTBACK;
 
- had_err = SvTRUE(ERRSV);
- if (had_err)
-  flags |= G_KEEPERR;
+ if (SvTRUE(ERRSV)) {
+  old_err = ERRSV;
+  ERRSV   = newSV(0);
+ }
+
+ if (cxstack_ix < cxstack_max) {
+  cxix = cxstack_ix + 1;
+  if (CxTYPE(cxstack + cxix) == CXt_EVAL)
+   in_eval = 1;
+ }
 
 #if VMG_HAS_PERL(5, 9, 5)
  /* This context should not be used anymore, but since we croak in places the
   * core doesn't even dare to, some pointers to it may remain in the upper call
   * stack. Make sure call_sv() doesn't clobber it. */
- if (cxstack_ix < cxstack_max)
-  cxix = cxstack_ix + 1;
- else
-  cxix = Perl_cxinc(aTHX);
  saved_cx = cxstack[cxix];
 #endif
 
- call_sv(w->cb_free, flags);
+ call_sv(w->cb_free, G_SCALAR | G_EVAL);
 
 #if VMG_HAS_PERL(5, 9, 5)
  cxstack[cxix] = saved_cx;
 #endif
 
- has_err = SvTRUE(ERRSV);
- if (IN_PERL_COMPILETIME && !had_err && has_err) {
-  if (PL_errors)
-   sv_catsv(PL_errors, ERRSV);
-  else
-   Perl_warn(aTHX_ "%s", SvPV_nolen(ERRSV));
+ if (SvTRUE(ERRSV)) {
+  if (old_err) {
+   sv_setsv(old_err, ERRSV);
+   SvREFCNT_dec(ERRSV);
+   ERRSV = old_err;
+  }
+  if (IN_PERL_COMPILETIME) {
+   if (!PL_in_eval) {
+    if (PL_errors)
+     sv_catsv(PL_errors, ERRSV);
+    else
+     Perl_warn(aTHX_ "%s", SvPV_nolen(ERRSV));
+    SvCUR_set(ERRSV, 0);
+   }
 #ifdef PL_parser
-  if (PL_parser)
+   if (PL_parser)
 #endif
-   ++PL_error_count;
+    ++PL_error_count;
+  } else if (!in_eval)
+   croak(NULL);
+ } else {
+  if (old_err) {
+   SvREFCNT_dec(ERRSV);
+   ERRSV = old_err;
+  }
  }
 
  SPAGAIN;
diff --git a/t/17-ctl.t b/t/17-ctl.t
index a7557ae..b25c5ee 100644
--- a/t/17-ctl.t
+++ b/t/17-ctl.t
@@ -1,21 +1,28 @@
-#!perl -T
+#!perl
 
 use strict;
 use warnings;
 
-use Test::More tests => 10 + 1;
+use Test::More tests => 14 + 1;
 
 use Variable::Magic qw/wizard cast/;
 
 my $wiz;
 
+sub expect {
+ my ($name, $where, $suffix) = @_;
+ $where  = defined $where ? quotemeta $where : '\(eval \d+\)';
+ my $end = defined $suffix ? "$suffix\$" : '$';
+ qr/^\Q$name\E at $where line \d+\.$end/
+}
+
 eval {
  $wiz = wizard data => sub { $_[1]->() };
  my $x;
  cast $x, $wiz, sub { die "carrot" };
 };
 
-like $@, qr/carrot/, 'die in data callback';
+like $@, expect('carrot', $0), 'die in data callback';
 
 eval {
  $wiz = wizard data => sub { $_[1] },
@@ -25,7 +32,7 @@ eval {
  $x = 5;
 };
 
-like $@, qr/lettuce/, 'die in set callback';
+like $@, expect('lettuce', $0), 'die in set callback';
 
 my $res = eval {
  $wiz = wizard data => sub { $_[1] },
@@ -35,7 +42,7 @@ my $res = eval {
  @a;
 };
 
-like $@, qr/potato/, 'die in len callback';
+like $@, expect('potato', $0), 'die in len callback';
 
 eval {
  $wiz = wizard data => sub { $_[1] },
@@ -44,7 +51,41 @@ eval {
  cast $x, $wiz, sub { die "spinach" };
 };
 
-like $@, qr/spinach/, 'die in free callback';
+like $@, expect('spinach', $0), 'die in free callback';
+
+eval {
+ $wiz = wizard free => sub { die 'zucchini' };
+ $@ = "";
+ {
+  my $x;
+  cast $x, $wiz;
+ }
+ die 'not reached';
+};
+
+like $@, expect('zucchini', $0),
+                          'die in free callback in block in eval with $@ unset';
+
+eval {
+ $wiz = wizard free => sub { die 'eggplant' };
+ $@ = "vuvuzela";
+ {
+  my $x;
+  cast $x, $wiz;
+ }
+ die 'not reached again';
+};
+
+like $@, expect('eggplant', $0),
+                            'die in free callback in block in eval with $@ set';
+
+eval q{BEGIN {
+ $wiz = wizard free => sub { die 'onion' };
+ my $x;
+ cast $x, $wiz;;
+}};
+
+like $@, expect('onion', undef, "\nBEGIN.*"), 'die in free callback in BEGIN';
 
 # Inspired by B::Hooks::EndOfScope
 
@@ -54,7 +95,7 @@ eval q{BEGIN {
  cast $x, $wiz, sub { die "pumpkin" };
 }};
 
-like $@, qr/pumpkin/, 'die in data callback in BEGIN';
+like $@, expect('pumpkin', undef, "\nBEGIN.*"), 'die in data callback in BEGIN';
 
 eval q{BEGIN {
  $wiz = wizard data => sub { $_[1] },
@@ -63,7 +104,7 @@ eval q{BEGIN {
  cast %^H, $wiz, sub { die "macaroni" };
 }};
 
-like $@, qr/macaroni/, 'die in free callback in BEGIN';
+like $@, expect('macaroni'), 'die in free callback at end of scope';
 
 eval q{BEGIN {
  $wiz = wizard data => sub { $_[1] },
@@ -73,12 +114,16 @@ eval q{BEGIN {
  cast @a, $wiz, sub { die "pepperoni" };
 }};
 
-like $@, qr/pepperoni/, 'die in len callback in BEGIN';
+like $@, expect('pepperoni', undef, "\nBEGIN.*"),'die in len callback in BEGIN';
 
 use lib 't/lib';
+
+
 eval "use Variable::Magic::TestScopeEnd";
 
-like $@, qr/turnip/, 'die in BEGIN in require triggers hints hash destructor';
+like $@,
+     expect('turnip', 't/lib/Variable/Magic/TestScopeEnd.pm', "\nBEGIN(?s:.*)"),
+     'die in BEGIN in require in eval string triggers hints hash destructor';
 
 eval q{BEGIN {
  Variable::Magic::TestScopeEnd::hook {
@@ -87,4 +132,26 @@ eval q{BEGIN {
  die "tomato";
 }};
 
-like $@, qr/tomato/, 'die in BEGIN in eval triggers hints hash destructor';
+like $@, expect('tomato', undef, "\nBEGIN.*"),
+                          'die in BEGIN in eval triggers hints hash destructor';
+
+sub run_perl {
+ my $code = shift;
+
+ my $SystemRoot   = $ENV{SystemRoot};
+ local %ENV;
+ $ENV{SystemRoot} = $SystemRoot if $^O eq 'MSWin32' and defined $SystemRoot;
+
+ system { $^X } $^X, '-T', map("-I$_", @INC), '-e', $code;
+}
+
+SKIP:
+{
+ skip 'Capture::Tiny 0.08 is not installed' => 1
+                                     unless eval "use Capture::Tiny 0.08 (); 1";
+ my $code = 'use Variable::Magic qw/wizard cast/; { BEGIN { $^H |= 0x020000; cast %^H, wizard free => sub { die q[cucumber] } } }';
+ my $output = Capture::Tiny::capture_merged(sub { run_perl $code });
+ skip 'Test code didn\'t run properly' => 1 unless defined $output;
+ like $output, expect('cucumber', '-e', "\nExecution(?s:.*)"),
+                                   'die at compile time and not in eval string';
+}