MULTICALL *shouldn't* clear savestack
authorDavid Mitchell <davem@iabyn.com>
Thu, 31 Dec 2015 10:39:17 +0000 (10:39 +0000)
committerDavid Mitchell <davem@iabyn.com>
Wed, 3 Feb 2016 09:19:21 +0000 (09:19 +0000)
About 25 commits ago in this branch I added a commit:

    MULTICALL should clear scope after each call

To fix RT #116577, which reported that lexicals were only being freed
at the end of the MULTICALL, not after each individual call to the sub.

In that commit, I added a LEAVE_SCOPE() to the end of the MULTICALL()
definition. However, after further thought I realise that's wrong.  If a
multicall sub does something like { my $x = $_*2; $x }, then the returned
value would be freed before the XS code which calls MULTICALL() has a
chance to do anything with it (e.g. test for truth, or add it to the return
args or whatever).

So I think popping the save stack should be the responsibility of the
caller of MULTICALL(), rather than of MULTICALL() itself.

cop.h
ext/XS-APItest/t/multicall.t
regexec.c

diff --git a/cop.h b/cop.h
index 269bdeb..16254ec 100644 (file)
--- a/cop.h
+++ b/cop.h
@@ -1083,8 +1083,7 @@ See L<perlcall/LIGHTWEIGHT CALLBACKS>.
 
 #define dMULTICALL \
     OP  *multicall_cop;                                                        \
-    bool multicall_oldcatch;                                           \
-    I32  multicall_saveix_floor
+    bool multicall_oldcatch
 
 #define PUSH_MULTICALL(the_cv) \
     PUSH_MULTICALL_FLAGS(the_cv, 0)
@@ -1105,7 +1104,6 @@ See L<perlcall/LIGHTWEIGHT CALLBACKS>.
                   PL_stack_sp, PL_savestack_ix);                       \
         cx_pushsub(cx, cv, NULL, 0);                                    \
        SAVEOP();                                                       \
-        multicall_saveix_floor = PL_savestack_ix;                       \
         if (!(flags & CXp_SUB_RE_FAKE))                                 \
             CvDEPTH(cv)++;                                             \
        if (CvDEPTH(cv) >= 2)                                           \
@@ -1118,7 +1116,6 @@ See L<perlcall/LIGHTWEIGHT CALLBACKS>.
     STMT_START {                                                       \
        PL_op = multicall_cop;                                          \
        CALLRUNOPS(aTHX);                                               \
-        LEAVE_SCOPE(multicall_saveix_floor);                            \
     } STMT_END
 
 #define POP_MULTICALL \
index d152644..51ef276 100644 (file)
@@ -161,15 +161,10 @@ use XS::APItest;
         @a = multicall_return \&f9, $gimme;
         gimme_check($gimme, \@a, ["one", "two"], "for-return two args lval");
     }
-}
-
-# RT #116577: MULTICALL should clear scope after each call
-
-{
-    my @r;
 
-    my $s = sub { my $x; push @r, \$x; 1 };
+    # MULTICALL *shouldn't* clear savestack after each call
 
-    XS::APItest::multicall_each \&$s, 1,2;
-    isnt($r[0], $r[1], "#116577");
+    sub f10 { my $x = 1; $x };
+    my @a = XS::APItest::multicall_return \&f10, G_SCALAR;
+    ::is($a[0], 1, "leave scope");
 }
index 80d3576..b87bb74 100644 (file)
--- a/regexec.c
+++ b/regexec.c
@@ -6660,7 +6660,6 @@ S_regmatch(pTHX_ regmatch_info *reginfo, char *startpos, regnode *prog)
                before = (IV)(SP-PL_stack_base);
                PL_op = nop;
                CALLRUNOPS(aTHX);                       /* Scalar context. */
-                PERL_UNUSED_VAR(multicall_saveix_floor); /* used by MULTICALL */
                SPAGAIN;
                if ((IV)(SP-PL_stack_base) == before)
                    ret = &PL_sv_undef;   /* protect against empty (?{}) blocks. */