From 04435a70ac2b4c6b3b60336b86fffef39e86587d Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Wed, 21 Oct 2026 11:44:13 -0500 Subject: [PATCH] Address the second round of review comments on prototype toString conversions --- packages/compiler/src/backend/llvm/dyn.ts | 36 ++++++++++--------- packages/runtime/src/scr_dyn_invoke.c | 9 +++-- packages/runtime/src/scr_json.c | 4 +++ .../corpus/prototype-tostring-conversions.cjs | 12 +++++++ 4 files changed, 43 insertions(+), 18 deletions(-) diff --git a/packages/compiler/src/backend/llvm/dyn.ts b/packages/compiler/src/backend/llvm/dyn.ts index ff975a800..6911ef98b 100644 --- a/packages/compiler/src/backend/llvm/dyn.ts +++ b/packages/compiler/src/backend/llvm/dyn.ts @@ -3362,18 +3362,22 @@ export class LlDyn { B.line(`call void @scr_jb_putc(ptr %b, i8 44)`); B.br(lel); B.startBlock(lel); - // An element's toString can append to, shrink or reallocate the - // array: each index reads the current items, an index past the - // current length prints empty, and the element stays retained - // while it converts. - const now = this.lenOf(B, "%d"); - const inside = B.tmp(); - B.line(`${inside} = icmp ult ${this.S} ${i}, ${now}`); - const lIn = B.newLabel("ds.in"); - const lSkip = B.newLabel("ds.sk"); - B.condBr(inside, lIn, lSkip); - B.startBlock(lIn); - const e = this.itemAt(B, this.itemsOf(B, "%d"), i); + // Each index reads the element as join does (scr_dyn_arr_at, +1): + // from the current storage, undefined past the current length, + // and through the prototype for a hole. An element's toString + // can append to, shrink or reallocate the array; a getter that + // throws answers NULL with the exception pending. + host.declare(`declare ptr @scr_dyn_arr_at(ptr, double)`); + host.declare(`declare void @scr_dyn_release_v(ptr)`); + const index = B.tmp(); + B.line(`${index} = uitofp ${this.S} ${i} to double`); + const e = B.tmp(); + B.line(`${e} = call ptr @scr_dyn_arr_at(ptr %d, double ${index})`); + const missing = B.tmp(); + B.line(`${missing} = icmp eq ptr ${e}, null`); + const lGot = B.newLabel("ds.gt"); + B.condBr(missing, done, lGot); + B.startBlock(lGot); const ek = this.kindOf(B, e); const isU = B.tmp(); const isN = B.tmp(); @@ -3382,14 +3386,14 @@ export class LlDyn { B.line(`${isN} = icmp eq i32 ${ek}, ${DYN_KIND.NULL}`); B.line(`${unit} = or i1 ${isU}, ${isN}`); const lRec = B.newLabel("ds.rc"); + const lSkip = B.newLabel("ds.sk"); B.condBr(unit, lSkip, lRec); B.startBlock(lSkip); + B.line(`call void @scr_dyn_release_v(ptr ${e})`); brNext(); B.startBlock(lRec); - const held = this.retainDyn(B, e); - host.declare(`declare void @scr_dyn_release_v(ptr)`); - B.line(`call void @sc_ds_buf(ptr %b, ptr ${held})`); - B.line(`call void @scr_dyn_release_v(ptr ${held})`); + B.line(`call void @sc_ds_buf(ptr %b, ptr ${e})`); + B.line(`call void @scr_dyn_release_v(ptr ${e})`); // The first exception an element's conversion throws ends the // conversion; later elements are not read. const thrown = B.tmp(); diff --git a/packages/runtime/src/scr_dyn_invoke.c b/packages/runtime/src/scr_dyn_invoke.c index e413acfe9..ffe1d0330 100644 --- a/packages/runtime/src/scr_dyn_invoke.c +++ b/packages/runtime/src/scr_dyn_invoke.c @@ -1215,7 +1215,7 @@ static ScrDyn *scr_dyn_invoke_impl( scr_jb_init(&b); ScrStr *separator = argc > 0 && args[0]->kind != SCR_DYN_UNDEF ? scr_dyn_string_coerce_js(args[0]) : scr_str_new(",", 1); - if (!separator) { free(b.data); return NULL; } + if (!separator) return NULL; /* nothing written yet */ for (size_t i = 0; i < len; i++) { if (i > 0) scr_jb_write(&b, separator->data, separator->len); ScrDyn *e = scr_dyn_arr_at(recv, (double)i); @@ -1223,7 +1223,12 @@ static ScrDyn *scr_dyn_invoke_impl( if (e->kind != SCR_DYN_UNDEF && e->kind != SCR_DYN_NULL) scr_dyn_display_buf(&b, e, true); scr_dyn_release(e); } - if (!e || scr_exc_pending()) { scr_str_release(separator); free(b.data); return NULL; } + if (!e || scr_exc_pending()) { + /* The buffer's bytes live in a string allocation: discard it as one. */ + scr_str_release(scr_jb_finish(&b)); + scr_str_release(separator); + return NULL; + } } scr_str_release(separator); ScrStr *joined = scr_jb_finish(&b); diff --git a/packages/runtime/src/scr_json.c b/packages/runtime/src/scr_json.c index 13fc73187..a9acb8f53 100644 --- a/packages/runtime/src/scr_json.c +++ b/packages/runtime/src/scr_json.c @@ -7507,6 +7507,10 @@ ScrStr *scr_dyn_to_string(const ScrDyn *d, const ScrStr *enc) { out = joined; } ScrDyn *e = scr_dyn_arr_at(d, (double)i); + if (!e) { /* an inherited getter threw: the exception is pending */ + scr_str_release(out); + return NULL; + } if (e->kind == SCR_DYN_UNDEF || e->kind == SCR_DYN_NULL) { /* JS join: empty */ scr_dyn_release(e); if (scr_exc_pending()) { diff --git a/tests/corpus/prototype-tostring-conversions.cjs b/tests/corpus/prototype-tostring-conversions.cjs index a5c6ba83c..562f1ae9a 100644 --- a/tests/corpus/prototype-tostring-conversions.cjs +++ b/tests/corpus/prototype-tostring-conversions.cjs @@ -64,3 +64,15 @@ try { console.log(`${stops}`); } catch (e) { console.log("template", e.message, try { console.log(stops.join()); } catch (e) { console.log("join", e.message, visited.length); } try { console.log(String(stops)); } catch (e) { console.log("String", e.message, visited.length); } try { console.log(`${[stops]}`); } catch (e) { console.log("nested", e.message, visited.length); } + +// A hole reads through the prototype: a getter there that throws ends each +// conversion with its exception. +const holeyProto = Object.create(Array.prototype); +Object.defineProperty(holeyProto, '1', { get() { throw new Error('getter'); } }); +const holey = []; +holey[0] = 'a'; +holey[2] = 'c'; +Object.setPrototypeOf(holey, holeyProto); +try { console.log(String(holey)); } catch (e) { console.log('String', e.message); } +try { console.log(holey.join('|')); } catch (e) { console.log('join', e.message); } +try { console.log(`${holey}`); } catch (e) { console.log('template', e.message); }