From 9be866df0206f60f466670b5c86a217651b5d39c Mon Sep 17 00:00:00 2001 From: dchuk <38986+dchuk@users.noreply.github.com> Date: Sat, 10 Oct 2026 17:31:17 +0000 Subject: [PATCH 1/2] Compose boxed String builtin freshness with user returns Gate: green tree 38933e8f4081 master dacaa29cb84e (linux-x86_64 gcc-12.2.0) tests 6801/0 --- docs/limitations.md | 11 + src/analyze_share.c | 288 ++++++++++++++++-- src/call_plan.c | 13 + src/call_plan.h | 4 + ...xed_pure_user_arm_keeps_nested_argument.rb | 26 ++ ...user_arm_keeps_nested_argument.rb.expected | 1 + test/share/boxed_sub_fresh_with_user_arm.rb | 22 ++ .../boxed_sub_fresh_with_user_arm.rb.expected | 4 + test/share/boxed_sub_user_arms_run_both.rb | 65 ++++ .../boxed_sub_user_arms_run_both.rb.expected | 6 + 10 files changed, 418 insertions(+), 22 deletions(-) create mode 100644 test/share/boxed_pure_user_arm_keeps_nested_argument.rb create mode 100644 test/share/boxed_pure_user_arm_keeps_nested_argument.rb.expected create mode 100644 test/share/boxed_sub_fresh_with_user_arm.rb create mode 100644 test/share/boxed_sub_fresh_with_user_arm.rb.expected create mode 100644 test/share/boxed_sub_user_arms_run_both.rb create mode 100644 test/share/boxed_sub_user_arms_run_both.rb.expected diff --git a/docs/limitations.md b/docs/limitations.md index 7dc88dcd6c..ce2cdd533c 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -866,6 +866,17 @@ String-keyed, String-valued type. The promotion preserves the Hash and stored String identities through callers and ivars. RBS-pinned returns and methods with nonempty or alternate Hash producers keep their existing inferred types. +With `--share-strings`, a boxed call may also compose an exact String builtin +row with same-named user-method returns when the settled dispatch plan accounts +for both. For example, a blockless two-argument `String#sub` can return a fresh +String through its String arm while a same-named user method returns `nil`; +matched and unmatched substitutions both keep their String result separate +from the receiver. A builtin row's argument peeks also cannot discard values +retained by its same-named user arm. The analysis still joins every reachable +user return, so a user arm that returns an existing String retains that alias +and any required mutation refusal. Ambiguous builtin or dynamic dispatch +remains conservative. + Not yet shared: - String `self` handed to a mutating block through `yield` or `block.call` in the default build. Build with `--share-strings` to keep receiver mutations through this route. Read-only blocks and fresh String returns work in both builds; diff --git a/src/analyze_share.c b/src/analyze_share.c index 4243d62a00..00430b4627 100644 --- a/src/analyze_share.c +++ b/src/analyze_share.c @@ -54,6 +54,18 @@ enum { SHU_STMT = 1, SHU_TAIL = 2, SHU_SPLIT = 4, SHU_PEEK = 8 }; /* per-node answers built from the flows (ShareFacts.into) */ enum { SHI_INTO = 1, SHI_MUTATED = 2, SHI_LITOUT = 4 }; +/* The builtin side of a mixed boxed String call can be fresh even when its + joined user result is not. Keep the exact selected row and call-shape fact + from the share walk so the later return-tail proof can revalidate it without + replaying sh_builtin's effects. */ +typedef struct { + int call, share, family, container, recv_type, result_type, plan_ret; + int argc, ntg; + const BuiltinOp *row; + int *arg_types; + int *targets; +} ShBoxedFreshFact; + typedef struct ShareFacts { int n, cap; /* key: per element, the keys a Hash's lookups ask for, which its default @@ -145,6 +157,13 @@ typedef struct ShareFacts { new Strings (`s.scan(re)`, `s.split`), whose value is a class of its own (sh_builtin) */ unsigned char *fresh_cont; + /* mixed boxed-String calls whose selected builtin row actually answered + with no carried identity (`sh_builtin` returned -1), alongside the + selected row and input signature used to revalidate the proof */ + ShBoxedFreshFact *boxed_fresh; + int nboxed_fresh, cboxed_fresh; + /* lazily allocated per-node index into boxed_fresh (stored as index + 1) */ + int *boxed_fresh_index; /* `break v` and `next v` (sh_jumps): per node, the value the breaks out of a call's block or a loop hand the call or the loop, or the nexts of a block hand the block (-1 none); and the nodes the walk that finds @@ -1332,6 +1351,10 @@ static void sh_settle_peeks(ShareFacts *F, Compiler *c) { if (c->scopes[mi].def_node >= 0 && !F->mread[mi]) sh_mark_unused(F, nt, c->scopes[mi].body, SHU_TAIL); for (int i = 0; i < F->npk; i++) { int n = F->pk[i] < 0 ? -F->pk[i] - 1 : F->pk[i]; + /* A builtin's pure/argument peek describes only that dispatch arm. A + same-named user arm can retain these values, so its flows take + precedence over the builtin-only no-retention fact. */ + if (sh_has_targets(c, n)) continue; if (F->pk[i] < 0) { if (!F->unused[n]) continue; F->unused[n] |= SHU_PEEK; @@ -1681,6 +1704,224 @@ static int sh_container_default(ShareFacts *F, Compiler *c, int n, int rv, int b return sh_join(F, rv, sh_elem(F, rv)); } +/* The row the no-user boxed-call path already uses for a receiver whose + type is poly/unknown. Keep its precedence in one place so a user-target + call can compose that same row beside the user returns when its dispatch + plan proves that those are the only returning arms. */ +static int sh_boxed_builtin_row(ShareFacts *F, Compiler *c, int n, + int *share_out, int *family_out, int *container_out) { + const NodeTable *nt = c->nt; + const char *name = nt_str(nt, n, "name"); + int args = nt_ref(nt, n, "arguments"), argc = 0; + if (!name) return 0; + if (args >= 0) (void)nt_arr(nt, args, "arguments", &argc); + int blk = nt_ref(nt, n, "block"); + int s = 0, family = 0, container = 1; + /* A separator form of String#partition is the String row; Array's + block-form Enumerable row is a different call shape. */ + if (argc == 1 && blk < 0 && is_partition_family(name)) { + s = bop_share_boxed(TY_STRING, name); + if (s) family = TY_STRING; + } + if (!s) { s = bop_share_named(BOP_ANY_ARRAY, name); if (s) family = BOP_ANY_ARRAY; } + if (!s) { s = bop_share_named(BOP_ANY_HASH, name); if (s) family = BOP_ANY_HASH; } + if (!s) { s = bop_share_named(BOP_ANY_RECV, name); if (s) family = BOP_ANY_RECV; } + if (!s && !F->ostruct) { s = bop_share_named(TY_CLASS, name); if (s) family = TY_CLASS; } + /* A String-only operation answering a new String Array keeps that fact + even when its receiver is boxed: it is no container's row. */ + if (!s && !F->ostruct && blk < 0 && c->ntype[n] == TY_STR_ARRAY) { + const IterRow *ir = iter_row(TY_STRING, name, argc, 0); + if (ir && ir->nyield == 1 && ir->yield[0] == YS_FRESH && ir->answer == IA_RECV) { + s = BSH_PURE; family = TY_STRING; container = 0; + } + } + if (!s && !F->ostruct) { s = bop_share_boxed(TY_STRING, name); if (s) family = TY_STRING; } + /* Explicit IO rows describe boxed arms too. User targets and OpenStruct + fields stay above; an IO's wildcard cannot prove this. */ + if (!s && !F->ostruct) { s = bop_share_boxed(TY_IO, name); if (s) family = TY_IO; } + if (!s) return 0; + *share_out = s; + if (family_out) *family_out = family; + *container_out = container; + return 1; +} + +static int sh_plan_target_has(const int *tg, int ntg, int mi) { + for (int i = 0; i < ntg; i++) if (tg[i] == mi) return 1; + return 0; +} + +static int sh_bop_shape_fits(const BuiltinOp *op, const char *name, int argc, int has_block) { + if (!op || !op->name || !sp_streq(op->name, name) || argc < op->argc_min || argc > op->argc_max) return 0; + if ((op->block == BF_NONE && has_block) || (op->block == BF_REQUIRED && !has_block)) return 0; + return 1; +} + +typedef struct { + Compiler *c; + const int *argv; + int argc; +} ShBopArgs; + +static TyKind sh_bop_arg_type(const void *ud, int i) { + const ShBopArgs *a = ud; + return i >= 0 && i < a->argc ? a->c->ntype[a->argv[i]] : TY_UNKNOWN; +} + +/* A boxed String row can replace the container default only when the + selected typed row describes this call shape and the dispatch plan and + builtin ownership tables account for the other routes. The plan may offer + PT_STR directly, or pair PB_ND_GENERIC with PT_GENERIC_TAIL for the + generic builtin re-entry. Gather the plan facts before calling sh_builtin, + which can recurse through arguments and invalidate memoized plan storage. */ +static int sh_boxed_string_row_composable(ShareFacts *F, Compiler *c, int n, + int ntg, const int *tg, int share, + int family, int container, + const BuiltinOp **row_out, TyKind *plan_ret_out) { + const NodeTable *nt = c->nt; + const char *name = nt_str(nt, n, "name"); + int blk = nt_ref(nt, n, "block"); + int argc = 0, args = nt_ref(nt, n, "arguments"); + const int *argv = args >= 0 ? nt_arr(nt, args, "arguments", &argc) : NULL; + if (!name || family != TY_STRING || F->ostruct || blk >= 0 || + !poly_string_read_p(name) || + container != 1 || share != bop_share_boxed(TY_STRING, name) || + !nt_call_args_plain(nt, n)) return 0; + + /* String readers that are also a different builtin face need that face's + result too. The name-only reader table covers the boxed scalar/container + surfaces; the exact typed rows below cover the remaining BOP owners. */ + if (bop_name_has_reader(name, BOP_READ_NUMERIC | BOP_READ_CONTAINER)) return 0; + unsigned faces = ty_poly_face_owners(name, argc, 0, nt_call_args_plain(nt, n), 1); + if ((faces & PF_OWNERS) & ~PF_STRING) return 0; + ShBopArgs bop_args = { c, argv, argc }; + const BuiltinOp *op = bop_find_arg(TY_STRING, name, argc, 0, + sh_bop_arg_type, &bop_args); + if (!op || op->emit == BOPE_NONE || op->result != TY_STRING) return 0; + int has_other_builtin = 0; + for (int i = 0; i < bop_row_count(); i++) { + const BuiltinOp *row = bop_row(i); + if (row && row->recv != TY_STRING && row->recv != TY_STRBUF && + sh_bop_shape_fits(row, name, argc, 0)) { has_other_builtin = 1; break; } + } + if (has_other_builtin || bop_share_named(BOP_ANY_RECV, name)) return 0; + + /* Match the existing boxed-freshness exclusions for Object and dynamic + lookup. These routes can answer without being an ordinary user target. */ + if (object_public_method_name(name) || comp_method_index(c, "method_missing") >= 0) return 0; + int missing = 0; + comp_poly_candidates(c, "method_missing", &missing); + if (missing) return 0; + comp_cmethod_candidates(c, "method_missing", &missing); + if (missing) return 0; + + const PolyPlan *p = cplan_poly_fresh(c, n); + if (!p || (p->ret != TY_STRING && p->ret != TY_POLY)) return 0; + int has_str = 0, has_generic_tail = 0, has_generic_default = 0, ok = p->n > 0; + for (int i = 0; ok && i < p->n; i++) { + const PolyArm *a = &p->arm[i]; + if (a->kind == PA_USER || a->kind == PA_PROC_FORM) { + if (a->mi < 0 || !sh_plan_target_has(tg, ntg, a->mi)) ok = 0; + } + else if (a->kind == PA_ARITY) { + /* The arm raises ArgumentError and has no return value to join. */ + } + else if (a->kind == PA_TRIAL) { + if (a->key == PA_KEY_TRIAL + PT_STR) has_str = 1; + else if (a->key == PA_KEY_TRIAL + PT_GENERIC_TAIL) has_generic_tail = 1; + else ok = 0; + } + else if (a->kind == PA_BUILTIN) { + if (a->key == PA_KEY_BUILTIN + PB_ND_GENERIC) has_generic_default = 1; + else ok = 0; + } + else ok = 0; + } + int generic_tail = has_generic_tail && has_generic_default; + int partial_generic_tail = has_generic_tail != has_generic_default; + if (!ok || partial_generic_tail || (!has_str && !generic_tail)) return 0; + if (row_out) *row_out = op; + if (plan_ret_out) *plan_ret_out = p->ret; + return 1; +} + +static void sh_record_boxed_fresh(ShareFacts *F, Compiler *c, int n, + int share, int family, int container, + const BuiltinOp *row, TyKind plan_ret, + int ntg, const int *tg) { + const NodeTable *nt = c->nt; + if (n < 0 || n >= F->nnodes) return; + if (!F->boxed_fresh_index) { + F->boxed_fresh_index = calloc((size_t)(F->nnodes > 0 ? F->nnodes : 1), + sizeof(*F->boxed_fresh_index)); + if (!F->boxed_fresh_index) { fprintf(stderr, "spinel: out of memory\n"); exit(1); } + } + int args = nt_ref(nt, n, "arguments"), argc = 0; + const int *argv = args >= 0 ? nt_arr(nt, args, "arguments", &argc) : NULL; + int recv = nt_ref(nt, n, "receiver"); + if (F->nboxed_fresh >= F->cboxed_fresh) { + F->cboxed_fresh = F->cboxed_fresh ? F->cboxed_fresh * 2 : 8; + F->boxed_fresh = realloc(F->boxed_fresh, + sizeof(*F->boxed_fresh) * (size_t)F->cboxed_fresh); + if (!F->boxed_fresh) { fprintf(stderr, "spinel: out of memory\n"); exit(1); } + } + ShBoxedFreshFact fact = { + .call = n, .share = share, .family = family, .container = container, + .recv_type = recv >= 0 ? c->ntype[recv] : TY_UNKNOWN, + .result_type = c->ntype[n], .plan_ret = plan_ret, + .argc = argc, .ntg = ntg, .row = row + }; + fact.arg_types = argc ? malloc(sizeof(*fact.arg_types) * (size_t)argc) : NULL; + fact.targets = ntg ? malloc(sizeof(*fact.targets) * (size_t)ntg) : NULL; + if ((argc && !fact.arg_types) || (ntg && !fact.targets)) { + free(fact.arg_types); free(fact.targets); + fprintf(stderr, "spinel: out of memory\n"); exit(1); + } + for (int i = 0; i < argc; i++) fact.arg_types[i] = c->ntype[argv[i]]; + for (int i = 0; i < ntg; i++) fact.targets[i] = tg[i]; + int old = F->boxed_fresh_index[n] - 1; + if (old >= 0 && old < F->nboxed_fresh) { + free(F->boxed_fresh[old].arg_types); + free(F->boxed_fresh[old].targets); + F->boxed_fresh[old] = fact; + return; + } + int at = F->nboxed_fresh++; + F->boxed_fresh[at] = fact; + F->boxed_fresh_index[n] = at + 1; +} + +/* Revalidate the complete row/plan proof against the exact effect signature + captured by sh_builtin in the sharing walk. */ +static int sh_boxed_fresh_recorded(Compiler *c, int n) { + ShareFacts *F = c->share; + if (!F) return 0; + const ShBoxedFreshFact *fact = NULL; + int at = n >= 0 && n < F->nnodes && F->boxed_fresh_index + ? F->boxed_fresh_index[n] - 1 : -1; + if (at >= 0 && at < F->nboxed_fresh) fact = &F->boxed_fresh[at]; + if (!fact || fact->call != n) return 0; + + int tg[CPT_MAX], ntg = cplan_targets(c, n, tg, CPT_MAX); + int share = 0, family = 0, container = 1; + const BuiltinOp *row = NULL; + TyKind plan_ret = TY_UNKNOWN; + if (ntg < 0 || !sh_boxed_builtin_row(F, c, n, &share, &family, &container) || + !sh_boxed_string_row_composable(F, c, n, ntg, tg, share, family, container, + &row, &plan_ret)) return 0; + const NodeTable *nt = c->nt; + int args = nt_ref(nt, n, "arguments"), argc = 0; + const int *argv = args >= 0 ? nt_arr(nt, args, "arguments", &argc) : NULL; + int recv = nt_ref(nt, n, "receiver"); + int recv_type = recv >= 0 ? c->ntype[recv] : TY_UNKNOWN; + if (share != fact->share || family != fact->family || container != fact->container || + row != fact->row || plan_ret != fact->plan_ret || c->ntype[n] != fact->result_type || + recv_type != fact->recv_type || argc != fact->argc || ntg != fact->ntg) return 0; + for (int i = 0; i < argc; i++) if (c->ntype[argv[i]] != fact->arg_types[i]) return 0; + for (int i = 0; i < ntg; i++) if (tg[i] != fact->targets[i]) return 0; + return 1; +} + /* A call the walk does not follow: what it is handed and what it answers are UNKNOWN. */ static int sh_unknown_call(ShareFacts *F, Compiler *c, int n, int blk) { @@ -2306,6 +2547,17 @@ static int sh_call(ShareFacts *F, Compiler *c, int n) { if (argc == 2 && nt_kind(nt, argv[1]) == NK_KeywordHashNode && bop_share_named(BOP_ANY_ARRAY, name) == BSH_PACK) r = sh_join(F, r, sh_builtin(F, c, n, BSH_PACK, rv, blk, 1)); + int share = 0, family = 0, container = 1; + const BuiltinOp *row = NULL; + TyKind plan_ret = TY_UNKNOWN; + if (sh_boxed_builtin_row(F, c, n, &share, &family, &container) && + sh_boxed_string_row_composable(F, c, n, ntg, tg, share, family, container, + &row, &plan_ret)) { + int builtin = sh_builtin(F, c, n, share, rv, blk, container); + if (builtin == -1) + sh_record_boxed_fresh(F, c, n, share, family, container, row, plan_ret, ntg, tg); + return sh_join(F, r, builtin); + } return sh_join(F, r, sh_container_default(F, c, n, rv, blk)); } @@ -2389,27 +2641,9 @@ static int sh_call(ShareFacts *F, Compiler *c, int n) { } /* any receiver it may be: an Array's or a Hash's row (a String's keeps its arguments least), or the container default */ - /* The separator form is String's; Array partition takes a block. */ - int s = argc == 1 && blk < 0 && is_partition_family(name) - ? bop_share_boxed(TY_STRING, name) : 0; - if (!s) s = bop_share_named(BOP_ANY_ARRAY, name); - if (!s) s = bop_share_named(BOP_ANY_HASH, name); - if (!s) s = bop_share_named(BOP_ANY_RECV, name); - /* a Module's name (no OpenStruct field of the name in the program); a - name only a String answers, the String's row */ - if (!s && !F->ostruct) s = bop_share_named(TY_CLASS, name); - /* A String-only operation answering a new String Array keeps that - fact even when its receiver is boxed: it is no container's row. */ - if (!s && !F->ostruct && blk < 0 && c->ntype[n] == TY_STR_ARRAY) { - const IterRow *ir = iter_row(TY_STRING, name, argc, 0); - if (ir && ir->nyield == 1 && ir->yield[0] == YS_FRESH && ir->answer == IA_RECV) - return sh_builtin(F, c, n, BSH_PURE, rv, blk, 0); - } - if (!s && !F->ostruct) s = bop_share_boxed(TY_STRING, name); - /* Explicit IO rows describe the boxed arms too. User targets and - OpenStruct fields stay above; an IO's wildcard cannot prove this. */ - if (!s && !F->ostruct) s = bop_share_boxed(TY_IO, name); - if (s) return sh_builtin(F, c, n, s, rv, blk, 1); + int s = 0, container = 1; + if (sh_boxed_builtin_row(F, c, n, &s, NULL, &container)) + return sh_builtin(F, c, n, s, rv, blk, container); return sh_container_default(F, c, n, rv, blk); } if (rt == TY_OPENSTRUCT) return sh_ostruct_call(F, c, n, name, rv, blk); @@ -3160,6 +3394,12 @@ static void sh_free(ShareFacts *F) { free(F->any_new_blk); free(F->attr_r); free(F->attr_w); free(F->blkp); free(F->jump); free(F->jseen); free(F->fgen); free(F->key); free(F->lk_c); free(F->lk_k); free(F->lk_done); + for (int i = 0; i < F->nboxed_fresh; i++) { + free(F->boxed_fresh[i].arg_types); + free(F->boxed_fresh[i].targets); + } + free(F->boxed_fresh); + free(F->boxed_fresh_index); free(F); } @@ -3965,13 +4205,17 @@ static int sh_arm_io_read_fresh(Compiler *c, int call, const PolyArm *a) { int share_builtin_fresh(Compiler *c, int call) { const char *name = nt_str(c->nt, call, "name"); if (bop_share_named(BOP_ANY_RECV, name) == BSH_PURE && !is_receiver_conversion(name)) return 1; + /* A mixed boxed call may have a fresh builtin String arm beside user + returns. Use only the effect recorded after sh_builtin actually returned + no carried identity, with the same row and full plan proof revalidated. */ + if (sh_boxed_fresh_recorded(c, call)) return 1; /* The share walk's builtin surfaces exclude typed-only methods such as Thread#value; the settled plan below still checks every arm. */ if (!sh_builtin_fresh(c, call, !c->share || c->share->ostruct)) return 0; /* A scope-name lookup can miss an alias's method. Every returning user arm must occur in the target set whose return identities we check. */ int tg[CPT_MAX], n = cplan_targets(c, call, tg, CPT_MAX); - const PolyPlan *p = cplan_poly(c, call); + const PolyPlan *p = cplan_poly_fresh(c, call); for (int i = 0; i < p->n; i++) { const PolyArm *a = &p->arm[i]; if (a->kind != PA_USER && a->kind != PA_PROC_FORM) continue; diff --git a/src/call_plan.c b/src/call_plan.c index cd7e067c1d..31d1f87846 100644 --- a/src/call_plan.c +++ b/src/call_plan.c @@ -1897,6 +1897,19 @@ static const PolyPlan *cplan_poly_level(Compiler *c, int id, int full) { return p; } +const PolyPlan *cplan_poly_fresh(Compiler *c, int id) { + static PolyPlan fresh; + if (id < 0 || id >= c->node_cap) { free(fresh.arm); fresh.arm = NULL; fresh.n = 0; return &fresh; } +#ifndef NDEBUG + int tmp0 = g_tmp; +#endif + cpoly_resolve(c, id, &fresh, 1); +#ifndef NDEBUG + assert(g_tmp == tmp0); /* resolving emits nothing */ +#endif + return &fresh; +} + const PolyPlan *cplan_poly_block(Compiler *c, int id) { static PolyPlan bp; free(bp.arm); diff --git a/src/call_plan.h b/src/call_plan.h index d5b6f7ffb4..417ac6c616 100644 --- a/src/call_plan.h +++ b/src/call_plan.h @@ -290,6 +290,10 @@ const PolyPlan *cplan_poly(Compiler *c, int id); receiver form). Cheaper: the builtin families and trials, which only the --plan-check shadow compares, are left out. */ const PolyPlan *cplan_poly_arms(Compiler *c, int id); +/* Resolve the complete plan without reading or writing the per-node memo. + Use while inference facts are still moving; the scratch plan is overwritten + by the next fresh resolve, so finish reading it before another planner call. */ +const PolyPlan *cplan_poly_fresh(Compiler *c, int id); int cplan_struct_aset(Compiler *c, int cid, const char *name, int argc); /* A plan the caller keeps across emissions that may resolve others (a resolve outside the memo reuses one buffer); cplan_poly_free drops it. */ diff --git a/test/share/boxed_pure_user_arm_keeps_nested_argument.rb b/test/share/boxed_pure_user_arm_keeps_nested_argument.rb new file mode 100644 index 0000000000..28747e45de --- /dev/null +++ b/test/share/boxed_pure_user_arm_keeps_nested_argument.rb @@ -0,0 +1,26 @@ +# String#delete_prefix has a pure boxed String row. This mixed call's user +# arm keeps the Array argument and mutates its nested String after the call. +# The zero-argument test run selects Params while retaining String in the +# receiver expression, so the boxed builtin peek must not erase that flow. +# spinel: share +class Params + def delete_prefix(prefix) + @saved_prefix = prefix + nil + end + + def saved_prefix + @saved_prefix + end +end + +params = Params.new +source = String.new("captured") +receiver = [params, String.new("unused")][ARGV.length] +receiver.delete_prefix([source]) +if ARGV.empty? + params.saved_prefix[0] << "!" + puts source +else + puts "string" +end diff --git a/test/share/boxed_pure_user_arm_keeps_nested_argument.rb.expected b/test/share/boxed_pure_user_arm_keeps_nested_argument.rb.expected new file mode 100644 index 0000000000..1667b7bb82 --- /dev/null +++ b/test/share/boxed_pure_user_arm_keeps_nested_argument.rb.expected @@ -0,0 +1 @@ +captured! diff --git a/test/share/boxed_sub_fresh_with_user_arm.rb b/test/share/boxed_sub_fresh_with_user_arm.rb new file mode 100644 index 0000000000..8690a013b3 --- /dev/null +++ b/test/share/boxed_sub_fresh_with_user_arm.rb @@ -0,0 +1,22 @@ +# A same-named but unreachable user method stays in the program while the +# String receiver's sub result is used. The unrelated Thread.value mutation +# must still see the shared String, and a non-matching sub must remain fresh. +# spinel: share +module Params + def self.sub(a, b) + nil + end +end + +protocol = Thread.new { "https://" }.value.sub("://", "") +trigger = String.new("trigger") +Thread.new { trigger }.value << "-after" + +no_match_source = Thread.new { String.new("plain") }.value +no_match = no_match_source.sub("://", "!") +no_match << "!" + +puts protocol +puts trigger +puts no_match_source +puts no_match diff --git a/test/share/boxed_sub_fresh_with_user_arm.rb.expected b/test/share/boxed_sub_fresh_with_user_arm.rb.expected new file mode 100644 index 0000000000..29f9cf25fe --- /dev/null +++ b/test/share/boxed_sub_fresh_with_user_arm.rb.expected @@ -0,0 +1,4 @@ +https +trigger-after +plain +plain! diff --git a/test/share/boxed_sub_user_arms_run_both.rb b/test/share/boxed_sub_user_arms_run_both.rb new file mode 100644 index 0000000000..04077c5fb1 --- /dev/null +++ b/test/share/boxed_sub_user_arms_run_both.rb @@ -0,0 +1,65 @@ +# One call site runs both String#sub and same-named user methods. Its user +# arms return nil, a fresh String, a borrowed String, or a container holding +# a borrowed String; those return facts must remain intact beside the builtin. +# spinel: share +class ParamsNil + def sub(pattern, replacement) + nil + end +end + +class ParamsFresh + def sub(pattern, replacement) + String.new("fresh") + end +end + +class ParamsBorrowed + attr_reader :returned + + def initialize + @returned = String.new("borrowed") + end + + def sub(pattern, replacement) + @returned + end +end + +class ParamsContainer + attr_reader :returned + + def initialize + @returned = String.new("container") + end + + def sub(pattern, replacement) + [@returned] + end +end + +def invoke_sub(receiver) + receiver.sub("://", "!") +end + +source = Thread.new { "https://" }.value +string_result = invoke_sub(source) +string_result << "!" +puts source +puts string_result + +puts invoke_sub(ParamsNil.new).nil? + +fresh_result = invoke_sub(ParamsFresh.new) +fresh_result << "!" +puts fresh_result + +borrowed = ParamsBorrowed.new +borrowed_result = invoke_sub(borrowed) +borrowed_result << "!" +puts borrowed.returned + +container = ParamsContainer.new +container_result = invoke_sub(container) +container_result[0] << "!" +puts container.returned diff --git a/test/share/boxed_sub_user_arms_run_both.rb.expected b/test/share/boxed_sub_user_arms_run_both.rb.expected new file mode 100644 index 0000000000..d0d999f595 --- /dev/null +++ b/test/share/boxed_sub_user_arms_run_both.rb.expected @@ -0,0 +1,6 @@ +https:// +https!! +true +fresh! +borrowed! +container! From f2c32c866942562d8f4627c08176b2c4fb9dc545 Mon Sep 17 00:00:00 2001 From: dchuk <38986+dchuk@users.noreply.github.com> Date: Sat, 10 Oct 2026 21:16:56 +0000 Subject: [PATCH 2/2] Keep boxed String composition conservative around singleton accessors Singleton accessor publication is not fully represented in the sharing holder graph. Replacing the generic container effects could lose an alias returned directly by a class method or indirectly through an instance arm. Keep the previous effects whenever the program declares singleton readers or writers; explicit class-method writes remain eligible. Add direct and transitive identity regressions and an explicit-write control. --- docs/limitations.md | 6 +++- src/analyze_share.c | 22 +++++++++--- ..._class_value_return_keeps_nested_string.rb | 34 ++++++++++++++++++ ...lue_return_keeps_nested_string.rb.expected | 2 ++ ...tive_class_accessor_keeps_nested_string.rb | 33 +++++++++++++++++ ...s_accessor_keeps_nested_string.rb.expected | 1 + ...explicit_class_ivar_keeps_nested_string.rb | 35 +++++++++++++++++++ ...class_ivar_keeps_nested_string.rb.expected | 1 + 8 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 test/share/boxed_sub_class_value_return_keeps_nested_string.rb create mode 100644 test/share/boxed_sub_class_value_return_keeps_nested_string.rb.expected create mode 100644 test/share/boxed_sub_transitive_class_accessor_keeps_nested_string.rb create mode 100644 test/share/boxed_sub_transitive_class_accessor_keeps_nested_string.rb.expected create mode 100644 test/share/boxed_sub_transitive_explicit_class_ivar_keeps_nested_string.rb create mode 100644 test/share/boxed_sub_transitive_explicit_class_ivar_keeps_nested_string.rb.expected diff --git a/docs/limitations.md b/docs/limitations.md index ce2cdd533c..a39fdfd19e 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -875,7 +875,11 @@ from the receiver. A builtin row's argument peeks also cannot discard values retained by its same-named user arm. The analysis still joins every reachable user return, so a user arm that returns an existing String retains that alias and any required mutation refusal. Ambiguous builtin or dynamic dispatch -remains conservative. +remains conservative. If the program declares singleton readers or writers, +this composition precision is also disabled: a user method can reach class-side +publication through a helper even when that accessor is absent from the call's +dispatch plan, and those holder edges are not fully represented here. Such +programs retain the previous boxed-container effects. Not yet shared: diff --git a/src/analyze_share.c b/src/analyze_share.c index 3d970d5782..1767bdae2b 100644 --- a/src/analyze_share.c +++ b/src/analyze_share.c @@ -194,6 +194,8 @@ typedef struct ShareFacts { int exc; int ostruct; /* the program names OpenStruct, whose fields a poly receiver's call may read */ + int singleton_accessors; /* some class has singleton readers/writers whose + class-side holder edges are not in this graph */ int closed; /* unions with UNKNOWN are dropped (the stats' second build) */ int union_stack_cap; int *union_stack; @@ -1783,7 +1785,9 @@ static int sh_boxed_string_row_composable(ShareFacts *F, Compiler *c, int n, int blk = nt_ref(nt, n, "block"); int argc = 0, args = nt_ref(nt, n, "arguments"); const int *argv = args >= 0 ? nt_arr(nt, args, "arguments", &argc) : NULL; - if (!name || family != TY_STRING || F->ostruct || blk >= 0 || + /* A user arm may reach a helper that reads a published singleton-accessor + value even when that accessor is not in this call's dispatch plan. */ + if (!name || family != TY_STRING || F->ostruct || F->singleton_accessors || blk >= 0 || !poly_string_read_p(name) || container != 1 || share != bop_share_boxed(TY_STRING, name) || !nt_call_args_plain(nt, n)) return 0; @@ -1821,7 +1825,7 @@ static int sh_boxed_string_row_composable(ShareFacts *F, Compiler *c, int n, for (int i = 0; ok && i < p->n; i++) { const PolyArm *a = &p->arm[i]; if (a->kind == PA_USER || a->kind == PA_PROC_FORM) { - if (a->mi < 0 || !sh_plan_target_has(tg, ntg, a->mi)) ok = 0; + if (a->mi < 0 || a->mi >= c->nscopes || !sh_plan_target_has(tg, ntg, a->mi)) ok = 0; } else if (a->kind == PA_ARITY) { /* The arm raises ArgumentError and has no return value to join. */ @@ -1891,8 +1895,8 @@ static void sh_record_boxed_fresh(ShareFacts *F, Compiler *c, int n, F->boxed_fresh_index[n] = at + 1; } -/* Revalidate the complete row/plan proof against the exact effect signature - captured by sh_builtin in the sharing walk. */ +/* Revalidate the selected builtin row and dispatch shape against the exact + effect signature captured by sh_builtin in the sharing walk. */ static int sh_boxed_fresh_recorded(Compiler *c, int n) { ShareFacts *F = c->share; if (!F) return 0; @@ -2554,6 +2558,8 @@ static int sh_call(ShareFacts *F, Compiler *c, int n) { sh_boxed_string_row_composable(F, c, n, ntg, tg, share, family, container, &row, &plan_ret)) { int builtin = sh_builtin(F, c, n, share, rv, blk, container); + /* This records only the builtin arm's lack of carried identity; r + still contains every reachable user arm's return holder. */ if (builtin == -1) sh_record_boxed_fresh(F, c, n, share, family, container, row, plan_ret, ntg, tg); return sh_join(F, r, builtin); @@ -3823,6 +3829,14 @@ static ShareFacts *sh_build(Compiler *c, int closed) { F->closed = closed; F->unknown = sh_new(F, SHK_UNKNOWN); F->exc = -1; + for (int ci = 0; ci < c->nclasses; ci++) + if (c->classes[ci].nsg_readers || c->classes[ci].nsg_writers) { + /* Singleton accessors can publish values through class-side storage + that the share holder graph does not connect to those readers. Keep + the previous boxed-container effects for this program. */ + F->singleton_accessors = 1; + break; + } NT_FOREACH_KIND(nt, NK_ConstantReadNode, cr) if (!F->ostruct && nt_str(nt, cr, "name") && sp_streq(nt_str(nt, cr, "name"), "OpenStruct")) F->ostruct = 1; F->flags[F->unknown] = SHF_UNKNOWN; diff --git a/test/share/boxed_sub_class_value_return_keeps_nested_string.rb b/test/share/boxed_sub_class_value_return_keeps_nested_string.rb new file mode 100644 index 0000000000..196ab63031 --- /dev/null +++ b/test/share/boxed_sub_class_value_return_keeps_nested_string.rb @@ -0,0 +1,34 @@ +# spinel: share +# A boxed String/class/instance receiver union must retain a nested String +# returned from a reachable class-value Hash arm. +class Params + class << self + attr_accessor :payload + + def sub(pattern, replacement) + { "payload" => @payload } + end + end +end + +class OtherArm + def sub(pattern, replacement) + nil + end +end + +source = String.new("https://") +Params.payload = source +seed = Params.sub("seed", "") +puts "seed_payload_is_source=#{seed.fetch("payload").equal?(source)}" + +[source, Params, OtherArm.new].each do |receiver| + result = receiver.sub("://", "") + if result.is_a?(Hash) + nested = result.fetch("payload") + puts "nested_is_source=#{nested.equal?(source)}" + nested << "!" + elsif result.is_a?(String) + result << "!" + end +end diff --git a/test/share/boxed_sub_class_value_return_keeps_nested_string.rb.expected b/test/share/boxed_sub_class_value_return_keeps_nested_string.rb.expected new file mode 100644 index 0000000000..53279cf868 --- /dev/null +++ b/test/share/boxed_sub_class_value_return_keeps_nested_string.rb.expected @@ -0,0 +1,2 @@ +seed_payload_is_source=true +nested_is_source=true diff --git a/test/share/boxed_sub_transitive_class_accessor_keeps_nested_string.rb b/test/share/boxed_sub_transitive_class_accessor_keeps_nested_string.rb new file mode 100644 index 0000000000..ff440a5936 --- /dev/null +++ b/test/share/boxed_sub_transitive_class_accessor_keeps_nested_string.rb @@ -0,0 +1,33 @@ +# spinel: share +# The outer boxed call has only String and instance-method arms. The instance +# arm returns a Hash from a class helper that reads a published class ivar; +# Payload intentionally defines no class-side `sub` method. +class Payload + class << self + attr_accessor :payload + + def box + { "payload" => @payload } + end + end +end + +class OtherArm + def sub(pattern, replacement) + Payload.box + end +end + +source = String.new("https://") +Payload.payload = source + +[source, OtherArm.new].each do |receiver| + result = receiver.sub("://", "") + if result.is_a?(Hash) + nested = result.fetch("payload") + puts "nested_is_source=#{nested.equal?(source)}" + nested << "!" + elsif result.is_a?(String) + result << "!" + end +end diff --git a/test/share/boxed_sub_transitive_class_accessor_keeps_nested_string.rb.expected b/test/share/boxed_sub_transitive_class_accessor_keeps_nested_string.rb.expected new file mode 100644 index 0000000000..4a6dc09ea6 --- /dev/null +++ b/test/share/boxed_sub_transitive_class_accessor_keeps_nested_string.rb.expected @@ -0,0 +1 @@ +nested_is_source=true diff --git a/test/share/boxed_sub_transitive_explicit_class_ivar_keeps_nested_string.rb b/test/share/boxed_sub_transitive_explicit_class_ivar_keeps_nested_string.rb new file mode 100644 index 0000000000..c40ef34965 --- /dev/null +++ b/test/share/boxed_sub_transitive_explicit_class_ivar_keeps_nested_string.rb @@ -0,0 +1,35 @@ +# spinel: share +# The outer boxed call has only String and instance-method arms. A helper +# publishes its class ivar through an explicit class method, without readers +# or writers generated by attr_accessor. +class Payload + class << self + def put(value) + @payload = value + end + + def box + { "payload" => @payload } + end + end +end + +class OtherArm + def sub(pattern, replacement) + Payload.box + end +end + +source = String.new("https://") +Payload.put(source) + +[source, OtherArm.new].each do |receiver| + result = receiver.sub("://", "") + if result.is_a?(Hash) + nested = result.fetch("payload") + puts "nested_is_source=#{nested.equal?(source)}" + nested << "!" + elsif result.is_a?(String) + result << "!" + end +end diff --git a/test/share/boxed_sub_transitive_explicit_class_ivar_keeps_nested_string.rb.expected b/test/share/boxed_sub_transitive_explicit_class_ivar_keeps_nested_string.rb.expected new file mode 100644 index 0000000000..4a6dc09ea6 --- /dev/null +++ b/test/share/boxed_sub_transitive_explicit_class_ivar_keeps_nested_string.rb.expected @@ -0,0 +1 @@ +nested_is_source=true