compiler: add static lint pass (unreachable code, dead branches, self-assign, % 0)#614
Merged
Merged
Conversation
…-assign, % 0)
Adds a new advisory static-analysis pass to hemlockc that flags well-typed,
memory-safe code that is almost certainly a mistake — the kind of thing worth
catching before shipping a binary:
- unreachable code (statements after return/break/continue/throw/panic,
or after an if/else whose arms all diverge)
- dead branches (constant if/while conditions, redundant else)
- self-assignment (x = x, obj.f = obj.f, a[i] = a[i] — side-effect-free only)
- modulo by zero (x % 0 with a literal zero — traps at runtime)
- unused variables (strict mode: a let/const never read in its scope)
Design mirrors the borrow checker: advisory warnings by default, promotable to
fatal with --lint-error, tightened with --lint-strict, disabled with --no-lint,
and surfaced under --check. The pass runs before the optimizer so it diagnoses
the source as written (rather than what survives dead-code elimination), and
only the program's own top-level module is analysed so clean builds don't drown
the user in warnings about imported library code.
The analysis is deliberately conservative — every diagnostic is something the
compiler is certain about, so there are no false positives: self-assignment is
only flagged when both sides are pure, while(true) is left alone, and hoisted
declarations after a return are not reported as unreachable.
Tests: new tests/lint suite (8 fixtures incl. 3 negative false-positive guards),
wired into make test-lint and test-all. Borrow runner now passes --no-lint so
the two diagnostic suites stay isolated. Verified: parity 300/300 (100%, no
regressions), 1233/1233 repo files compile, compiler 54/54, borrow 53/53.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WKLiroxGKnQhJBa43UbHJT
CI fix: tests/lint fixtures are compiler-diagnostic cases and some trap at
runtime by design (modulo_zero.hml), but the interpreter test runner sweeps
tests/**/*.hml and executes them — failing the test/asan/macos-arm jobs.
Exclude */lint/* from tests/run_tests.sh and add lint to the full-parity
SKIP_CATEGORIES, exactly how tests/compiler is handled.
New lint checks (all advisory, all certain):
- self-comparison x < x / x > x (always false, even for NaN) and
x && x / x || x (redundant). ==, !=, <=, >= are
deliberately exempt: every float comparison with NaN
is false, so x == x is NOT always true and x != x is
the canonical NaN check. Literal-literal operands are
skipped (constant-folder territory, and operator tests
exercise them deliberately).
- duplicate field { a: 1, a: 2 } — the later value silently wins.
Spread entries are skipped.
- duplicate case repeated literal switch case values — the second case
is unreachable. Identifier/computed cases are skipped.
A repo-wide sweep of 1200+ .hml files confirms zero false positives (the only
hits are two factually-correct warnings on a test that deliberately verifies
NaN self-comparison semantics).
Tests: 5 new fixtures (incl. 2 negative guards for the NaN idiom and
spread/computed cases) — lint suite 13/13, borrow 53/53, interpreter suite no
longer runs lint fixtures.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WKLiroxGKnQhJBa43UbHJT
…ever fall through
Named arguments: a named argument that matches no parameter was silently
skipped by the type checker ("runtime will catch it") — but only the
interpreter catches it (throws "Unknown parameter name"); the compiled binary
silently binds the argument positionally and runs, so the same program
produced different behavior on the two backends. Reject it at compile time
("unknown named argument 'nam' to 'process'"), following the existing
precedent of too-many/too-few argument errors. A repo-wide sweep confirms no
existing .hml file triggers the new error.
Unreachable code after infinite loops: `loop { ... }` and `while (true)`
only fall through via a break bound to them. The lint pass now models break
binding (unlabeled breaks bind the innermost enclosing loop/switch; labeled
breaks match the loop's label) and flags statements after an infinite loop
with no escaping break. The one repo hit is a true positive: a test server's
trailing `return null;` after a break-less accept loop.
Tests: 2 new lint fixtures (loop_no_break + negative guard covering
conditional, labeled, and nested-loop breaks) — lint 15/15; 2 new type_check
runner cases (unknown named arg rejected, valid named args accepted);
compiler 54/54, memory and annotation codegen suites green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WKLiroxGKnQhJBa43UbHJT
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
2 participants
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.
Adds a new advisory static-analysis pass to hemlockc that flags well-typed,
memory-safe code that is almost certainly a mistake — the kind of thing worth
catching before shipping a binary:
or after an if/else whose arms all diverge)
Design mirrors the borrow checker: advisory warnings by default, promotable to
fatal with --lint-error, tightened with --lint-strict, disabled with --no-lint,
and surfaced under --check. The pass runs before the optimizer so it diagnoses
the source as written (rather than what survives dead-code elimination), and
only the program's own top-level module is analysed so clean builds don't drown
the user in warnings about imported library code.
The analysis is deliberately conservative — every diagnostic is something the
compiler is certain about, so there are no false positives: self-assignment is
only flagged when both sides are pure, while(true) is left alone, and hoisted
declarations after a return are not reported as unreachable.
Tests: new tests/lint suite (8 fixtures incl. 3 negative false-positive guards),
wired into make test-lint and test-all. Borrow runner now passes --no-lint so
the two diagnostic suites stay isolated. Verified: parity 300/300 (100%, no
regressions), 1233/1233 repo files compile, compiler 54/54, borrow 53/53.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01WKLiroxGKnQhJBa43UbHJT