Handle anonymous scalar params as well

leonerd · Nov 18, 2024 · 9f5f43f · 9f5f43f
1 parent 3f85616
commit 9f5f43f
Show file tree

Hide file tree

Showing 8 changed files with 165 additions and 51 deletions.
diff --git a/dump.c b/dump.c
@@ -1576,9 +1576,15 @@ S_do_op_dump_bar(pTHX_ I32 level, UV bar, PerlIO *file, const OP *o)
             S_opdump_indent(aTHX_ o, level, bar, file, "PARAMS = %" UVuf "\n",
                     nparams);
 
-        for(Size_t i = 0; i < nparams; i++)
-            S_opdump_indent(aTHX_ o, level, bar, file, "  PARAM [%zd] PADIX = %" UVuf "%s\n",
-                    i, aux->param_padix[i], i >= nparams_mandatory ? " OPT" : "");
+        for(Size_t i = 0; i < nparams; i++) {
+            PADOFFSET padix = aux->param_padix[i];
+            if(padix)
+                S_opdump_indent(aTHX_ o, level, bar, file, "  PARAM [%zd] PADIX = %" UVuf "%s\n",
+                        i, aux->param_padix[i], i >= nparams_mandatory ? " OPT" : "");
+            else
+                S_opdump_indent(aTHX_ o, level, bar, file, "  PARAM [%zd] ANON\n",
+                        i);
+        }
 
         if(aux->slurpy)
             S_opdump_indent(aTHX_ o, level, bar, file, "SLURPY = '%c' PADIX = %" UVuf "\n",

diff --git a/lib/B/Op_private.pm b/lib/B/Op_private.pm
diff --git a/opcode.h b/opcode.h
diff --git a/peep.c b/peep.c
@@ -1046,10 +1046,18 @@ S_maybe_multiparam(pTHX_ OP *o)
 
     /* Phase 1: Test for validity and count things */
 
-    assert(o->op_type == OP_LEAVESUB || o->op_type == OP_LEAVESUBLV);
+    if(OP_TYPE_IS(o, OP_LEAVEEVAL))
+        /* eval blocks don't have signatures */
+        return;
+
+    assert(OP_TYPE_IS(o, OP_LEAVESUB) || OP_TYPE_IS(o, OP_LEAVESUBLV));
     o = cUNOPo->op_first;
-    assert(o->op_type == OP_LINESEQ);
-    o = cLISTOPo->op_first;
+
+    /* Most subs have an OP_LINESEQ at toplevel, whose first child is the
+     * argcheck subtree, the rest is the body. However, subs with no body will
+     * go straight to the NULL[LINESEQ[ARGCHECK...]]
+     */
+    if(o->op_type == OP_LINESEQ) o = cLISTOPo->op_first;
     if(o->op_type == OP_NULL && (o->op_flags & OPf_KIDS)) o = cUNOPo->op_first;
     if(o->op_type == OP_LINESEQ) o = cLISTOPo->op_first;
 
@@ -1059,7 +1067,7 @@ S_maybe_multiparam(pTHX_ OP *o)
         o = OpSIBLING(o);
     }
 
-    if(o->op_type != OP_ARGCHECK)
+    if(!o || o->op_type != OP_ARGCHECK)
         return;
 
     OP *argcheck = o;
@@ -1068,22 +1076,33 @@ S_maybe_multiparam(pTHX_ OP *o)
     UV nparams = argcheck_aux->params;
     UV nparams_mandatory = nparams - argcheck_aux->opt_params;
 
-    /* Now we should expect to see 'params' count of COP/ARGELEM pairs. Check
-     * we have each.
+    o = OpSIBLING(o);
+
+    /* Now we should expect to see some COP/ARGELEM pairs.
+     * Anonymous scalar params do not appear in the sequence at all, so we can
+     * ignore those for now
      */
     OP *final_argelem = NULL;
