fix: multi-line fn body with record-constructor tail no longer trips ILO-P020#361
Merged
Conversation
is_fn_decl_start_strict was scanning forward for the > return-type separator to confirm a candidate ident param:type ... shape was a real fn header rather than a record-constructor expression. The scan only stopped at ; / } / EOF, so when a multi-line function body ended with a bare record-constructor tail like cr country:name revenue:rv, the scan walked past the end of the line into the NEXT function's name>type header, found a >, and incorrectly classified the record as a fn-decl start. The current body terminated, parse_program then tried to parse cr country:name revenue:rv as a fn header, and ILO-P020 fired because that line has no > at all. Add a per-token decl_boundary check inside the scan loop. A real fn header always has its > on the same logical line as the name, so a boundary before > unambiguously means the candidate is not a header. Reported by db-analyst rerun8 friction #3 (record-constructor-as-tail is a common JSON / API shape pattern, currently forcing a paren wrap on every multi-line function returning a record).
Exercises the four shapes the strict-fn-decl-start scan needs to
classify correctly:
(a) cr field:val in a multi-line body, followed by another fn
(1, 2, 3 fields) — the original db-analyst rerun8 repro
(b) cr field:val with no trailing fn (EOF branch of the scan)
(c) cr field:val on a single-line body after an explicit ;
(d) real fn declaration with > on the header line still parses
as a fn decl (regression guard for the original purpose of
is_fn_decl_start_strict)
All cases run on tree, VM, and Cranelift via the ENGINES_ALL fan-out
already used by sibling regression tests.
Drives the multi-line record-constructor tail shape through every engine via tests/examples_engines.rs and gives an in-context example that agents will encounter when grepping examples/ for record-shape patterns. Reads named fields rather than asserting on the full record's Display output because record-field iteration order on the tree-walker is HashMap-backed and therefore non-deterministic across runs.
Now that the parser accepts cr field:val as a tail expression without paren-wrapping, add it to the SPEC.md table of safe endings for non-last functions in a file. ai.txt and skills/ilo/SKILL.md regenerate from SPEC.md via build.rs and pick up the same row.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
CI run failed because rec=build was binding rec to the function reference F _, not to the call result, so rec.x then errored with ILO-T018 field access on non-record type F _. Add the explicit zero-arg call form rec=build(). Also reshape the negative-control test (d) to use a single-line file (no newlines), which is the shape that actually exercises the strict check's > scan and would catch a regression where someone over-tightens the new boundary guard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A bare record constructor (
cr field:val field:val) at the start of a continuation line in a multi-line function body was getting mis-classified as a new top-level fn declaration. The body terminated, the parser tried to readcr country:name revenue:rvas a fn header, and ILO-P020 fired because that line has no>at all. The persona-side workaround was paren-wrapping the constructor(cr country:name revenue:rv)— a four-character cost on every multi-line function returning a record, which is a very common JSON / API shape.Reported by db-analyst rerun8 friction #3 (severity red). Manifesto framing: record-as-tail multiplied across every agent reaching for a record-shaped return is a measurable token-cost paper-cut that this fix removes for the whole population.
Repro
Before:
After:
100(clean run on tree, VM, Cranelift).Root cause
Parser::is_fn_decl_start_strictinsrc/parser/mod.rsis the guard that preventsparse_body_withfrom terminating a body when the next;-separated statement looks likeIdent param:type ...but is actually a record-constructor expression. It scans forward looking for>(the return-type separator that confirms a real header) and stops only at;,}, or EOF. Newlines have already been filtered out of the token stream (their positions are recorded indecl_boundaryfor ILO-P020 anchoring), so the scan would walk straight across a top-level declaration boundary, find the>of the NEXT function's header, and incorrectly return true.What's in the diff
src/parser/mod.rs: per-tokendecl_boundarycheck inside the scan loop. A real fn header always has its>on the same logical line as the name, so finding a boundary before>means this is not a header.tests/regression_p020_record_tail.rs: cross-engine coverage (tree + VM + Cranelift) for (a) 1/2/3-field record tails followed by another fn — the original repro; (b) record tail with no trailing fn (EOF branch); (c) record tail on single-line body after explicit;; (d) regression guard for real fn decls still being recognised correctly (the case the strict check was originally added for).examples/record-tail.ilo: agent-facing example, picked up by theexamples_engines.rsharness so it runs on every engine on every commit.SPEC.md: record-constructor row added to the "safe non-last function body endings" table.ai.txtandskills/ilo/SKILL.mdregenerate fromSPEC.mdviabuild.rsand pick up the same row.Test plan
examples/record-tail.iloruns on every engine via the harnessai.txt,SKILL.md) clean aftercargo buildcargo test --release --features craneliftgreenFollow-ups
None. The fix is narrow to one helper with one new early-return; no adjacent bugs surfaced.