mruby-compiler: complete migration from cons-list to struct-based argument handling

This commit completes the transformation of mruby's argument processing from
cons-list based representation to direct struct field access.

Key changes:
- Transform new_args() to return struct mrb_ast_args* instead of cons-list
- Update lambda_body() to use direct struct field access for all argument types
- Fix anonymous keyword rest (**) to use intern_op(pow) marker for proper bytecode generation
- Fix argument forwarding (...) to correctly pass rest_arg to new_args()
- Eliminate mrb_ast_args_tail_node allocation by embedding fields directly in mrb_ast_args
- Update all node structure definitions to use struct mrb_ast_args*
- Remove unused NODE_ARGS enum value since args are now plain C structs

The new approach provides:
- More efficient memory usage by eliminating intermediate cons-list allocations
- Cleaner code generation with direct struct field access
- Proper distinction between anonymous kwrest and no kwrest
- Correct bytecode generation for both anonymous kwrest and argument forwarding

Fixes both anonymous keyword rest (def m(**) end) and argument forwarding
(def a(...) p(...) end) to generate correct bytecode and execute properly.

Co-authored-by: Claude <noreply@anthropic.com>

mrbgems/mruby-compiler/core/codegen.c

@@ -2306,7 +2306,7 @@ search_upvar(codegen_scope *s, mrb_sym id, int *idx)
  * @return The index of the newly created `mrb_irep` in the parent scope's `reps` array.
  */
 static int
