mruby-compiler: simplify case statement implementation and fix infinite loop bug

- Remove obsolete NODE_ARGS_TAIL enum value and all references
- Simplify mrb_ast_case_node from variable-sized array back to simple cons-list structure
- Update new_case() function to use original cons-list approach instead of flattening
- Fix infinite loop in gen_case_var() when case statements have no matching clauses
- Improve code readability by renaming pos3 to case_end_jumps in gen_case_var()
- Restore memory-efficient case statement parsing without complex array management

The variable-sized array approach for case nodes provided no memory benefit
since cons lists aren't recycled. This change restores the simpler original
implementation while fixing a critical bug that caused mrbtest to hang
on "register window of calls" test.

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Yukihiro "Matz" Matsumoto
2025-09-16 14:54:43 +09:00
parent 8999442441
commit cbe01d574c
4 changed files with 1156 additions and 1228 deletions
+34 -39
View File
@@ -4103,17 +4103,15 @@ static void
gen_case_var(codegen_scope *s, node *varnode, int val)
{
struct mrb_ast_case_node *case_n = case_node(varnode);
node *value = CASE_NODE_VALUE(case_n);
node *else_body = CASE_NODE_ELSE(case_n);
uint16_t when_count = CASE_NODE_WHEN_COUNT(case_n);
struct mrb_ast_node **when_clauses = CASE_NODE_WHENS(case_n);
node *value = case_n->value;
node *body = case_n->body;
int head = 0;
uint32_t pos3, tmp;
uint32_t case_end_jumps, tmp;
uint32_t next_when_pos = JMPLINK_START;
node *n;
pos3 = JMPLINK_START;
case_end_jumps = JMPLINK_START;
/* Handle case value exactly like original */
if (value) {
@@ -4121,10 +4119,10 @@ gen_case_var(codegen_scope *s, node *varnode, int val)
codegen(s, value, VAL);
}
/* Iterate through when clauses array with JMPNOT optimization */
for (int i = 0; i < when_count; i++) {
node *when_clause = (node*)when_clauses[i];
if (!when_clause) continue;
/* Iterate through when clauses list with JMPNOT optimization */
node *current_when = body;
while (current_when) {
node *when_clause = current_when->car;
/* Dispatch previous when's "next" jump to this location */
if (next_when_pos != JMPLINK_START) {
@@ -4134,7 +4132,7 @@ gen_case_var(codegen_scope *s, node *varnode, int val)
/* when_clause is (condition . body) cons node */
node *args = when_clause->car; /* when conditions */
node *body = when_clause->cdr; /* when body */
node *when_body = when_clause->cdr; /* when body */
/* Process when conditions with JMPNOT optimization */
n = args;
@@ -4174,39 +4172,36 @@ gen_case_var(codegen_scope *s, node *varnode, int val)
dispatch_linked(s, condition_success_pos);
}
/* Generate when body inline - no extra JMP needed! */
codegen(s, body, val);
/* Generate when body */
codegen(s, when_body, val);
if (val) pop();
tmp = genjmp(s, OP_JMP, pos3);
pos3 = tmp;
/* Check if this is the last when clause before else, or if there's no else clause */
node *next_node = current_when->cdr;
tmp = genjmp(s, OP_JMP, case_end_jumps);
case_end_jumps = tmp;
current_when = next_node;
}
/* Dispatch final "next when" jump to else clause or nil handling */
/* Handle case where no else clause was found */
if (next_when_pos != JMPLINK_START) {
dispatch_linked(s, next_when_pos);
}
/* Handle else clause - DON'T generate unnecessary JMP */
if (else_body) {
codegen(s, else_body, val);
if (val) pop();
/* Only generate JMP if there will be more code after this (i.e., LOADNIL case) */
if (!val || !else_body) {
tmp = genjmp(s, OP_JMP, pos3);
pos3 = tmp;
}
}
/* Apply original's "nil-first, align-last" strategy for VAL case */
if (val) {
if (!else_body) {
/* Only generate LOADNIL if there's no else clause */
/* No else clause, generate LOADNIL for VAL case */
if (val) {
genop_1(s, OP_LOADNIL, cursp());
}
/* Always dispatch pos3 jumps */
if (pos3 != JMPLINK_START) dispatch_linked(s, pos3);
}
/* Apply stack management strategy for cases without else clause */
if (val) {
/* Dispatch remaining case_end_jumps */
if (case_end_jumps != JMPLINK_START) {
dispatch_linked(s, case_end_jumps);
}
if (head) {
/* This is the crucial MOVE instruction! Move result to original case value position */
/* Move result to original case value position */
gen_move(s, head, cursp(), 0);
pop();
}
@@ -4214,9 +4209,9 @@ gen_case_var(codegen_scope *s, node *varnode, int val)
push();
}
else {
/* NOVAL case - original logic unchanged */
if (pos3 != JMPLINK_START) {
dispatch_linked(s, pos3);
/* NOVAL case */
if (case_end_jumps != JMPLINK_START) {
dispatch_linked(s, case_end_jumps);
}
if (head) {
pop();
+5 -10
View File
@@ -302,14 +302,11 @@ struct mrb_ast_until_node {
struct mrb_ast_node *body; /* Loop body */
};
/* Variable-sized case node with variable when clauses */
/* Variable-sized case node - revert to original cons list approach */
struct mrb_ast_case_node {
struct mrb_ast_var_header header; /* 8 bytes */
struct mrb_ast_node *value; /* Case value expression */
uint16_t when_count; /* Number of when clauses */
uint16_t padding; /* Padding for alignment */
struct mrb_ast_node *else_body; /* Else clause (can be NULL) */
struct mrb_ast_node *when_clauses[1]; /* Variable array for when clauses */
struct mrb_ast_var_header header; /* 8 bytes */
struct mrb_ast_node *value; /* Case value expression */
struct mrb_ast_node *body; /* Original when/else body list */
};
/* Variable-sized for node */
@@ -469,9 +466,7 @@ struct mrb_ast_super_node {
#define UNTIL_NODE_BODY(n) (until_node(n)->body)
#define CASE_NODE_VALUE(n) (case_node(n)->value)
#define CASE_NODE_WHEN_COUNT(n) (case_node(n)->when_count)
#define CASE_NODE_ELSE(n) (case_node(n)->else_body)
#define CASE_NODE_WHENS(n) (case_node(n)->when_clauses)
#define CASE_NODE_BODY(n) (case_node(n)->body)
#define FOR_NODE_VAR(n) (for_node(n)->var)
#define FOR_NODE_ITERABLE(n) (for_node(n)->iterable)
+3 -34
View File
@@ -682,44 +682,13 @@ new_for(parser_state *p, node *v, node *o, node *b)
static node*
new_case(parser_state *p, node *a, node *b)
{
uint16_t when_count = 0;
node *else_body = NULL;
node *current_when = b;
void_expr_error(p, a);
// First pass: count when clauses and identify else_body
// The when_list is a linked list where each element's car is a (condition . body) cons node.
// The last element's car might be 0, and its cdr is the else_body.
while (current_when) {
node *clause = current_when->car;
if (clause && node_to_int(clause->car) == 0) { // This is the else clause
else_body = clause->cdr;
break; // Else body is always the last
}
when_count++;
current_when = current_when->cdr;
}
struct mrb_ast_case_node *n = (struct mrb_ast_case_node*)parser_alloc_var(p, sizeof(struct mrb_ast_case_node), SIZE_CLASS_MEDIUM);
size_t base_size = sizeof(struct mrb_ast_case_node);
size_t when_clauses_size = when_count * sizeof(struct mrb_ast_node*);
size_t total_size = base_size + when_clauses_size;
enum mrb_ast_size_class class = size_to_class(total_size);
struct mrb_ast_case_node *n = (struct mrb_ast_case_node*)parser_alloc_var(p, total_size, class);
init_var_header(&n->header, p, NODE_CASE, class);
init_var_header(&n->header, p, NODE_CASE, SIZE_CLASS_MEDIUM);
n->value = a;
n->when_count = when_count;
n->else_body = else_body;
// Second pass: copy when clauses into flexible array
current_when = b;
for (int i = 0; i < when_count; i++) {
n->when_clauses[i] = current_when->car; // Each car is a (condition . body) cons node
current_when = current_when->cdr;
}
n->body = b;
return cons_head((node*)NODE_VARIABLE, (node*)n);
}
File diff suppressed because it is too large Load Diff