Pass PADOFFSET integers in to subsignature_append_*() functions

This simplifies potential callsites from other locations, avoiding their
need to create the special weird OP_ARGELEM fragments. Additionally this
decouples the knowledge that these operations are run on OP_ARGELEM ops
in the first place, allowing for a future version that operates
differently.

Author: leonerd

embed.fnc

@@ -3111,12 +3111,12 @@ CRp	|NV	|str_to_version |NN SV *sv
 p	|void	|sub_crush_depth|NN CV *cv
 : Used in perly.y
 p	|void	|subsignature_append_positional 			\
-				|NULLOK OP *varop			\
+				|PADOFFSET padix			\
 				|OPCODE defmode 			\
 				|NULLOK OP *defexpr
 p	|void	|subsignature_append_slurpy				\
 				|I32 sigil				\
-				|NULLOK OP *varop
+				|PADOFFSET padix
 p	|OP *	|subsignature_finish
 p	|void	|subsignature_start
 Adp	|void	|suspend_compcv |NN struct suspended_compcv *buffer

op.c

@@ -16519,17 +16519,15 @@ Perl_subsignature_start(pTHX)
 }
 
 /* Appends another positional scalar parameter to the accumulated set of
- * subroutine params. `varop` may be NULL, but if not it must be an OP_ARGELEM
- * whose op_targ refers to an already-declared pad lexical. That lexical must
- * be a scalar. It is not necessary to set the argument index in the op_aux
- * field; that will be filled in by this function.
+ * subroutine params. `padix` may be zero, but if not it must be the pad
+ * index of a scalar pad lexical to store the incoming argument value into.
  * If `defexpr` is not NULL, it gives a defaulting expression to be evaluated
  * if required, according to `defmode` - one of zero, `OP_DORASSIGN` or
  * `OP_ORASSIGN`.
  */
 
 void