-lambda_body(codegen_scope *s, node *locals, node *args, node *body, int blk)
+lambda_body(codegen_scope *s, node *locals, struct mrb_ast_args *args, node *body, int blk)
 {
   codegen_scope *parent = s;
   /* Create a new scope for the lambda/block body. */
@@ -2332,31 +2332,25 @@ lambda_body(codegen_scope *s, node *locals, node *args, node *body, int blk)
     uint32_t pos;
     node *opt;
     node *margs, *pargs;
-    node *tail;
+
+    /* args is already struct mrb_ast_args * */
 
     /* mandatory arguments */
-    ma = node_len(args->car);
-    margs = args->car;
-    tail = args->cdr->cdr->cdr->cdr;
+    ma = node_len(args->mandatory_args);
+    margs = args->mandatory_args;
 
     /* optional arguments */
-    oa = node_len(args->cdr->car);
+    oa = node_len(args->optional_args);
     /* rest argument? */
-    ra = args->cdr->cdr->car ? 1 : 0;
+    ra = args->rest_arg ? 1 : 0;
     /* mandatory arguments after rest argument */
-    pa = node_len(args->cdr->cdr->cdr->car);
-    pargs = args->cdr->cdr->cdr->car;
+    pa = node_len(args->post_mandatory_args);
+    pargs = args->post_mandatory_args;
+
     /* keyword arguments */
-    if (tail) {
-      /* Handle variable-sized NODE_ARGS_TAIL */
-      struct mrb_ast_args_tail_node *tail_node = args_tail_node(tail->cdr);
-      ka = tail_node->keywords ? node_len(tail_node->keywords) : 0;
-      kd = tail_node->kwrest ? 1 : 0;
-      ba = tail_node->block ? 1 : 0;
-    }
-    else {
-      ka = kd = ba = 0;
-    }
+    ka = args->keyword_args ? node_len(args->keyword_args) : 0;
+    kd = args->kwrest_arg ? 1 : 0;
+    ba = args->block_arg ? 1 : 0;
 
     if (ma > 0x1f || oa > 0x1f || pa > 0x1f || ka > 0x1f) {
       codegen_error(s, "too many formal arguments");
@@ -2384,7 +2378,7 @@ lambda_body(codegen_scope *s, node *locals, node *args, node *body, int blk)
     if (oa > 0) {
       genjmp_0(s, OP_JMP); /* Jump to skip all default assignments if all optional args are provided. */
     }
-    opt = args->cdr->car; /* AST node for optional arguments. */
+    opt = args->optional_args; /* AST node for optional arguments. */
     i = 0;
     while (opt) { /* Iterate through optional arguments. */
       int idx;
@@ -2408,16 +2402,11 @@ lambda_body(codegen_scope *s, node *locals, node *args, node *body, int blk)
     }
 
     /* Keyword argument processing */
-    if (tail) { /* `tail` contains keyword arguments and block argument */
+    if (ka > 0 || kd > 0) { /* Has keyword arguments or keyword rest */
       node *kwds;
-      int kwrest = 0; /* Flag for keyword rest argument (e.g., **kwargs) */
+      int kwrest = kd; /* Flag for keyword rest argument (e.g., **kwargs) */
 
-      /* Handle variable-sized NODE_ARGS_TAIL */
-      struct mrb_ast_args_tail_node *tail_node = args_tail_node(tail->cdr);
-      kwds = tail_node->keywords;
-      if (tail_node->kwrest) {
-        kwrest = 1;
-      }
+      kwds = args->keyword_args;
 
       while (kwds) {
         int jmpif_key_p, jmp_def_set = -1;
@@ -2451,20 +2440,20 @@ lambda_body(codegen_scope *s, node *locals, node *args, node *body, int blk)
         kwds = kwds->cdr;
       }
       /* Check if there are keyword args but no keyword rest */
-      int has_keywords = args_tail_node(tail->cdr)->keywords != NULL;
+      int has_keywords = args->keyword_args != NULL;
 
       if (has_keywords && !kwrest) { /* If there are keyword args but no keyword rest. */
         genop_0(s, OP_KEYEND); /* Signal end of keyword arguments. */
       }
-      /* Block argument processing */
-      if (ba) { /* If a block argument (e.g., &blk) is present. */
-        /* Handle variable-sized NODE_ARGS_TAIL */
-        mrb_sym bparam = args_tail_node(tail->cdr)->block;
-        pos = ma+oa+ra+pa+(ka||kd); /* Calculate register offset for the block parameter. */
-        if (bparam) { /* If it's a named block parameter. */
-          int idx = lv_idx(s, bparam);
-          genop_2(s, OP_MOVE, idx, pos+1); /* Move the block from its argument slot to the local variable. */
-        }
+    }
+
+    /* Block argument processing */
+    if (ba) { /* If a block argument (e.g., &blk) is present. */
+      mrb_sym bparam = args->block_arg;
+      pos = ma+oa+ra+pa+(ka||kd); /* Calculate register offset for the block parameter. */
+      if (bparam) { /* If it's a named block parameter. */
+        int idx = lv_idx(s, bparam);
+        genop_2(s, OP_MOVE, idx, pos+1); /* Move the block from its argument slot to the local variable. */
       }
     }
 

mrbgems/mruby-compiler/core/node.h

@@ -245,7 +245,7 @@ struct mrb_ast_hash_node {
 struct mrb_ast_def_node {
   struct mrb_ast_var_header header;  /* 8 bytes */
   mrb_sym name;                      /* Method name */
-  struct mrb_ast_node *args;         /* Method arguments */
+  struct mrb_ast_args *args;         /* Method arguments */
   struct mrb_ast_node *body;         /* Method body */
   struct mrb_ast_node *locals;       /* Local Variables */
 } ;
@@ -254,7 +254,7 @@ struct mrb_ast_def_node {
 struct mrb_ast_sdef_node {
   struct mrb_ast_var_header header;  /* 8 bytes */
   mrb_sym name;                      /* Method name */
-  struct mrb_ast_node *args;         /* Method arguments */
+  struct mrb_ast_args *args;         /* Method arguments */
   struct mrb_ast_node *body;         /* Method body */
   struct mrb_ast_node *locals;       /* Local Variables */
   struct mrb_ast_node *obj;          /* receiver */
@@ -609,31 +609,27 @@ struct mrb_ast_rescue_node {
 struct mrb_ast_block_node {
   struct mrb_ast_var_header hdr;
   struct mrb_ast_node *locals;
-  struct mrb_ast_node *args;
+  struct mrb_ast_args *args;
   struct mrb_ast_node *body;
 };
 
-struct mrb_ast_args_node {
-  struct mrb_ast_var_header hdr;
-  struct mrb_ast_node *mandatory;
-  struct mrb_ast_node *optional;
-  mrb_sym rest;
-  struct mrb_ast_node *mandatory_after_rest;
-  struct mrb_ast_node *tail;
-};
+/* Unified argument structure - eliminates args_tail_node allocation */
+struct mrb_ast_args {
+  /* Core argument lists (parser builds these naturally) */
+  struct mrb_ast_node *mandatory_args;      /* Cons list of mandatory arguments */
+  struct mrb_ast_node *optional_args;       /* Cons list of optional arguments */
+  struct mrb_ast_node *post_mandatory_args; /* Cons list of post-mandatory arguments */
+  struct mrb_ast_node *keyword_args;        /* Cons list of keyword arguments */
 
-struct mrb_ast_args_tail_node {
-  struct mrb_ast_var_header hdr;
-  struct mrb_ast_node *keywords;
-  struct mrb_ast_node *kwrest;
-  mrb_sym block;
+  /* Special arguments (directly embedded) */
+  mrb_sym rest_arg;                         /* Rest argument symbol (0 = none) */
+  mrb_sym kwrest_arg;                       /* Keyword rest argument (0 = none) */
+  mrb_sym block_arg;                        /* Block argument symbol (0 = none) */
 };
 
 /* Advanced node casting macros */
 #define rescue_node(n) ((struct mrb_ast_rescue_node*)(n))
 #define block_node(n) ((struct mrb_ast_block_node*)(n))
-#define args_node(n) ((struct mrb_ast_args_node*)(n))
-#define args_tail_node(n) ((struct mrb_ast_args_tail_node*)(n))
 
 /* Advanced node value access macros */
 #define RESCUE_NODE_BODY(n) (rescue_node(n)->body)
@@ -673,7 +669,6 @@ struct mrb_ast_retry_node {
   struct mrb_ast_var_header hdr;
 };
 
-
 #define break_node(n) ((struct mrb_ast_break_node*)(n))
 #define next_node(n) ((struct mrb_ast_next_node*)(n))
 #define redo_node(n) ((struct mrb_ast_redo_node*)(n))
@@ -798,7 +793,7 @@ struct mrb_ast_defined_node {
 struct mrb_ast_lambda_node {
   struct mrb_ast_var_header hdr;
   struct mrb_ast_node *locals;
-  struct mrb_ast_node *args;
+  struct mrb_ast_args *args;
   struct mrb_ast_node *body;
 };
 

mrbgems/mruby-compiler/core/parse.y

@@ -1273,7 +1273,7 @@ new_def(parser_state *p, mrb_sym name)
 
   init_var_header(&n->header, p, NODE_DEF, class);
   n->name = name;
-  n->args = int_to_node(p->cmdarg_stack);
+  n->args = (struct mrb_ast_args *)int_to_node(p->cmdarg_stack);
   n->locals = local_switch(p);
   n->body = NULL;
 
@@ -1288,7 +1288,7 @@ defn_setup(parser_state *p, node *d, node *a, node *b)
 
   n->locals = locals_node(p);
   p->cmdarg_stack = node_to_int(n->args);
-  n->args = a;
+  n->args = (struct mrb_ast_args *)a;
   n->body = b;
   local_resume(p, locals);
 }
@@ -1305,7 +1305,7 @@ new_sdef(parser_state *p, node *o, mrb_sym name)
   init_var_header(&sdef_node->header, p, NODE_SDEF, class);
   sdef_node->obj = o;
   sdef_node->name = name;
-  sdef_node->args = int_to_node(p->cmdarg_stack);
+  sdef_node->args = (struct mrb_ast_args *)int_to_node(p->cmdarg_stack);
   sdef_node->locals = local_switch(p);
   sdef_node->body = NULL;
   return cons_head((node*)NODE_VARIABLE, (node*)sdef_node);
@@ -1368,31 +1368,54 @@ local_add_lv(parser_state *p, node *lv)
 static node*
 new_args(parser_state *p, node *m, node *opt, mrb_sym rest, node *m2, node *tail)
 {
-  node *n;
-
   local_add_margs(p, m);
   local_add_margs(p, m2);
 
-  n = cons(m2, tail);
-  n = cons(sym_to_node(rest), n);
-  n = cons(opt, n);
+  /* Save original optional arguments before processing */
+  node *orig_opt = opt;
+
+  /* Process optional arguments (keep original side effects) */
   while (opt) {
     /* opt: (sym . (opt . lv)) -> (sym . opt) */
     local_add_lv(p, opt->car->cdr->cdr);
     opt->car->cdr = opt->car->cdr->car;
     opt = opt->cdr;
   }
-  return cons(m, n);
+
+  /* Allocate struct mrb_ast_args (no hdr) */
+  struct mrb_ast_args *args = (struct mrb_ast_args*)parser_palloc(p, sizeof(struct mrb_ast_args));
+
+  /* Initialize members */
+  args->mandatory_args = m;
+  args->optional_args = orig_opt;
+  args->rest_arg = rest;
+  args->post_mandatory_args = m2;
+
+  /* Deconstruct tail cons list: (kws . (kwrest . blk)) */
+  if (tail) {
+    args->keyword_args = (node*)tail->car;          /* kws */
+    args->kwrest_arg = (mrb_sym)(intptr_t)tail->cdr->car; /* kwrest */
+    args->block_arg = (mrb_sym)(intptr_t)tail->cdr->cdr;  /* blk */
+    cons_free(tail->cdr);
+    cons_free(tail);
+  }
+  else {
+    args->keyword_args = NULL;
+    args->kwrest_arg = 0;
+    args->block_arg = 0;
+  }
+
+  return (node*)args;
 }
 
 /* (:args_tail keywords rest_keywords_sym block_sym) */
 static node*
-new_args_tail(parser_state *p, node *kws, node *kwrest, mrb_sym blk)
+new_args_tail(parser_state *p, node *kws, mrb_sym kwrest, mrb_sym blk)
 {
   node *k;
 
-  if (kws || kwrest) {
-    local_add_kw(p, (kwrest && kwrest->cdr)? node_to_sym(kwrest->cdr) : 0);
+  if (kwrest) {
+    local_add_kw(p, kwrest);
   }
 
   local_add_blk(p);
@@ -1413,17 +1436,8 @@ new_args_tail(parser_state *p, node *kws, node *kwrest, mrb_sym blk)
     }
   }
 
-  size_t total_size = sizeof(struct mrb_ast_args_tail_node);
-  enum mrb_ast_size_class class = size_to_class(total_size);
-
-  struct mrb_ast_args_tail_node *n = (struct mrb_ast_args_tail_node*)parser_alloc_var(p, total_size, class);
-
-  init_var_header(&n->hdr, p, NODE_ARGS_TAIL, class);
-  n->keywords = kws;
-  n->kwrest = kwrest;
-  n->block = blk;
-
-  return cons_head((node*)NODE_VARIABLE, (node*)n);
+  /* Return cons list: (keyword . (kwrest . blk)) */
+  return cons(kws, cons(sym_to_node(kwrest), sym_to_node(blk)));
 }
 
 /* (:kw_arg kw_sym def_arg) */
@@ -1448,8 +1462,7 @@ new_args_dots(parser_state *p, node *m)
   mrb_sym k = intern_op(pow);
   mrb_sym b = intern_op(and);
   local_add_f(p, r);
-  return new_args(p, m, 0, r, 0,
-                  new_args_tail(p, 0, new_kw_rest_args(p, k), b));
+  return new_args(p, m, 0, r, 0, new_args_tail(p, NULL, k, b));
 }
 
 /* (:block_arg . a) */
@@ -1471,8 +1484,10 @@ setup_numparams(parser_state *p, node *a)
   if (nvars > 0) {
     int i;
     mrb_sym sym;
-    // m || opt || rest || tail
-    if (a && (a->car || (a->cdr && a->cdr->car) || (a->cdr->cdr && a->cdr->cdr->car) || (a->cdr->cdr->cdr->cdr && a->cdr->cdr->cdr->cdr->car))) {
+    // Check if any arguments are already defined
+    struct mrb_ast_args *args = (struct mrb_ast_args *)a;
+    if (a && (args->mandatory_args || args->optional_args || args->rest_arg ||
+              args->post_mandatory_args || args->keyword_args || args->kwrest_arg)) {
       yyerror(NULL, p, "ordinary parameter is defined");
     }
     else if (p->locals) {
@@ -1507,7 +1522,7 @@ new_block(parser_state *p, node *a, node *b)
 
   init_var_header(&n->hdr, p, NODE_BLOCK, class);
   n->locals = locals_node(p);
-  n->args = a;
+  n->args = (struct mrb_ast_args *)a;
   n->body = b;
 
   return cons_head((node*)NODE_VARIABLE, (node*)n);
@@ -1524,7 +1539,7 @@ new_lambda(parser_state *p, node *a, node *b)
   struct mrb_ast_lambda_node *lambda_node = (struct mrb_ast_lambda_node*)parser_alloc_var(p, total_size, class);
   init_var_header(&lambda_node->hdr, p, NODE_LAMBDA, class);
   lambda_node->locals = locals_node(p);
-  lambda_node->args = a;
+  lambda_node->args = (struct mrb_ast_args *)a;
   lambda_node->body = b;
   return cons_head((node*)NODE_VARIABLE, (node*)lambda_node);
 }
@@ -2339,9 +2354,9 @@ prohibit_literals(parser_state *p, node *n)
 %type <nd> heredoc words symbols
 %type <num> call_op call_op2     /* 0:'&.', 1:'.', 2:'::' */
 
-%type <nd> args_tail opt_args_tail f_kwarg f_kw f_kwrest
+%type <nd> args_tail opt_args_tail f_kwarg f_kw
 %type <nd> f_block_kwarg f_block_kw block_args_tail opt_block_args_tail
-%type <id> f_label
+%type <id> f_label f_kwrest
 
 %token tUPLUS             "unary plus"
 %token tUMINUS            "unary minus"
@@ -4532,11 +4547,11 @@ kwrest_mark     : tPOW
 
 f_kwrest        : kwrest_mark tIDENTIFIER
                     {
-                      $$ = new_kw_rest_args(p, $2);
+                      $$ = $2;
                     }
                 | kwrest_mark
                     {
-                      $$ = new_kw_rest_args(p, 0);
+                      $$ = intern_op(pow);
                     }
                 ;