-    for(UV parami = 0; parami < nparams; parami++) {
-        o = OpSIBLING(o);
+    for(/**/; o; o = OpSIBLING(o)) {
         SKIP_COP(o);
 
+        if(!o)
+            /* We ran out of args already after the final cop */
+            break;
+
+        /* If this is now anything but an OP_ARGELEM then we don't understand
+         * what's going on; just give up
+         */
         if(o->op_type != OP_ARGELEM)
             return;
 
+        UV parami = PTR2IV(cUNOP_AUXo->op_aux);
+
         if(parami < nparams_mandatory) {
             if(o->op_flags & OPf_STACKED)
                 return;
         }
-        else {
+        else if(parami < nparams) {
             if(!(o->op_flags & OPf_STACKED))
                 return;
             if(!(o->op_flags & OPf_KIDS))
@@ -1095,24 +1114,22 @@ S_maybe_multiparam(pTHX_ OP *o)
             if(!(defelem->op_flags & OPf_KIDS))
                 return;
         }
+        else {
+            if(!argcheck_aux->slurpy)
+                return;
+        }
 
         final_argelem = o;
     }
 
     if(argcheck_aux->slurpy) {
-        o = OpSIBLING(o);
-        SKIP_COP(o);
-
-        if(o->op_type != OP_ARGELEM)
+        /* Make sure we did find the slurpy */
+        if(!final_argelem)
             return;
 
-        U8 priv = o->op_private & OPpARGELEM_MASK;
-        assert(priv == OPpARGELEM_AV || priv == OPpARGELEM_HV);
-
-        final_argelem = o;
-    }
-    else {
-        /* TODO: Maybe look down the chain to see that we *don't* have an OP_ARGELEM ? */
+        U8 priv = final_argelem->op_private & OPpARGELEM_MASK;
+        if(!(priv == OPpARGELEM_AV || priv == OPpARGELEM_HV))
+            return;
     }
 
     OP *next_after_args = (final_argelem) ?
@@ -1133,17 +1150,38 @@ S_maybe_multiparam(pTHX_ OP *o)
     aux->slurpy     = argcheck_aux->slurpy;
     aux->slurpy_padix = 0;
 
-    OP *paramtests = NULL;
+    /* We could store an OP_LINESEQ but it's only temporary. Instead store
+     * head+tail separately
+     */
+    OP *paramtests_first = NULL, *paramtests_last = NULL;
 
     o = OpSIBLING(argcheck);
-    for(UV parami = 0; parami < argcheck_aux->params; parami++) {
+    UV max_parami = 0;
+    for(OP *onext; o; o = onext) {
         OP *cop_for_param = NULL;
         if(OP_TYPE_IS_COP(o)) {
             cop_for_param = o;
             o = OpSIBLING(o);
         }
 
-        OP *onext = OpSIBLING(o);
+        if(!o)
+            break;
+
+        onext = OpSIBLING(o);
+
+        UV parami = PTR2IV(cUNOP_AUXo->op_aux);
+        while(max_parami < parami) {
+            aux->param_padix[max_parami] = 0;
+            max_parami++;
+        }
+
+        if(parami >= nparams) {
+            /* This is final slurpy */
+            aux->slurpy_padix = o->op_targ;
+            break;
+        }
+
+        /* Otherwise this is some kind of non-final scalar */
 
         PADOFFSET padix = aux->param_padix[parami] = o->op_targ;
 
@@ -1156,7 +1194,7 @@ S_maybe_multiparam(pTHX_ OP *o)
                 op_free(cop_for_param);
             op_free(o);
         }
-        else {
+        else if(parami < nparams) {
             /* This is optional param */
 
             /* We'll have to capture the defaulting expression subtree and
@@ -1177,6 +1215,7 @@ S_maybe_multiparam(pTHX_ OP *o)
             OP *defelem = cUNOPo->op_first;
             OP *defexpr       = cLOGOPx(defelem)->op_first;
             OP *defexpr_start = cLOGOPx(defelem)->op_other;
+            U8  defexpr_priv  = defelem->op_private;
 
             cLOGOPx(defelem)->op_first = NULL;
             cLOGOPx(defelem)->op_other = NULL;
@@ -1196,23 +1235,29 @@ S_maybe_multiparam(pTHX_ OP *o)
              * more ->op_next issues. We'll have to fix them up later. */
             OP *paramtest = (OP *)alloc_LOGOP(OP_PARAMTEST, paramstore, defexpr_start);
             paramtest->op_flags |= OPf_WANT_VOID;
+            if(defexpr_priv & OPpARG_IF_UNDEF)
+                paramtest->op_private |= OPpPARAM_IF_UNDEF;
+            if(defexpr_priv & OPpARG_IF_FALSE)
+                paramtest->op_private |= OPpPARAM_IF_FALSE;
             paramtest->op_targ = padix;
             OpLASTSIB_set(paramstore, paramtest);
 
-            paramtests = op_append_elem(OP_LINESEQ, paramtests,
-                cop_for_param);
-            paramtests = op_append_elem(OP_LINESEQ, paramtests,
-                paramtest);
+            OpLASTSIB_set(cop_for_param, NULL);
+            if(paramtests_last) OpMORESIB_set(paramtests_last, cop_for_param);
+            paramtests_last = cop_for_param;
+            if(!paramtests_first) paramtests_first = paramtests_last;
+
+            OpLASTSIB_set(paramtest, NULL);
+            if(paramtests_last) OpMORESIB_set(paramtests_last, paramtest);
+            paramtests_last = paramtest;
         }
 
-        o = onext;
+        max_parami = parami + 1;
     }
 
-    if(argcheck_aux->slurpy) {
-        o = OpSIBLING(o);
-        SKIP_COP(o);
-
-        aux->slurpy_padix = o->op_targ;
+    while(max_parami < nparams) {
+        aux->param_padix[max_parami] = 0;
+        max_parami++;
     }
 
     op_free(argcheck); argcheck = NULL;
@@ -1224,8 +1269,8 @@ S_maybe_multiparam(pTHX_ OP *o)
 
     OP *tail = multiparam;
 
-    if(paramtests) {
-        for(OP *kid = cLISTOPx(paramtests)->op_first, *nextkid; kid; kid = nextkid) {
+    if(paramtests_first) {
+        for(OP *kid = paramtests_first, *nextkid; kid; kid = nextkid) {
             nextkid = OpSIBLING(kid);
             OpMORESIB_set(tail, kid);
 
@@ -1238,12 +1283,6 @@ S_maybe_multiparam(pTHX_ OP *o)
 
             tail = kid;
         }
-
-        /* Free the (now-empty) temporary lineseq */
-        cLISTOPx(paramtests)->op_first = NULL;
-        cLISTOPx(paramtests)->op_last  = NULL;
-        paramtests->op_flags &= ~OPf_KIDS;
-        op_free(paramtests);
     }
 
     OpMORESIB_set(tail, cop_after_args);

diff --git a/pp.c b/pp.c
@@ -7816,7 +7816,14 @@ PP(pp_multiparam)
 
     UV parami;
     for(parami = 0; parami < nparams; parami++) {
-        SV **padentry = &PAD_SVl(aux->param_padix[parami]);
+        PADOFFSET padix = aux->param_padix[parami];
+        if(!padix) {
+            if(argc)
+                argc--;
+            continue;
+        }
+
+        SV **padentry = &PAD_SVl(padix);
         save_clearsv(padentry);
 
         if(!argc) {
@@ -7897,6 +7904,11 @@ PP(pp_paramtest)
 
     bool ok = TARG && !SvPADSTALE(TARG);
 
+    if (ok && (PL_op->op_private & OPpPARAM_IF_UNDEF) && !SvOK(TARG))
+        ok = false;
+    if (ok && (PL_op->op_private & OPpPARAM_IF_FALSE) && !SvTRUE(TARG))
+        ok = false;
+
     if(!ok)
         return cLOGOP->op_other;
 

diff --git a/regen/op_private b/regen/op_private
@@ -925,6 +925,12 @@ addbits('argdefelem',
     6 => qw(OPpARG_IF_FALSE  IF_FALSE),
 );
 
+# These paramtest bits must be in the same place as argdefelem
+addbits('paramtest',
+    7 => qw(OPpPARAM_IF_UNDEF  IF_UNDEF),
+    6 => qw(OPpPARAM_IF_FALSE  IF_FALSE),
+);
+
 addbits('helemexistsor',
     7 => qw(OPpHELEMEXISTSOR_DELETE DELETE),
 );