-Perl_subsignature_append_positional(pTHX_ OP *varop, OPCODE defmode, OP *defexpr)
+Perl_subsignature_append_positional(pTHX_ PADOFFSET padix, OPCODE defmode, OP *defexpr)
 {
     PERL_ARGS_ASSERT_SUBSIGNATURE_APPEND_POSITIONAL;
     assert(PL_parser);
@@ -16541,13 +16539,13 @@ Perl_subsignature_append_positional(pTHX_ OP *varop, OPCODE defmode, OP *defexpr
 
     UV argix = signature->elems;
 
-    if(varop) {
-        assert(varop->op_type == OP_ARGELEM);
-        assert((varop->op_private & OPpARGELEM_MASK) == OPpARGELEM_SV);
-        assert(varop->op_targ);
-        assert(PadnamePV(PadnamelistARRAY(PL_comppad_name)[varop->op_targ])[0] == '$');
+    OP *varop = NULL;
+    if(padix) {
+        assert(PadnamePV(PadnamelistARRAY(PL_comppad_name)[padix])[0] == '$');
 
-        /* Now fill in the argix */
+        varop = newUNOP_AUX(OP_ARGELEM, 0, NULL, NULL);
+        varop->op_private |= OPpARGELEM_SV;
+        varop->op_targ = padix;
         cUNOP_AUXx(varop)->op_aux = INT2PTR(UNOP_AUX_item *, argix);
     }
 
@@ -16604,14 +16602,13 @@ Perl_subsignature_append_positional(pTHX_ OP *varop, OPCODE defmode, OP *defexpr
 }
 
 /* Appends a final slurpy parameter to the accumulated set of subroutine
- * params. `varop` may be NULL, but if not it must be an OP_ARGELEM whose
- * op_targ refers to an already-declared pad lexical. That lexical must match
- * the `sigil` parameter. It is not necessary to set the argument index in the
- * op_aux field; that will be filled in by this function.
+ * params. `padix` may be zero, but if not it must be the pad index of an
+ * array or hash lexical to store the remaining argument values into. Its
+ * sigil must match the `sigil` parameter.
  */
 
 void
-Perl_subsignature_append_slurpy(pTHX_ I32 sigil, OP *varop)
+Perl_subsignature_append_slurpy(pTHX_ I32 sigil, PADOFFSET padix)
 {
     PERL_ARGS_ASSERT_SUBSIGNATURE_APPEND_SLURPY;
     assert(PL_parser);
@@ -16624,21 +16621,19 @@ Perl_subsignature_append_slurpy(pTHX_ I32 sigil, OP *varop)
 
     UV argix = signature->elems;
 
-    if(varop) {
-        assert(varop->op_type == OP_ARGELEM);
-        assert((varop->op_private & OPpARGELEM_MASK) ==
-                ((sigil == '@') ? OPpARGELEM_AV : OPpARGELEM_HV));
-        assert(varop->op_targ);
-        assert(PadnamePV(PadnamelistARRAY(PL_comppad_name)[varop->op_targ])[0] == sigil);
+    signature->slurpy = (char)sigil;
 
-        /* Now fill in the argix */
-        cUNOP_AUXx(varop)->op_aux = INT2PTR(UNOP_AUX_item *, argix);
-    }
+    if(padix) {
+        assert(PadnamePV(PadnamelistARRAY(PL_comppad_name)[padix])[0] == sigil);
 
-    signature->slurpy = (char)sigil;
+        OP *varop = newUNOP_AUX(OP_ARGELEM, 0, NULL, NULL);
+        if(sigil == '@')
+            varop->op_private |= OPpARGELEM_AV;
+        if(sigil == '%')
+            varop->op_private |= OPpARGELEM_HV;
+        varop->op_targ = padix;
+        cUNOP_AUXx(varop)->op_aux = INT2PTR(UNOP_AUX_item *, argix);
 
-    if(varop) {
-        /* TODO: assert() the sigil of the pad variable matches */
         signature->elemops = op_append_list(OP_LINESEQ, signature->elemops,
                 newSTATEOP(0, NULL, varop));
     }

perly.act

@@ -114,8 +114,8 @@
 %type <opval> optfieldattrlist fielddecl
 %type <opval> termbinop termunop anonymous termdo
 %type <opval> termrelop relopchain termeqop eqopchain
-%type <ival>  sigslurpsigil
-%type <opval> sigvarname sigscalarelem sigslurpelem
+%type <ival>  sigslurpsigil sigvar
+%type <opval> sigscalarelem sigslurpelem
 %type <opval> sigelem siglist optsiglist subsigguts subsignature optsubsignature
 %type <opval> subbody optsubbody sigsubbody optsigsubbody
 %type <opval> formstmtseq formline formarg
@@ -822,10 +822,10 @@ myattrlist:	COLONATTR THING
  */
 
 /* the '' or 'foo' part of a '$' or '@foo' etc signature variable  */
-sigvarname:     %empty
-			{ parser->in_my = 0; $$ = NULL; }
+sigvar:     %empty
+			{ parser->in_my = 0; $$ = 0; }
         |       PRIVATEREF
-                        { parser->in_my = 0; $$ = $PRIVATEREF; }
+                        { parser->in_my = 0; $$ = $PRIVATEREF->op_targ; op_free($PRIVATEREF); }
 	;
 
 sigslurpsigil:
@@ -835,36 +835,36 @@ sigslurpsigil:
                         { $$ = '%'; }
 
 /* @, %, @foo, %foo */
-sigslurpelem: sigslurpsigil sigvarname
+sigslurpelem: sigslurpsigil sigvar
                         {
-                            subsignature_append_slurpy($sigslurpsigil, $sigvarname);
+                            subsignature_append_slurpy($sigslurpsigil, $sigvar);
                             $$ = NULL;
                         }
-        |     sigslurpsigil sigvarname ASSIGNOP
+        |     sigslurpsigil sigvar ASSIGNOP
                         {
 			    yyerror("A slurpy parameter may not have a default value");
                         }
-        |     sigslurpsigil sigvarname ASSIGNOP term
+        |     sigslurpsigil sigvar ASSIGNOP term
                         {
 			    yyerror("A slurpy parameter may not have a default value");
                         }
         ;
 
 /* subroutine signature scalar element: e.g. '$x', '$=', '$x = $default' */
 sigscalarelem:
-                PERLY_DOLLAR sigvarname
+                PERLY_DOLLAR sigvar
                         {
-                            subsignature_append_positional($sigvarname, 0, NULL);
+                            subsignature_append_positional($sigvar, 0, NULL);
                             $$ = NULL;
                         }
-        |       PERLY_DOLLAR sigvarname ASSIGNOP
+        |       PERLY_DOLLAR sigvar ASSIGNOP
                         {
-                            subsignature_append_positional($sigvarname, $ASSIGNOP, newOP(OP_NULL, 0));
+                            subsignature_append_positional($sigvar, $ASSIGNOP, newOP(OP_NULL, 0));
                             $$ = NULL;
                         }
-        |       PERLY_DOLLAR sigvarname ASSIGNOP term[defop]
+        |       PERLY_DOLLAR sigvar ASSIGNOP term[defop]
                         {
-                            subsignature_append_positional($sigvarname, $ASSIGNOP, $defop);
+                            subsignature_append_positional($sigvar, $ASSIGNOP, $defop);
                             $$ = NULL;
                         }
         ;