From 0c25c0d95f7f22fb855bcfb64d5344198ea36414 Mon Sep 17 00:00:00 2001 From: "Yukihiro \"Matz\" Matsumoto" Date: Thu, 25 Sep 2025 18:29:51 +0900 Subject: [PATCH] mruby-compiler: refactor gen_string to use single loop Eliminate code duplication in gen_string by using a single loop with a first-element flag instead of separate first element processing. The previous structure had ~20 lines of duplicated string literal and expression processing logic. The refactored version uses a unified loop that handles concatenation only for non-first elements, reducing code duplication and improving maintainability. Functionality remains identical - all string interpolation, regex patterns, and heredoc processing work correctly. Co-authored-by: Claude --- mrbgems/mruby-compiler/core/codegen.c | 83 ++++++++------------------- 1 file changed, 24 insertions(+), 59 deletions(-) diff --git a/mrbgems/mruby-compiler/core/codegen.c b/mrbgems/mruby-compiler/core/codegen.c index e706fe8d9..f31d0cc17 100644 --- a/mrbgems/mruby-compiler/core/codegen.c +++ b/mrbgems/mruby-compiler/core/codegen.c @@ -3276,79 +3276,47 @@ gen_hash_var(codegen_scope *s, node *varnode, int val) static void gen_string(codegen_scope *s, node *list, int val) { - if (!list) { - if (val) { - gen_load_nil(s, 1); - } - return; - } - if (val) { /* Handle as cons list of string parts with safety checks */ node *n = list; + mrb_bool first = TRUE; - /* Generate first element */ - node *elem = n->car; - if (!elem) { - gen_load_nil(s, 1); - return; - } - - mrb_int len = node_to_int(elem->car); - - if (len >= 0) { - /* String literal: (len . str) */ - char *str = (char*)elem->cdr; - if (str) { - int off = new_lit_str(s, str, len); - genop_2(s, OP_STRING, cursp(), off); - push(); - } - else { - /* Handle null string */ - int off = new_lit_str(s, "", 0); - genop_2(s, OP_STRING, cursp(), off); - push(); - } - } - else { - /* Expression: (-1 . node) */ - codegen(s, (node*)elem->cdr, VAL); - } - - /* Concatenate remaining elements */ - n = n->cdr; while (n) { - elem = n->car; + node *elem = n->car; if (!elem) break; - len = node_to_int(elem->car); + mrb_int len = node_to_int(elem->car); if (len >= 0) { /* String literal: (len . str) */ - char *str = (char*)elem->cdr; - if (str) { - int off = new_lit_str(s, str, len); - genop_2(s, OP_STRING, cursp(), off); - push(); - } - else { - /* Handle null string */ - int off = new_lit_str(s, "", 0); - genop_2(s, OP_STRING, cursp(), off); - push(); - } + const char *str = (char*)elem->cdr; + if (!str) {str = ""; len = 0;} + int off = new_lit_str(s, str, len); + genop_2(s, OP_STRING, cursp(), off); + push(); } else { /* Expression: (-1 . node) */ codegen(s, (node*)elem->cdr, VAL); } - pop(); pop(); - genop_1(s, OP_STRCAT, cursp()); - push(); + /* Concatenate with previous parts (except for first element) */ + if (!first) { + pop(); pop(); + genop_1(s, OP_STRCAT, cursp()); + push(); + } + else { + first = FALSE; + } + n = n->cdr; } + + /* Handle empty list case */ + if (first) { + gen_load_nil(s, 1); + } } else { /* NOVAL case: only evaluate expressions for side effects */ @@ -3356,10 +3324,7 @@ gen_string(codegen_scope *s, node *list, int val) while (n) { node *elem = n->car; if (!elem) break; - - mrb_int len = node_to_int(elem->car); - - if (len < 0) { + if (node_to_int(elem->car) < 0) { /* Expression: (-1 . node) - evaluate for side effects */ codegen(s, (node*)elem->cdr, NOVAL); }