This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
S_padhv_rv2hv_common(): reorganise code
authorDavid Mitchell <davem@iabyn.com>
Wed, 19 Jul 2017 16:48:03 +0000 (17:48 +0100)
committerDavid Mitchell <davem@iabyn.com>
Thu, 27 Jul 2017 10:30:24 +0000 (11:30 +0100)
There are three main booleans in play here:

    * whether the hash is tied;
    * whether we're in boolean context;
    * whether we're implementing 'keys %h'

Reorganise the if-tree logic for these up to 8 permutations to make the
code simpler. In particular, make it so that all these are done in only
one place:

    * call HvUSEDKEYS();
    * call magic_scalarpack();
    * push an integer return value, either as TARG or mortal

The functionality should be unchanged, except that now 'scalar(%h)',
where %h isn't tied, will return an integer value using the targ if
available rather than always creating a new mortal.

pp_hot.c

index 233d4d4..eebed8c 100644 (file)
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -973,9 +973,12 @@ PP(pp_print)
 PERL_STATIC_INLINE OP*
 S_padhv_rv2hv_common(pTHX_ HV *hv, U8 gimme, bool is_keys, bool has_targ)
 {
-    bool tied;
+    bool is_tied;
+    bool is_bool;
     MAGIC *mg;
     dSP;
+    IV  i;
+    SV *sv;
 
     assert(PL_op->op_type == OP_PADHV || PL_op->op_type == OP_RV2HV);
 
@@ -988,28 +991,32 @@ S_padhv_rv2hv_common(pTHX_ HV *hv, U8 gimme, bool is_keys, bool has_targ)
         /* 'keys %h' masquerading as '%h': reset iterator */
         (void)hv_iterinit(hv);
 
-    tied = SvRMAGICAL(hv) && (mg = mg_find(MUTABLE_SV(hv), PERL_MAGIC_tied));
-
-    if (  (  PL_op->op_private & OPpTRUEBOOL
-         || (  PL_op->op_private & OPpMAYBE_TRUEBOOL
-            && block_gimme() == G_VOID)
-          )
-    ) {
-        if (tied)
-            PUSHs(magic_scalarpack(hv, mg));
-        else
-            PUSHs(HvUSEDKEYS(hv) ? &PL_sv_yes : &PL_sv_zero);
+    is_bool = (     PL_op->op_private & OPpTRUEBOOL
+              || (  PL_op->op_private & OPpMAYBE_TRUEBOOL
+                  && block_gimme() == G_VOID));
+    is_tied = SvRMAGICAL(hv) && (mg = mg_find(MUTABLE_SV(hv), PERL_MAGIC_tied));
+
+    if (UNLIKELY(is_tied)) {
+        if (is_keys && !is_bool) {
+            i = 0;
+            while (hv_iternext(hv))
+                i++;
+            goto push_i;
+        }
+        else {
+            sv = magic_scalarpack(hv, mg);
+            goto push_sv;
+        }
     }
-    else if (gimme == G_SCALAR) {
-        if (is_keys) {
-            IV i;
-            if (tied) {
-                i = 0;
-                while (hv_iternext(hv))
-                    i++;
-            }
-            else
-                i = HvUSEDKEYS(hv);
+    else {
+        i = HvUSEDKEYS(hv);
+        if (is_bool) {
+            sv = i ? &PL_sv_yes : &PL_sv_zero;
+          push_sv:
+            PUSHs(sv);
+        }
+        else {
+          push_i:
             if (has_targ) {
                 dTARGET;
                 PUSHi(i);
@@ -1017,12 +1024,6 @@ S_padhv_rv2hv_common(pTHX_ HV *hv, U8 gimme, bool is_keys, bool has_targ)
             else
                 mPUSHi(i);
         }
-        else {
-            if (tied)
-                PUSHs(magic_scalarpack(hv, mg));
-            else
-                mPUSHi(HvUSEDKEYS(hv));
-        }
     }
 
     PUTBACK;