Repository navigation
A multi-line binary operator's runtime error reports the operator's line (#1425) - #1444
InauguralPhysicist wants to merge 4 commits into
Conversation
…ine (#1425) Owner decision on #1425, option (a): a runtime error raised by a binary operation whose operands span lines reports the physical line of the OPERATOR in e.line, the `Error line N` header, the excerpt and caret, and the traceback. Temporal filing (#1381, the first line) is unchanged. x is 0 z is 1 / ( 0) Before: `Error line 3: division by zero`, no excerpt (the `/` column does not exist on line 3). After: `Error line 2`, excerpt of line 2, caret under `/`. Mechanism: the VM line when an op runs is the last OP_LINE executed in the frame (a call's return restores the caller's line since #1424/#1434). The compiler now tracks the min/max line its right operand REQUESTED from emit_line (compile_operand_then_op). Only when that operand requested a line other than the operator's does it re-stamp the operator's line before the op. Single-line code therefore compiles to identical bytecode (244 test programs and 78 lib modules: 0 differing dumps vs ea9ee90). Used by AST_BINOP and the compound-assignment ops on a field and an index (`x += ...` on a name already desugars to AST_BINOP). An f-string's lexer-synthesized `+` keeps the old stamping (binop.synth); an operator written inside an interpolation follows the rule. Tests: tests/test_operator_line.eigs (22 rows: / % + - * & <, a chain whose second + faults, a multi-line call operand on each side, an operator on the line after its left operand, += on a name/field/index, an operator inside an f-string interpolation, hot JIT callee and OSR loop rows, and #1381 filing) plus two uncaught fixtures pinning header, excerpt, caret and traceback, all enrolled in [0g] under the interpreter, the JIT and forced OSR. Base ea9ee90: 16 of 22 rows fail; both fixtures report the operand's line. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF
TRACE.md: the L record a multi-line binary operator adds (or moves) on the tape, and that single-line code and f-string concatenation emit nothing extra. llms.txt: the error-line sentence names the operator's line for a binary operator whose operands span lines. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF
|
Round-1 blind critic (GPT-6 Astra, 252 executions): FAIL, on two counts, both being fixed in round 2.
|
Save the executed line around a multiline binary instruction and restore it on success in the VM, JIT and OSR. Runtime state preserves short-circuit attribution; one-line code emits no new bytecode. Append and verify adjacent scope opcodes, disallow jump/handler entries into them, and restore worker trace lines without reading the main thread cache. Revert dedicated compound field/index restamping to ea9ee90 behavior. Compound field/index forms are left for a separate owner decision; plain name += remains a binary operation. Document the exact binary-only scope. Validation receipts against ea9ee90: - Bytecode: tests=244/244 lib=78/78 identical; differing=0. - Parent controls: base/fixed 20/20, head 5b25a89 3/20 in each of 3 tiers. - Final parent regression: 35/35 in each tier; eight per-expression native stamp witnesses in each compiled tier. - Targeted sections 0g/42a/42f/42f2: 304/304, 0 failed/skipped; chunks=5 bearing=5. - make test-changed BASE=ea9ee90: 1398/1398, 0 failed/skipped; chunks=38 bearing=37; full=no. - ASan targeted plus 42h, detect_leaks=1: 326/326, 0 failed/skipped; chunks=6 bearing=6; no leak report. - precheck: 18 passed, 0 failed/skipped; JIT smoke: all cases passed. - Strict differential: 99 raises, 0 silent/misattributed; 31 pins, 0 broken; valid-input 255/255 unchanged in all 3 modes; no-gfx rows skipped. - Tape diff: 246 programs x 3 tiers=738 pairs; 691 identical after pinning, 35 changed, 12 boundary-nondeterministic; exact records retained. - Strict replay: 738 recordings x 3 tiers=2214 combinations; 2115 matched, 99 documented boundary refusals, 0 stable divergences, 0 unresolved. - Critic matrix: 312 executions; 99 identical cases, 57 intended binary diagnostic changes. - Plants: head fails 17/20 parent controls per tier; missing verifier constraints fail 4/29 checks; stale MT trace save fails 1/22 checks (2000 zero-line records), restored 22/22. - jit_diff: OK (246 programs x {jit, osr} vs the interpreter; OBS examined=591 denied=12; 31 arms adjudicated; 3 ledgered), unchanged. - replay_diff: OK (246 programs record+replay; 11 at the documented boundary; 0 nondeterministic; 0 ledgered). Full suites were intentionally not run on this machine per the builder brief. Tape differences include known thread scheduling/transition deduplication, checkout paths in import warnings, and untaped mktemp paths (filed #1464). The complete changed-record inventory and native witness receipts are retained under /tmp/es-1425c-evidence; RESULT.md documents caveats. Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97c0eb57cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (scope) emit_op_u32(c, OP_BINARY_LINE, (uint32_t)op_line, op_line); | ||
| emit(c, opc, op_line); |
There was a problem hiding this comment.
When this path handles a multiline plain-name compound assignment such as x += (\n "s"), the desugared AST_BINOP still carries the variable's column (parser.c:1841) rather than the += token's column. Consequently the new operator-line scope reports the correct line but places the caret under x, contradicting the documented rule that the caret identifies the faulting operator. Preserve the compound token's column when constructing the binary node so the opcode emitted here is attributed to +=.
Useful? React with 👍 / 👎.
|
Codex review (agentic, GPT-6.1 Sol): Didn't find any major issues (P0/P1). Reviewed commit: |
|
Closing as superseded by drift, not rejected. The design (#1425, option a: report the operator's line) stands, and Codex's review of Since this branch was cut, #1665 arm D (B0–B10, #1701–#1726) rewrote large parts of The plan is to re-implement #1425 on current main, using this diff as the reference ( |
2 participants
Closes #1425
This implements the owner's decision on #1425 (option a): a runtime error raised by a binary operator whose operands span lines reports the operator's line. That applies to
e.line, theError line Nheader, the excerpt and caret, and the traceback. Temporal filing (the #1381 first-line rule) is unchanged.How.
compile_operand_then_opinsrc/compiler.crecords whether the right operand askedemit_linefor another line. It re-stamps the operator's line before the op only in that case, so single-line code compiles to identical bytecode. An f-string's synthesized+is excluded (newbinop.synthflag); an operator the user writes inside an interpolation follows the rule.One judgement call (owner may veto): the rule also covers compound
+=on a field or index when it spans lines. Plainx +=on a name already parses as a binary operator, so leaving the field and index forms out would make one operator report two different lines. Other multi-line constructs keep their old line, since #1425 leaves them undecided: call arguments, indexing, a non-compound index or field store, and unary-/not.Verified.
New
tests/test_operator_line.eigs(22 rows) plus module and function fixtures in [0g], covering the interpreter, the JIT and OSR, each with a compiled-code witness. On the base: 6 of 22 rows pass. Plants:Bytecode: 244 programs and 78 lib modules are byte-identical to the base, so there is no cost to single-line code. Ir is unchanged:
bench_perf−0.003%,bench_dmg_shape−0.001%.Tapes: the only added
Lrecords come fromeval'd multi-line f-strings, and none are removed. Strict replay is identical on 36/36 tier pairs.make test-changed: 1327/1327.tools/jit_diff.sh: OK.Docs: SPEC.md (an executed example), TRACE.md and llms.txt.
🤖 Generated with Claude Code
https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF