This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
harmonise S_pushav() and pp_padav()
authorDavid Mitchell <davem@iabyn.com>
Wed, 19 Jul 2017 13:04:36 +0000 (14:04 +0100)
committerDavid Mitchell <davem@iabyn.com>
Thu, 27 Jul 2017 10:30:24 +0000 (11:30 +0100)
These two functions contain a similar block of code to push an array
onto the stack. However they have some slight differences, which this
commit removes. This will allow padav() to call S_pushav() in the next
commit.

The two differences are:

1) S_pushav() when pushing elements of a magical array, calls mg_get()
on each element. This is to ensure that e.g. in

    sub f { /..../; @+ }

when the elements of @+ are returned, they are set *before* the current
pattern goes out of scope.

However, since probably v5.11.5-132-gfd69380 and v5.13.0-22-g2d961f6,
the mg_get is no longer required.

2) S_pushav() uses the SvRMAGICAL() test to decide whether its unsafe
to access AvARRAY directly; pp_padav() uses SvMAGICAL(). The latter
seems too severe, so I've changed it to SvRMAGICAL().

pp_hot.c

index 079fe35..06639fc 100644 (file)
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -320,10 +320,7 @@ PP(pp_concat)
   }
 }
 
-/* push the elements of av onto the stack.
- * XXX Note that padav has similar code but without the mg_get().
- * I suspect that the mg_get is no longer needed, but while padav
- * differs, it can't share this function */
+/* push the elements of av onto the stack */
 
 STATIC void
 S_pushav(pTHX_ AV* const av)
@@ -335,10 +332,7 @@ S_pushav(pTHX_ AV* const av)
         PADOFFSET i;
         for (i=0; i < (PADOFFSET)maxarg; i++) {
             SV ** const svp = av_fetch(av, i, FALSE);
-            /* See note in pp_helem, and bug id #27839 */
-            SP[i+1] = svp
-                ? SvGMAGICAL(*svp) ? (mg_get(*svp), *svp) : *svp
-                : &PL_sv_undef;
+            SP[i+1] = svp ? *svp : &PL_sv_undef;
         }
     }
     else {
@@ -1057,7 +1051,7 @@ PP(pp_padav)
         /* XXX see also S_pushav in pp_hot.c */
        const SSize_t maxarg = AvFILL(MUTABLE_AV(TARG)) + 1;
        EXTEND(SP, maxarg);
-       if (SvMAGICAL(TARG)) {
+       if (SvRMAGICAL(TARG)) {
            SSize_t i;
            for (i=0; i < maxarg; i++) {
                SV * const * const svp = av_fetch(MUTABLE_AV(TARG), i, FALSE);