Skip to content

Fix optional-access edge cases, predicate error display, MinInt64, and compile-time call-target checks#24

Merged
myzie merged 4 commits into
mainfrom
claude/repo-review-improvements-e2fm8r
Jun 10, 2026
Merged

Fix optional-access edge cases, predicate error display, MinInt64, and compile-time call-target checks#24
myzie merged 4 commits into
mainfrom
claude/repo-review-improvements-e2fm8r

Conversation

@myzie

@myzie myzie commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

A review pass over the repo turned up a handful of real bugs plus two stale spec claims. All are fixed here; everything is covered by new regression tests, and go test -race ./..., go vet, and short runs of every fuzz target pass.

Bugs fixed

1. Optional access (?. / ?[) declined or mangled valid receivers

The LHS walker in internal/optaccess had several gaps relative to the non-optional operators:

Expression Before After
[1, 2, 3]?[0] compile error (illegal character '?') 1
{"a": 1}?.a compile error leaking the sentinel (expected 'EOF', found __try_select__) 1
map(xs, it*2)?[0] compile error (LHS chopped at the paren) works
if(true, [1], [2])?[0] compile error works
a?.map, a?.if compile error (while a.map works) works

The walker now handles brace-closed primaries (bare object literals), bracket pairs that open the input, and the map/if keyword tokens that expr accepts as call targets and selector links. Rewrites whose LHS would be empty are declined outright so malformed input gets the parser's plain ? diagnostic instead of garbage sentinel source.

2. Predicate error messages leaked internal sentinels

formatPredicate only mapped __expr_map__ back to map, via a string ReplaceAll. So:

  • filter(xs, if(it,1,2) > "x") reported `__expr_if__(it, 1, 2) > "x"`
  • filter(xs, it?.name) reported `__try_select__(it, "name")`
  • a string literal containing __expr_map__ was corrupted to "map" in the message
  • JSON-style literals printed as []any{...} / map[string]any{...}

The fix replaces the string substitution with an AST-level display transform (on a copy — the shared tree is never mutated): keyword sentinels recover their names, optional-access calls print as recv?.field / recv?[idx], and composite literals print in the bare JSON style users type.

3. -9223372036854775808 (MinInt64) was unrepresentable

The parser produces unary-minus around bare digits that overflow int64 on their own, so the most negative integer failed at Run time. The fold pass now reads the negation and literal as a single signed parse (-0x8000000000000000 works too); one past MinInt64 still errors.

4. Unsupported call targets surfaced only at Run time

fns[0](), (f)(x), f()(x), and a?.b() compiled fine and then failed every Run with unsupported call target *ast.IndexExpr. They are now rejected at Compile time (continuing the direction of #15), and a?.b() gets a tailored message — cannot call the result of optional access (a?.b()) — instead of leaking the sentinel call. The runtime rejection stays as defense in depth. Spec updated.

5. Stale spec/llms.txt: integer overflow does not wrap

docs/reference/spec.md and llms.txt both claimed integer overflow "wraps (matching Go)", but the implementation has checked +/-/* and rejects -MinInt64 and MinInt64 / -1 with ErrEvaluate. Docs now describe the implemented (safer) behavior, and the previously tolerant adversarial overflow tests now pin it.

Noted but deliberately not changed

  • Registered functions that panic propagate the panic (methods recover runtime errors). The sandboxing guide explicitly says not to register panicking functions, so this stays a documented contract.
  • Generalizing call targets to arbitrary callable-valued expressions (e.g. fns[0](x)) would be a language-surface decision; this PR keeps the documented ident/selector rule and just moves the failure to Compile time.

https://claude.ai/code/session_01V9oqpXChcB6zJFr6yw956M


Generated by Claude Code

claude added 4 commits June 10, 2026 03:13
The ?. / ?[ rewriter declined or mangled several receivers that the
non-optional operators accept:

- An array literal that opens the expression ([1, 2]?[0]) walked off
  the start of the token stream and was declined, leaving a stray ?
  for the parser to choke on.
- Bare object literals ({"a": 1}?.a) had no brace handling at all;
  the walker fell into the default case and spliced an empty LHS,
  producing a parse error that leaked the __try_select__ sentinel.
- map(...) and if(...) call results (map(xs, it)?[0]) lost their
  keyword token during the paren walk-back, chopping the LHS to just
  the argument list.
- a?.map / a?.if were rejected even though a.map / a.if work.

Teach lhsStartIdx about brace-closed primaries and the map/if keyword
tokens, return the opening bracket when it starts the input, and
decline any rewrite whose LHS would be empty so malformed input gets
the parser's plain illegal-character error instead of sentinel soup.

https://claude.ai/code/session_01V9oqpXChcB6zJFr6yw956M
formatPredicate translated only the map sentinel back to its source
spelling, and did so with a string ReplaceAll. Predicate errors
therefore leaked __expr_if__ and __try_select__/__try_index__ into
messages, printed JSON-style literals as []any{...} composites, and
corrupted string literals whose content happened to spell
__expr_map__.

Replace the string substitution with an AST-level display transform:
a copied tree renames keyword sentinels, prints optional-access
sentinel calls as recv?.field / recv?[idx], and prints []any /
map[string]any composite literals in the bare JSON style users type.
The original tree is never mutated, so concurrent Runs are unaffected.

https://claude.ai/code/session_01V9oqpXChcB6zJFr6yw956M
The parser yields a UnaryExpr SUB of the bare digits, which overflow
int64 on their own, so the expression failed at Run time even though
the value it denotes is representable. Fold the negation and literal
into a single signed parse during prewalk. One past MinInt64 still
errors, and overflow tests now pin the checked (erroring) behavior
that the spec previously misdescribed as silent wrapping.

https://claude.ai/code/session_01V9oqpXChcB6zJFr6yw956M
Calls whose target is an index expression, another call, a paren
group, or an optional access used to compile fine and then fail every
Run with "unsupported call target". Surface the rejection from
validate instead, continuing the move of unsupported-syntax detection
to Compile time (#15). Calls on optional-access results (a?.b()) get
a tailored message so the internal sentinel call the rewrite produces
never leaks into a confusing generic error.

Also sync the spec and llms.txt with the implemented overflow-checked
integer arithmetic, which they still described as wrapping silently.

https://claude.ai/code/session_01V9oqpXChcB6zJFr6yw956M
@myzie myzie merged commit c15bc6b into main Jun 10, 2026
1 check passed
@myzie myzie deleted the claude/repo-review-improvements-e2fm8r branch June 10, 2026 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants