Skip to content

fix(hints): silence cond{~v} discard-hint on multi-stmt guard bodies#315

Merged
danieljohnmorris merged 2 commits into
mainfrom
fix/cond-discard-hint-fp
May 16, 2026
Merged

fix(hints): silence cond{~v} discard-hint on multi-stmt guard bodies#315
danieljohnmorris merged 2 commits into
mainfrom
fix/cond-discard-hint-fp

Conversation

@danieljohnmorris
Copy link
Copy Markdown
Collaborator

Summary

The cond{^"err"} / cond{~v} discarded-result hint added in #300 fires as a false positive on multi-statement guard bodies like

f n:n>R t t;=n 0{prnt "empty";~"ok"};~"done"

Surfaced by interactive-cli rerun4, where every successful run of a tracker emitted the hint. The author has clearly composed the multi-stmt block on purpose; firing there is noise that erodes trust in the hint.

Tightened the walker in collect_hints to require body.len() == 1. Only the original bare single-stmt {^"err"} / {~v} footgun trips the hint now.

Repro

Before (on main):

$ ilo run prog.ilo multi 1
done
{"hints":["hint: `cond{^\"err\"}` and `cond{~v}` discard the body expression..."]}

After:

$ ilo run prog.ilo multi 1
done

Single-stmt shape still fires:

$ ilo run single.ilo g 5    # source: g x:n>R n t;<x 0{^"neg"};~x
5
{"hints":["hint: `cond{^\"err\"}` and `cond{~v}` discard the body expression..."]}

What's in the diff

  • src/main.rs: walker requires body.len() == 1 for the discard heuristic; two regression tests (collect_hints_multi_stmt_guard_body_with_trailing_ok_no_hint, collect_hints_multi_stmt_guard_body_with_trailing_err_no_hint).
  • examples/cond-multi-stmt-guard-return.ilo: cross-engine pin for the no-hint shape via the examples_engines harness.

Existing tests cover the still-flagged single-stmt shapes (collect_hints_discarded_err_in_guard_body, collect_hints_discarded_ok_in_guard_body) plus nested foreach/match guards (collect_hints_discarded_err_inside_foreach_body, collect_hints_discarded_err_inside_match_arm_body), all of which have single-stmt guard bodies and continue to fire.

Test plan

  • cargo test --release --features cranelift (full suite green)
  • cargo fmt --check
  • cargo clippy --release --features cranelift -- -D warnings
  • CLI repro for the false-positive shape: no hint
  • CLI repro for single-stmt {^"neg"}: still emits the hint
  • examples_engines harness runs the new example across tree + vm engines

Follow-ups

None.

The cond{^"err"} / cond{~v} discarded-result hint added in #300 fired
on any braced-cond body whose last statement was an Err/Ok expression.
That over-triggers on multi-statement bodies like

  =n 0{prnt "empty";~"ok"};~"done"

where the trailing `~"ok"` is the body's composed value, not an
accidental early-return. The author has clearly written the block on
purpose and the hint becomes noise. Surfaced by interactive-cli rerun4
where every successful run of a tracker function fired the hint.

Tighten the walker to require body.len() == 1: only a bare single-stmt
{^...} or {~...} is the original footgun the hint targets. Multi-stmt
bodies are now silent.

Add two regression tests pinning the multi-stmt-no-hint behaviour for
both ~v and ^e tails.
Cross-engine example demonstrating that multi-statement braced-cond
bodies ending in `~v` or `^e` are valid composed blocks, even though
the trailing Result is technically discarded when the guard does not
short-circuit. The examples_engines harness now exercises this shape
on every engine; the hint suppression itself is pinned as a unit test.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@danieljohnmorris danieljohnmorris merged commit 62832ec into main May 16, 2026
5 checks passed
@danieljohnmorris danieljohnmorris deleted the fix/cond-discard-hint-fp branch May 16, 2026 20:54
danieljohnmorris added a commit that referenced this pull request May 16, 2026
twelve fixes since 0.11.3, surfaced by rerun4 personas plus standing asks: srt-Cranelift TLS desync (#306), CLI auto-run restoration (#307), OP_LISTAPPEND O(n^2) JIT memory regression (#308), precedence-pair hint false-positive on parens (#309), prefix ?? accepts call expression (#310), += pure-shape docs (#311), bare-mutation silent no-op verifier warning ILO-T033 (#312), asin/acos/atan inverse trig builtins (#313), flat cross-engine (#314), cond{~v} discard hint multi-stmt false-positive (#315), rsrt fn xs key-fn overloads (#316), xs.(expr) paren-after-dot diagnostic hint (#317).
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.

1 participant