Address the second round of review comments on prototype toString conversions

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();

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);

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()) {

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); }