Include callee name in "X is not a function" TypeError for global calls

[andreasrosdal]

Record the callee identifier in OP_get_var / OP_get_field2 via a new borrowed JSContext.error_callee_name slot and consume it in JS_ThrowTypeErrorNotAFunction, so calling a non-callable global or member reports e.g. "a is not a function" / "x is not a function" instead of a bare "not a function". The slot is cleared on bytecode-function entry, before invoking native callees, and after each call, so a name never leaks into an unrelated throw. Local/param callees remain unnamed (would require a new opcode that breaks precompiled bytecode). Addresses #1231.

[bnoordhuis]

This looks unsafe. On a scale of 1 to 10, how confident are you ctx->error_callee_name is never accessed after the atom is freed?

[andreasrosdal]

Thank you for reviewing. I have updated based on your feedback.

Note that this whole PR is the work of Claude, from the start ("find most high ROI issue to fix in quickjs"), to this PR, and the additional fix. I think hope the PR will add value to Quickjs, it seems like a good idea to me also.

[bnoordhuis]

Looks safer now. Can you run before/after benchmarks? I expect this to have measurable impact.

[andreasrosdal]

Performance findings: PR #1515 using Claude:

What the PR adds to hot paths. Every execution of OP_get_var/OP_get_var_undef (global reads) and OP_get_field2 (the obj.method() fetch) now runs an extra JS_FreeAtom(...) + JS_DupAtom(...) to record the callee name, plus a couple of JS_FreeAtom+null-out clears on each JS_CallInternal entry / call dispatch. For real name atoms (non-const) that's two rt->atom_array[] loads and two ref_count read-modify-write stores per opcode. So the theoretical concern is legitimate — it touches some of the busiest opcodes in the interpreter.

Measured impact (master vs PR, Release builds, interleaved runs). Times in ms, lower = better; I report the min of repeated runs since the machine is noisy:

┌────────────────────────────────────┬────────┬────────────┬───────────────────┐
│ Benchmark (patched opcode) │ Before │ After │ Δ │
├────────────────────────────────────┼────────┼────────────┼───────────────────┤
│ global_read (OP_get_var) │ ~688 │ ~689 │ ~0% │
├────────────────────────────────────┼────────┼────────────┼───────────────────┤
│ prop_read (OP_get_field2) │ ~933 │ ~956 │ ~+2% (noisy) │
├────────────────────────────────────┼────────┼────────────┼───────────────────┤
│ global_call (OP_get_var + call) │ ~1004 │ ~1015 │ ~+1% │
├────────────────────────────────────┼────────┼────────────┼───────────────────┤
│ method_call (OP_get_field2 + call) │ ~1054 │ ~1090–1112 │ ~+3–5% (noisiest) │
└────────────────────────────────────┴────────┴────────────┴───────────────────┘

Conclusion — smaller than expected. On the isolated opcode loops the regression is at most ~1–2%, and for global_read it's indistinguishable from noise. The reason: the added atom ref-count bump is cheap relative to what each opcode already does (the global-var hash lookup, the property shape walk, and especially the call itself). The largest signal (method_call, ~3–5%) is also the noisiest measurement on this box, so I wouldn't bank on the high end of that.

So the change is not free, but I could not reproduce a large, clear regression — it's low single-digit percent, mostly within run-to-run variance. The honest read is "measurable in theory, marginal in practice on this hardware."

Caveats:

• The machine has heterogeneous cores (pinning to core 2 was ~2× slower) and visible run-to-run variance of 15–20%, which limits
• The machine has heterogeneous cores (pinning to core 2 was ~2× slower) and visible run-to-run variance of 15–20%, which limits precision. A cleaner signal would need a quiet/pinned big core with CPU frequency locked, or perf stat instruction counts rather than wall-clock.
• I did not run the cause analysis through perf/instruction counts — if you want a more definitive number I can do that, and/or re-run on a specific core.

If you want to keep the feature but kill even the marginal cost: the atom only matters when the next thing is a call. The compiler knows that at codegen time (the OP_get_var/OP_get_field2 is followed by OP_call/OP_tail_call). Recording the name only for call-feeding loads — or stashing the atom value already in pc lazily in JS_ThrowTypeErrorNotAFunction instead of eagerly on every load — would remove the per-opcode ref-count churn entirely.

Closing.