From 3bde2f694add4df1a8fb5f0f842264238839165b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 26 Jun 2026 12:59:30 +0000 Subject: [PATCH 1/3] compiler: add static lint pass (unreachable code, dead branches, self-assign, % 0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01WKLiroxGKnQhJBa43UbHJT --- Makefile | 11 +- docs/README.md | 1 + docs/advanced/lint.md | 120 ++++ include/compiler/lint.h | 77 +++ src/backends/compiler/lint.c | 668 +++++++++++++++++++++++ src/backends/compiler/main.c | 50 ++ tests/borrow/run_borrow_tests.sh | 3 +- tests/lint/dead_branch.expected | 2 + tests/lint/dead_branch.hml | 11 + tests/lint/modulo_zero.expected | 1 + tests/lint/modulo_zero.hml | 5 + tests/lint/neg_clean.expected | 0 tests/lint/neg_clean.hml | 5 + tests/lint/neg_index_sideeffect.expected | 0 tests/lint/neg_index_sideeffect.hml | 8 + tests/lint/neg_while_true.expected | 0 tests/lint/neg_while_true.hml | 9 + tests/lint/run_lint_tests.sh | 101 ++++ tests/lint/self_assign.expected | 2 + tests/lint/self_assign.hml | 7 + tests/lint/unreachable.expected | 1 + tests/lint/unreachable.hml | 8 + tests/lint/unused.expected | 1 + tests/lint/unused.flags | 1 + tests/lint/unused.hml | 7 + 25 files changed, 1097 insertions(+), 2 deletions(-) create mode 100644 docs/advanced/lint.md create mode 100644 include/compiler/lint.h create mode 100644 src/backends/compiler/lint.c create mode 100644 tests/lint/dead_branch.expected create mode 100644 tests/lint/dead_branch.hml create mode 100644 tests/lint/modulo_zero.expected create mode 100644 tests/lint/modulo_zero.hml create mode 100644 tests/lint/neg_clean.expected create mode 100644 tests/lint/neg_clean.hml create mode 100644 tests/lint/neg_index_sideeffect.expected create mode 100644 tests/lint/neg_index_sideeffect.hml create mode 100644 tests/lint/neg_while_true.expected create mode 100644 tests/lint/neg_while_true.hml create mode 100755 tests/lint/run_lint_tests.sh create mode 100644 tests/lint/self_assign.expected create mode 100644 tests/lint/self_assign.hml create mode 100644 tests/lint/unreachable.expected create mode 100644 tests/lint/unreachable.hml create mode 100644 tests/lint/unused.expected create mode 100644 tests/lint/unused.flags create mode 100644 tests/lint/unused.hml diff --git a/Makefile b/Makefile index a49b6542..c85ae283 100644 --- a/Makefile +++ b/Makefile @@ -161,6 +161,9 @@ TYPECHECK_SRCS = $(wildcard $(SRC_DIR)/backends/compiler/type_*.c) # Borrow/ownership checker — shared so the LSP can surface its diagnostics BORROWCHECK_SRCS = $(SRC_DIR)/backends/compiler/borrow_check.c +# Static lint / diagnostics pass (compiler-only) +LINTCHECK_SRCS = $(SRC_DIR)/backends/compiler/lint.c + COMMON_SRCS = $(FRONTEND_SRCS) $(MODULES_SRCS) $(TYPECHECK_SRCS) $(BORROWCHECK_SRCS) $(SHARED_SRCS) SRCS = $(COMMON_SRCS) $(TOOL_SRCS) $(INTERP_SRCS) @@ -578,6 +581,7 @@ analyze-clean: COMPILER_SRCS = $(SRC_DIR)/backends/compiler/main.c \ $(wildcard $(SRC_DIR)/backends/compiler/codegen*.c) \ $(BORROWCHECK_SRCS) \ + $(LINTCHECK_SRCS) \ $(TYPECHECK_SRCS) \ $(SHARED_SRCS) \ $(FRONTEND_COMPILER_SRCS) @@ -866,6 +870,11 @@ test-compiler: compiler test-borrow: compiler @bash tests/borrow/run_borrow_tests.sh +# Run static lint / diagnostics tests +.PHONY: test-lint +test-lint: compiler + @bash tests/lint/run_lint_tests.sh + # Check that interpreter tests compile (does not check output parity) .PHONY: compile-check compile-check: compiler @@ -903,7 +912,7 @@ test-memory: compiler # Run all test suites .PHONY: test-all -test-all: test test-compiler test-borrow parity test-contracts test-bundler test-lsp test-memory test-formatter test-cli +test-all: test test-compiler test-borrow test-lint parity test-contracts test-bundler test-lsp test-memory test-formatter test-cli # ========== RELEASE BUILD ========== diff --git a/docs/README.md b/docs/README.md index 5c98f01f..5baba3ca 100644 --- a/docs/README.md +++ b/docs/README.md @@ -38,6 +38,7 @@ Welcome to the Hemlock programming language documentation! - [File I/O](advanced/file-io.md) - File operations and resource management - [Memory Ownership](advanced/memory-ownership.md) - Memory ownership semantics and lifetime management - [Borrow Checker](advanced/borrow-checker.md) - Rust-like ownership analysis (use-after-free, double-free, leaks) +- [Static Lint](advanced/lint.md) - Catch unreachable code, dead branches, self-assignment, and more before shipping - [Signal Handling](advanced/signals.md) - POSIX signal handling - [Command-Line Arguments](advanced/command-line-args.md) - Access program arguments - [Command Execution](advanced/command-execution.md) - Execute shell commands diff --git a/docs/advanced/lint.md b/docs/advanced/lint.md new file mode 100644 index 00000000..e1937ed7 --- /dev/null +++ b/docs/advanced/lint.md @@ -0,0 +1,120 @@ +# Static Lint (Diagnostics) + +The Hemlock compiler (`hemlockc`) ships a **static lint pass** — a small +analysis that flags code which is well-typed and memory-safe but almost +certainly a mistake. Where the [type checker](../language-guide/type-system.md) +proves values fit their types and the [borrow checker](borrow-checker.md) tracks +resource ownership, the linter catches the everyday slip-ups you want to find +*before you ship a binary*: unreachable code, dead branches, self-assignment, +modulo-by-zero, and unused variables. + +Like the borrow checker, it is **advisory by design.** It emits **warnings**, +not errors, and never changes what your program does. You can make it strict, +make it fatal, or turn it off. + +``` +Source (.hml) + ↓ + Parse → AST + ↓ + Resolve + ↓ + Lint ← this pass (warnings by default, sees your code as written) + ↓ + Optimize → Type Check → Borrow Check + ↓ + C Code Generation +``` + +The lint pass deliberately runs **before** the optimizer, so it diagnoses the +source you actually wrote rather than what is left after dead-code elimination. +Only your program's own top-level module is analysed; imported modules are +linted when *they* are compiled, so a clean build never buries you in warnings +about library code you did not write. + +--- + +## What it catches + +| Diagnostic | When | Default | +|------------|------|---------| +| **unreachable code** | a statement follows one that always diverges (`return`/`break`/`continue`/`throw`/`panic()`, or an `if`/`else` whose arms all diverge) | on | +| **dead branch** | an `if`/`while` condition is a constant (`if (false)`, `while (0)`) | on | +| **redundant branch** | an `if (true) { … } else { … }` whose `else` can never run | on | +| **self-assignment** | `x = x`, `obj.f = obj.f`, or `a[i] = a[i]` — a no-op | on | +| **modulo by zero** | `x % 0` with a literal zero divisor — traps at runtime | on | +| **unused variable** | a `let`/`const` that is never read in its scope | strict only | + +Every diagnostic is something the compiler is **certain** about. The analysis is +intentionally conservative — for example, self-assignment is only reported when +both sides are side-effect-free, so `a[next()] = a[next()]` is left alone, and +`while (true)` (a deliberate idiom) is never flagged. There are no false +positives to silence. + +### Examples + +```hemlock +fn classify(n: i32): string { + if (n > 0) { + return "positive"; + print("never runs"); // warning: unreachable code: control always + } // leaves via the return on line 3 + return "other"; +} + +fn dead(): i32 { + if (false) { // warning: condition is always false; this + return 1; // branch never executes + } + let x = 10; + x = x; // warning: self-assignment: 'x = x' has no effect + return x % 0; // warning: modulo by zero: traps at runtime +} +``` + +With `--lint-strict`, unused locals are flagged too. Names beginning with `_` +and function bindings are exempt by convention: + +```hemlock +fn f(): i32 { + let used = 1; + let unused = 2; // warning: variable 'unused' is declared but never used + let _scratch = 3; // exempt: leading underscore + return used; +} +``` + +--- + +## Flags + +| Flag | Effect | +|------|--------| +| *(default)* | Lint on; advisory warnings, build still succeeds | +| `--lint-strict` | Also flag unused variables | +| `--lint-error` | Treat every lint finding as an error and **fail the build** | +| `--no-lint` | Disable the lint pass entirely | + +These compose with `--check`, which runs the static analyses (type, borrow, +lint) and stops before code generation: + +```sh +hemlockc --check --lint-strict app.hml # report everything, compile nothing +hemlockc --lint-error app.hml -o app # refuse to build if anything is flagged +``` + +Use `--lint-error` in CI to keep unreachable code, dead branches, and +self-assignments out of a release binary, while leaving the default advisory +behaviour for everyday local builds. + +--- + +## Why warnings, not errors + +This mirrors Hemlock's stance everywhere else: *explicit over implicit, and the +programmer keeps control.* A dead branch or a `% 0` might be a placeholder you +are actively working on; the compiler points it out but does not stop you. When +you are ready to enforce a clean bill of health, opt in with `--lint-error`. + +See also: [Borrow Checker](borrow-checker.md) · +[Compiler Optimizations](compiler-optimizations.md) diff --git a/include/compiler/lint.h b/include/compiler/lint.h new file mode 100644 index 00000000..1fe6fd48 --- /dev/null +++ b/include/compiler/lint.h @@ -0,0 +1,77 @@ +/* + * Hemlock Compiler - Static Lint / Diagnostics Pass + * + * An opt-out static-analysis pass that runs after type checking and borrow + * checking and before code generation. Where the type checker proves that + * values fit their declared types and the borrow checker tracks resource + * ownership, the lint pass flags code that is well-typed and memory-safe but + * almost certainly a mistake — the kind of thing you want caught before you + * ship a binary: + * + * - unreachable code (statements after return/break/continue/throw/panic, + * or after an if/else where both arms always diverge) + * - dead branches (`if`/`while` conditions that are constant) + * - self-assignment (`x = x`, `obj.f = obj.f`, `a[i] = a[i]`) + * - modulo by zero (`x % 0` with a literal zero — traps at runtime) + * - unused variables (strict mode: a let/const that is never read) + * + * Only the program's own top-level module is analysed; imported modules are + * linted when they are compiled in their own right, so a clean build never + * drowns the user in warnings about library code they did not write. + * + * In keeping with Hemlock's "unsafe is a feature, explicit over implicit" + * philosophy these are reported as advisory warnings and do not fail the + * build unless --lint-error is passed. The analysis is intentionally + * conservative: every diagnostic is something the compiler is confident about, + * so there are no false positives to silence. + */ + +#ifndef HEMLOCK_LINT_H +#define HEMLOCK_LINT_H + +#include "frontend/ast.h" + +/* A single diagnostic. Mirrors BorrowDiag/TypeCheckError so the LSP can + * consume all three uniformly. */ +typedef struct LintDiag { + int line; + int column; + int end_column; + char *message; /* owned; freed by lint_free */ + int is_error; /* 0 = warning, 1 = error (lint-error mode) */ + struct LintDiag *next; +} LintDiag; + +/* Configuration / result handle for one analysis run. */ +typedef struct LintContext { + /* Configuration */ + const char *filename; + const char *source; /* optional, for future column resolution */ + int strict; /* enable the noisier checks (unused variables) */ + int errors_are_fatal; /* promote warnings to errors (affects exit code) */ + int collect; /* collect diagnostics instead of printing to stderr */ + + /* Internal: line of the statement under analysis, used as a fallback when + * an expression node carries no source line of its own. */ + int cur_line; + + /* Results */ + int warning_count; + int error_count; + LintDiag *diags; + LintDiag *diags_tail; +} LintContext; + +/* Create / destroy an analysis context. */ +LintContext *lint_new(const char *filename); +void lint_free(LintContext *ctx); + +/* Collect diagnostics instead of printing them (for LSP integration). */ +void lint_enable_collection(LintContext *ctx, const char *source); + +/* Run the analysis over a whole program. Returns the number of *errors* + * (0 unless errors_are_fatal is set and problems were found). Warnings do + * not contribute to the return value. */ +int lint_program(LintContext *ctx, Stmt **stmts, int stmt_count); + +#endif /* HEMLOCK_LINT_H */ diff --git a/src/backends/compiler/lint.c b/src/backends/compiler/lint.c new file mode 100644 index 00000000..7141e93d --- /dev/null +++ b/src/backends/compiler/lint.c @@ -0,0 +1,668 @@ +/* + * Hemlock Compiler - Static Lint / Diagnostics Pass + * + * See include/compiler/lint.h for the high-level description. + * + * The pass is a straightforward recursive walk of the AST. It carries no + * dataflow state of its own: every check is a local, syntactic pattern that + * the compiler can be certain about, which keeps the implementation small and + * guarantees no false positives. Each check is documented at its site. + */ + +#include +#include +#include +#include + +#include "compiler/lint.h" + +/* ========== DIAGNOSTICS ========== */ + +static void lint_warn(LintContext *ctx, int line, const char *fmt, ...) { + char message[1024]; + va_list args; + va_start(args, fmt); + vsnprintf(message, sizeof(message), fmt, args); + va_end(args); + + int is_error = ctx->errors_are_fatal; + if (is_error) ctx->error_count++; + else ctx->warning_count++; + + if (ctx->collect) { + LintDiag *d = calloc(1, sizeof(LintDiag)); + if (!d) return; + d->line = line; + d->column = 0; + d->end_column = 1; + d->message = strdup(message); + d->is_error = is_error; + d->next = NULL; + if (ctx->diags_tail) ctx->diags_tail->next = d; + else ctx->diags = d; + ctx->diags_tail = d; + } else { + fprintf(stderr, "%s:%d: %s: %s\n", + ctx->filename ? ctx->filename : "", line, + is_error ? "error" : "warning", message); + } +} + +/* Source line for an expression, falling back to the enclosing statement's + * line when the node itself carries none (e.g. assignment expressions). */ +static int lint_eline(LintContext *ctx, Expr *e) { + return (e && e->line > 0) ? e->line : ctx->cur_line; +} + +/* ========== SMALL PREDICATES ========== */ + +/* A constant boolean/numeric condition. Returns 1 and sets *truthy when the + * expression is a literal whose truthiness is unambiguous. Strings and null + * are deliberately excluded — their truthiness rules are easy to get subtly + * wrong, and `if ("")` / `if (null)` are vanishingly rare in real code. */ +static int lint_const_truth(Expr *e, int *truthy) { + if (!e) return 0; + switch (e->type) { + case EXPR_BOOL: + *truthy = e->as.boolean != 0; + return 1; + case EXPR_NUMBER: + if (e->as.number.is_float) *truthy = e->as.number.float_value != 0.0; + else if (e->as.number.is_u64) *truthy = e->as.number.uint_value != 0; + else *truthy = e->as.number.int_value != 0; + return 1; + default: + return 0; + } +} + +/* A literal integer zero (the only case where `%` definitely traps). */ +static int lint_is_int_zero(Expr *e) { + if (!e || e->type != EXPR_NUMBER || e->as.number.is_float) return 0; + if (e->as.number.is_u64) return e->as.number.uint_value == 0; + return e->as.number.int_value == 0; +} + +/* A call to the builtin `panic(...)`, which never returns. */ +static int lint_is_panic_call(Expr *e) { + if (!e || e->type != EXPR_CALL) return 0; + Expr *f = e->as.call.func; + return f && f->type == EXPR_IDENT && f->as.ident.name && + strcmp(f->as.ident.name, "panic") == 0; +} + +/* + * Structural equality for *pure* expressions: identifiers, literals, property + * reads, and indexing whose pieces are themselves pure. Anything containing a + * call (or any node type not listed) compares unequal, so the self-assignment + * check never fires on `a[next()] = a[next()]`, where the two sides may differ. + */ +static int lint_pure_equal(Expr *a, Expr *b) { + if (!a || !b || a->type != b->type) return 0; + switch (a->type) { + case EXPR_IDENT: + return a->as.ident.name && b->as.ident.name && + strcmp(a->as.ident.name, b->as.ident.name) == 0; + case EXPR_NUMBER: + if (a->as.number.is_float || b->as.number.is_float) + return a->as.number.is_float && b->as.number.is_float && + a->as.number.float_value == b->as.number.float_value; + return a->as.number.is_u64 == b->as.number.is_u64 && + a->as.number.int_value == b->as.number.int_value && + a->as.number.uint_value == b->as.number.uint_value; + case EXPR_STRING: + return a->as.string && b->as.string && + strcmp(a->as.string, b->as.string) == 0; + case EXPR_BOOL: + return a->as.boolean == b->as.boolean; + case EXPR_RUNE: + return a->as.rune == b->as.rune; + case EXPR_GET_PROPERTY: + return a->as.get_property.property && b->as.get_property.property && + strcmp(a->as.get_property.property, b->as.get_property.property) == 0 && + lint_pure_equal(a->as.get_property.object, b->as.get_property.object); + case EXPR_INDEX: + return lint_pure_equal(a->as.index.object, b->as.index.object) && + lint_pure_equal(a->as.index.index, b->as.index.index); + default: + return 0; + } +} + +/* + * Does control unconditionally leave the program point after this statement + * (so anything textually following it in the same block is unreachable)? + * Only cases the compiler is certain about are claimed; loops, switch, and + * try are treated as falling through to avoid false "unreachable" reports. + */ +static int lint_stmt_diverges(Stmt *s) { + if (!s) return 0; + switch (s->type) { + case STMT_RETURN: + case STMT_BREAK: + case STMT_CONTINUE: + case STMT_THROW: + return 1; + case STMT_EXPR: + return lint_is_panic_call(s->as.expr); + case STMT_BLOCK: + for (int i = 0; i < s->as.block.count; i++) + if (lint_stmt_diverges(s->as.block.statements[i])) return 1; + return 0; + case STMT_IF: + return s->as.if_stmt.else_branch && + lint_stmt_diverges(s->as.if_stmt.then_branch) && + lint_stmt_diverges(s->as.if_stmt.else_branch); + default: + return 0; + } +} + +/* A human-readable name for the diverging statement, used in messages. */ +static const char *lint_diverge_word(Stmt *s) { + switch (s->type) { + case STMT_RETURN: return "return"; + case STMT_BREAK: return "break"; + case STMT_CONTINUE: return "continue"; + case STMT_THROW: return "throw"; + case STMT_EXPR: return "panic()"; + case STMT_IF: return "if/else that always diverges"; + case STMT_BLOCK: return "block that always diverges"; + default: return "diverging statement"; + } +} + +/* + * Declarations that are hoisted (visible regardless of textual position) and + * therefore not "unreachable" even when they trail a diverging statement: + * type/enum/object definitions, imports/exports, extern fns, and named + * function bindings (`let f = fn ...`). + */ +static int lint_is_hoisted_decl(Stmt *s) { + if (!s) return 1; + switch (s->type) { + case STMT_DEFINE_OBJECT: + case STMT_ENUM: + case STMT_TYPE_ALIAS: + case STMT_IMPORT: + case STMT_EXPORT: + case STMT_IMPORT_FFI: + case STMT_EXTERN_FN: + return 1; + case STMT_LET: + return s->as.let.value && s->as.let.value->type == EXPR_FUNCTION; + case STMT_CONST: + return s->as.const_stmt.value && s->as.const_stmt.value->type == EXPR_FUNCTION; + default: + return 0; + } +} + +/* ========== READ COUNTING (for unused-variable detection) ========== */ + +static int lint_count_reads_expr(Expr *e, const char *name); +static int lint_count_reads_stmt(Stmt *s, const char *name); + +static int lint_count_reads_expr(Expr *e, const char *name) { + if (!e) return 0; + int n = 0; + switch (e->type) { + case EXPR_IDENT: + if (e->as.ident.name && strcmp(e->as.ident.name, name) == 0) n++; + break; + case EXPR_BINARY: + n += lint_count_reads_expr(e->as.binary.left, name); + n += lint_count_reads_expr(e->as.binary.right, name); + break; + case EXPR_UNARY: + n += lint_count_reads_expr(e->as.unary.operand, name); + break; + case EXPR_TERNARY: + n += lint_count_reads_expr(e->as.ternary.condition, name); + n += lint_count_reads_expr(e->as.ternary.true_expr, name); + n += lint_count_reads_expr(e->as.ternary.false_expr, name); + break; + case EXPR_CALL: + n += lint_count_reads_expr(e->as.call.func, name); + for (int i = 0; i < e->as.call.num_args; i++) + n += lint_count_reads_expr(e->as.call.args[i], name); + break; + case EXPR_ASSIGN: + /* The assignment target is a write, but the value is a read, and + * the target name itself counts as a use of the binding. */ + if (e->as.assign.name && strcmp(e->as.assign.name, name) == 0) n++; + n += lint_count_reads_expr(e->as.assign.value, name); + break; + case EXPR_GET_PROPERTY: + n += lint_count_reads_expr(e->as.get_property.object, name); + break; + case EXPR_SET_PROPERTY: + n += lint_count_reads_expr(e->as.set_property.object, name); + n += lint_count_reads_expr(e->as.set_property.value, name); + break; + case EXPR_INDEX: + n += lint_count_reads_expr(e->as.index.object, name); + n += lint_count_reads_expr(e->as.index.index, name); + break; + case EXPR_INDEX_ASSIGN: + n += lint_count_reads_expr(e->as.index_assign.object, name); + n += lint_count_reads_expr(e->as.index_assign.index, name); + n += lint_count_reads_expr(e->as.index_assign.value, name); + break; + case EXPR_FUNCTION: + n += lint_count_reads_stmt(e->as.function.body, name); + break; + case EXPR_ARRAY_LITERAL: + for (int i = 0; i < e->as.array_literal.num_elements; i++) + n += lint_count_reads_expr(e->as.array_literal.elements[i], name); + break; + case EXPR_OBJECT_LITERAL: + for (int i = 0; i < e->as.object_literal.num_fields; i++) + n += lint_count_reads_expr(e->as.object_literal.field_values[i], name); + break; + case EXPR_PREFIX_INC: n += lint_count_reads_expr(e->as.prefix_inc.operand, name); break; + case EXPR_PREFIX_DEC: n += lint_count_reads_expr(e->as.prefix_dec.operand, name); break; + case EXPR_POSTFIX_INC: n += lint_count_reads_expr(e->as.postfix_inc.operand, name); break; + case EXPR_POSTFIX_DEC: n += lint_count_reads_expr(e->as.postfix_dec.operand, name); break; + case EXPR_AWAIT: + n += lint_count_reads_expr(e->as.await_expr.awaited_expr, name); + break; + case EXPR_STRING_INTERPOLATION: + for (int i = 0; i < e->as.string_interpolation.num_parts; i++) + n += lint_count_reads_expr(e->as.string_interpolation.expr_parts[i], name); + break; + case EXPR_OPTIONAL_CHAIN: + n += lint_count_reads_expr(e->as.optional_chain.object, name); + n += lint_count_reads_expr(e->as.optional_chain.index, name); + for (int i = 0; i < e->as.optional_chain.num_args; i++) + n += lint_count_reads_expr(e->as.optional_chain.args[i], name); + break; + case EXPR_NULL_COALESCE: + n += lint_count_reads_expr(e->as.null_coalesce.left, name); + n += lint_count_reads_expr(e->as.null_coalesce.right, name); + break; + case EXPR_MATCH: + n += lint_count_reads_expr(e->as.match_expr.scrutinee, name); + for (int i = 0; i < e->as.match_expr.num_arms; i++) { + n += lint_count_reads_expr(e->as.match_expr.arms[i].guard, name); + n += lint_count_reads_expr(e->as.match_expr.arms[i].body, name); + } + break; + default: + break; + } + return n; +} + +static int lint_count_reads_stmt(Stmt *s, const char *name) { + if (!s) return 0; + int n = 0; + switch (s->type) { + case STMT_LET: n += lint_count_reads_expr(s->as.let.value, name); break; + case STMT_CONST: n += lint_count_reads_expr(s->as.const_stmt.value, name); break; + case STMT_EXPR: n += lint_count_reads_expr(s->as.expr, name); break; + case STMT_IF: + n += lint_count_reads_expr(s->as.if_stmt.condition, name); + n += lint_count_reads_stmt(s->as.if_stmt.then_branch, name); + n += lint_count_reads_stmt(s->as.if_stmt.else_branch, name); + break; + case STMT_WHILE: + n += lint_count_reads_expr(s->as.while_stmt.condition, name); + n += lint_count_reads_stmt(s->as.while_stmt.body, name); + break; + case STMT_LOOP: + n += lint_count_reads_stmt(s->as.loop_stmt.body, name); + break; + case STMT_FOR: + n += lint_count_reads_stmt(s->as.for_loop.initializer, name); + n += lint_count_reads_expr(s->as.for_loop.condition, name); + n += lint_count_reads_expr(s->as.for_loop.increment, name); + n += lint_count_reads_stmt(s->as.for_loop.body, name); + break; + case STMT_FOR_IN: + n += lint_count_reads_expr(s->as.for_in.iterable, name); + n += lint_count_reads_stmt(s->as.for_in.body, name); + break; + case STMT_BLOCK: + for (int i = 0; i < s->as.block.count; i++) + n += lint_count_reads_stmt(s->as.block.statements[i], name); + break; + case STMT_RETURN: n += lint_count_reads_expr(s->as.return_stmt.value, name); break; + case STMT_THROW: n += lint_count_reads_expr(s->as.throw_stmt.value, name); break; + case STMT_TRY: + n += lint_count_reads_stmt(s->as.try_stmt.try_block, name); + n += lint_count_reads_stmt(s->as.try_stmt.catch_block, name); + n += lint_count_reads_stmt(s->as.try_stmt.finally_block, name); + break; + case STMT_SWITCH: + n += lint_count_reads_expr(s->as.switch_stmt.expr, name); + for (int i = 0; i < s->as.switch_stmt.num_cases; i++) { + n += lint_count_reads_expr(s->as.switch_stmt.case_values[i], name); + n += lint_count_reads_stmt(s->as.switch_stmt.case_bodies[i], name); + } + break; + case STMT_DEFER: n += lint_count_reads_expr(s->as.defer_stmt.call, name); break; + case STMT_EXPORT: + if (s->as.export_stmt.declaration) + n += lint_count_reads_stmt(s->as.export_stmt.declaration, name); + break; + case STMT_DEFINE_OBJECT: + for (int i = 0; i < s->as.define_object.num_fields; i++) + n += lint_count_reads_expr(s->as.define_object.field_defaults[i], name); + for (int i = 0; i < s->as.define_object.num_methods; i++) + n += lint_count_reads_expr(s->as.define_object.method_defaults[i], name); + break; + case STMT_ENUM: + for (int i = 0; i < s->as.enum_decl.num_variants; i++) + n += lint_count_reads_expr(s->as.enum_decl.variant_values[i], name); + break; + default: + break; + } + return n; +} + +/* ========== CHECKS ========== */ + +/* self-assignment: `x = x`, `obj.f = obj.f`, `a[i] = a[i]`. */ +static void lint_check_self_assign(LintContext *ctx, Expr *e) { + switch (e->type) { + case EXPR_ASSIGN: { + Expr *v = e->as.assign.value; + if (v && v->type == EXPR_IDENT && v->as.ident.name && e->as.assign.name && + strcmp(v->as.ident.name, e->as.assign.name) == 0) + lint_warn(ctx, lint_eline(ctx, e), + "self-assignment: '%s = %s' has no effect", + e->as.assign.name, e->as.assign.name); + break; + } + case EXPR_SET_PROPERTY: { + Expr *v = e->as.set_property.value; + if (v && v->type == EXPR_GET_PROPERTY && v->as.get_property.property && + e->as.set_property.property && + strcmp(v->as.get_property.property, e->as.set_property.property) == 0 && + lint_pure_equal(v->as.get_property.object, e->as.set_property.object)) + lint_warn(ctx, lint_eline(ctx, e), + "self-assignment: assigning property '.%s' to itself has no effect", + e->as.set_property.property); + break; + } + case EXPR_INDEX_ASSIGN: { + Expr *v = e->as.index_assign.value; + if (v && v->type == EXPR_INDEX && + lint_pure_equal(v->as.index.object, e->as.index_assign.object) && + lint_pure_equal(v->as.index.index, e->as.index_assign.index)) + lint_warn(ctx, lint_eline(ctx, e), + "self-assignment: assigning an element to itself has no effect"); + break; + } + default: + break; + } +} + +/* A constant `if`/`while` condition: the branch/loop is dead or redundant. */ +static void lint_check_const_condition(LintContext *ctx, Expr *cond, + int has_else, int is_while, int line) { + int truthy; + if (!lint_const_truth(cond, &truthy)) return; + if (is_while) { + if (!truthy) + lint_warn(ctx, line, + "loop condition is always false; the loop body never executes"); + /* `while (true)` is a deliberate idiom — say nothing. */ + } else { + if (!truthy) + lint_warn(ctx, line, + "condition is always false; this branch never executes"); + else if (has_else) + lint_warn(ctx, line, + "condition is always true; the else branch never executes"); + } +} + +/* ========== RECURSIVE WALK ========== */ + +static void lint_expr(LintContext *ctx, Expr *e); +static void lint_stmt(LintContext *ctx, Stmt *s); +static void lint_stmt_list(LintContext *ctx, Stmt **stmts, int count); + +static void lint_expr(LintContext *ctx, Expr *e) { + if (!e) return; + switch (e->type) { + case EXPR_BINARY: + /* modulo by a literal zero traps at runtime. */ + if (e->as.binary.op == OP_MOD && lint_is_int_zero(e->as.binary.right)) + lint_warn(ctx, lint_eline(ctx, e), + "modulo by zero: this operation traps at runtime"); + lint_expr(ctx, e->as.binary.left); + lint_expr(ctx, e->as.binary.right); + break; + case EXPR_UNARY: + lint_expr(ctx, e->as.unary.operand); + break; + case EXPR_TERNARY: + lint_expr(ctx, e->as.ternary.condition); + lint_expr(ctx, e->as.ternary.true_expr); + lint_expr(ctx, e->as.ternary.false_expr); + break; + case EXPR_CALL: + lint_expr(ctx, e->as.call.func); + for (int i = 0; i < e->as.call.num_args; i++) + lint_expr(ctx, e->as.call.args[i]); + break; + case EXPR_ASSIGN: + lint_check_self_assign(ctx, e); + lint_expr(ctx, e->as.assign.value); + break; + case EXPR_GET_PROPERTY: + lint_expr(ctx, e->as.get_property.object); + break; + case EXPR_SET_PROPERTY: + lint_check_self_assign(ctx, e); + lint_expr(ctx, e->as.set_property.object); + lint_expr(ctx, e->as.set_property.value); + break; + case EXPR_INDEX: + lint_expr(ctx, e->as.index.object); + lint_expr(ctx, e->as.index.index); + break; + case EXPR_INDEX_ASSIGN: + lint_check_self_assign(ctx, e); + lint_expr(ctx, e->as.index_assign.object); + lint_expr(ctx, e->as.index_assign.index); + lint_expr(ctx, e->as.index_assign.value); + break; + case EXPR_FUNCTION: + lint_stmt(ctx, e->as.function.body); + break; + case EXPR_ARRAY_LITERAL: + for (int i = 0; i < e->as.array_literal.num_elements; i++) + lint_expr(ctx, e->as.array_literal.elements[i]); + break; + case EXPR_OBJECT_LITERAL: + for (int i = 0; i < e->as.object_literal.num_fields; i++) + lint_expr(ctx, e->as.object_literal.field_values[i]); + break; + case EXPR_PREFIX_INC: lint_expr(ctx, e->as.prefix_inc.operand); break; + case EXPR_PREFIX_DEC: lint_expr(ctx, e->as.prefix_dec.operand); break; + case EXPR_POSTFIX_INC: lint_expr(ctx, e->as.postfix_inc.operand); break; + case EXPR_POSTFIX_DEC: lint_expr(ctx, e->as.postfix_dec.operand); break; + case EXPR_AWAIT: + lint_expr(ctx, e->as.await_expr.awaited_expr); + break; + case EXPR_STRING_INTERPOLATION: + for (int i = 0; i < e->as.string_interpolation.num_parts; i++) + lint_expr(ctx, e->as.string_interpolation.expr_parts[i]); + break; + case EXPR_OPTIONAL_CHAIN: + lint_expr(ctx, e->as.optional_chain.object); + lint_expr(ctx, e->as.optional_chain.index); + for (int i = 0; i < e->as.optional_chain.num_args; i++) + lint_expr(ctx, e->as.optional_chain.args[i]); + break; + case EXPR_NULL_COALESCE: + lint_expr(ctx, e->as.null_coalesce.left); + lint_expr(ctx, e->as.null_coalesce.right); + break; + case EXPR_MATCH: + lint_expr(ctx, e->as.match_expr.scrutinee); + for (int i = 0; i < e->as.match_expr.num_arms; i++) { + lint_expr(ctx, e->as.match_expr.arms[i].guard); + lint_expr(ctx, e->as.match_expr.arms[i].body); + } + break; + default: + break; + } +} + +/* unused variables: a let/const, declared directly in this scope, that is + * never read anywhere in the scope. Strict mode only. Names beginning with + * '_' and function bindings are exempt by convention. */ +static void lint_check_unused(LintContext *ctx, Stmt **stmts, int count) { + for (int i = 0; i < count; i++) { + Stmt *s = stmts[i]; + const char *name = NULL; + Expr *val = NULL; + int line = s ? s->line : 0; + if (s && s->type == STMT_LET) { name = s->as.let.name; val = s->as.let.value; } + else if (s && s->type == STMT_CONST) { name = s->as.const_stmt.name; val = s->as.const_stmt.value; } + else continue; + + if (!name || name[0] == '_') continue; + if (val && val->type == EXPR_FUNCTION) continue; + + int reads = 0; + for (int j = 0; j < count && reads == 0; j++) { + if (j == i) continue; /* the declaration itself is not a use */ + reads += lint_count_reads_stmt(stmts[j], name); + } + if (reads == 0) + lint_warn(ctx, line, "variable '%s' is declared but never used", name); + } +} + +/* Report the first unreachable statement in a block (one warning per block). */ +static void lint_check_unreachable(LintContext *ctx, Stmt **stmts, int count) { + int diverged_at = -1; + for (int i = 0; i < count; i++) { + Stmt *s = stmts[i]; + if (diverged_at >= 0 && !lint_is_hoisted_decl(s)) { + lint_warn(ctx, s->line, + "unreachable code: control always leaves via the %s on line %d", + lint_diverge_word(stmts[diverged_at]), stmts[diverged_at]->line); + return; + } + if (diverged_at < 0 && lint_stmt_diverges(s)) diverged_at = i; + } +} + +static void lint_stmt_list(LintContext *ctx, Stmt **stmts, int count) { + if (!stmts) return; + lint_check_unreachable(ctx, stmts, count); + if (ctx->strict) lint_check_unused(ctx, stmts, count); + for (int i = 0; i < count; i++) lint_stmt(ctx, stmts[i]); +} + +static void lint_stmt(LintContext *ctx, Stmt *s) { + if (!s) return; + if (s->line > 0) ctx->cur_line = s->line; + switch (s->type) { + case STMT_LET: lint_expr(ctx, s->as.let.value); break; + case STMT_CONST: lint_expr(ctx, s->as.const_stmt.value); break; + case STMT_EXPR: lint_expr(ctx, s->as.expr); break; + case STMT_IF: + lint_expr(ctx, s->as.if_stmt.condition); + lint_check_const_condition(ctx, s->as.if_stmt.condition, + s->as.if_stmt.else_branch != NULL, 0, s->line); + lint_stmt(ctx, s->as.if_stmt.then_branch); + lint_stmt(ctx, s->as.if_stmt.else_branch); + break; + case STMT_WHILE: + lint_expr(ctx, s->as.while_stmt.condition); + lint_check_const_condition(ctx, s->as.while_stmt.condition, 0, 1, s->line); + lint_stmt(ctx, s->as.while_stmt.body); + break; + case STMT_LOOP: + lint_stmt(ctx, s->as.loop_stmt.body); + break; + case STMT_FOR: + lint_stmt(ctx, s->as.for_loop.initializer); + lint_expr(ctx, s->as.for_loop.condition); + lint_expr(ctx, s->as.for_loop.increment); + lint_stmt(ctx, s->as.for_loop.body); + break; + case STMT_FOR_IN: + lint_expr(ctx, s->as.for_in.iterable); + lint_stmt(ctx, s->as.for_in.body); + break; + case STMT_BLOCK: + lint_stmt_list(ctx, s->as.block.statements, s->as.block.count); + break; + case STMT_RETURN: lint_expr(ctx, s->as.return_stmt.value); break; + case STMT_THROW: lint_expr(ctx, s->as.throw_stmt.value); break; + case STMT_TRY: + lint_stmt(ctx, s->as.try_stmt.try_block); + lint_stmt(ctx, s->as.try_stmt.catch_block); + lint_stmt(ctx, s->as.try_stmt.finally_block); + break; + case STMT_SWITCH: + lint_expr(ctx, s->as.switch_stmt.expr); + for (int i = 0; i < s->as.switch_stmt.num_cases; i++) { + lint_expr(ctx, s->as.switch_stmt.case_values[i]); + lint_stmt(ctx, s->as.switch_stmt.case_bodies[i]); + } + break; + case STMT_DEFER: lint_expr(ctx, s->as.defer_stmt.call); break; + case STMT_EXPORT: + if (s->as.export_stmt.declaration) + lint_stmt(ctx, s->as.export_stmt.declaration); + break; + case STMT_DEFINE_OBJECT: + for (int i = 0; i < s->as.define_object.num_fields; i++) + lint_expr(ctx, s->as.define_object.field_defaults[i]); + for (int i = 0; i < s->as.define_object.num_methods; i++) + lint_expr(ctx, s->as.define_object.method_defaults[i]); + break; + case STMT_ENUM: + for (int i = 0; i < s->as.enum_decl.num_variants; i++) + lint_expr(ctx, s->as.enum_decl.variant_values[i]); + break; + default: + break; + } +} + +/* ========== PUBLIC API ========== */ + +LintContext *lint_new(const char *filename) { + LintContext *ctx = calloc(1, sizeof(LintContext)); + if (!ctx) return NULL; + ctx->filename = filename; + return ctx; +} + +void lint_free(LintContext *ctx) { + if (!ctx) return; + LintDiag *d = ctx->diags; + while (d) { + LintDiag *next = d->next; + free(d->message); + free(d); + d = next; + } + free(ctx); +} + +void lint_enable_collection(LintContext *ctx, const char *source) { + if (!ctx) return; + ctx->collect = 1; + ctx->source = source; +} + +int lint_program(LintContext *ctx, Stmt **stmts, int stmt_count) { + if (!ctx || !stmts) return 0; + lint_stmt_list(ctx, stmts, stmt_count); + return ctx->error_count; +} diff --git a/src/backends/compiler/main.c b/src/backends/compiler/main.c index 868d592d..671286e5 100644 --- a/src/backends/compiler/main.c +++ b/src/backends/compiler/main.c @@ -28,6 +28,7 @@ #include "codegen.h" #include "compiler/type_check.h" #include "compiler/borrow_check.h" +#include "compiler/lint.h" #define HEMLOCK_BUILD_DATE __DATE__ #define HEMLOCK_BUILD_TIME __TIME__ @@ -247,6 +248,9 @@ typedef struct { int borrow_check; // Enable Rust-like ownership/borrow checking (default: on) int borrow_strict; // Strict borrow checking (move tracking + leak detection) int borrow_error; // Treat borrow-check findings as errors (fail the build) + int lint; // Enable static lint / diagnostics pass (default: on) + int lint_strict; // Strict lint (also flag unused variables) + int lint_error; // Treat lint findings as errors (fail the build) int check_only; // Only type check, don't compile int static_link; // Static link all libraries for standalone binary int stack_check; // Enable stack overflow checking (default: on) @@ -277,6 +281,9 @@ static void print_usage(const char *progname) { fprintf(stderr, " --no-borrow-check Disable Rust-like ownership/borrow checking\n"); fprintf(stderr, " --borrow-strict Strict borrow checking (move tracking + leak detection)\n"); fprintf(stderr, " --borrow-error Treat borrow-check findings as errors (fail the build)\n"); + fprintf(stderr, " --no-lint Disable static lint pass (unreachable code, dead branches, ...)\n"); + fprintf(stderr, " --lint-strict Strict lint (also flag unused variables)\n"); + fprintf(stderr, " --lint-error Treat lint findings as errors (fail the build)\n"); fprintf(stderr, " --no-stack-check Disable stack overflow checking (faster, but no protection)\n"); fprintf(stderr, " --static Static-link the volatile native libs (default on Linux)\n"); fprintf(stderr, " --dynamic Dynamic-link instead (default on macOS; needed for runtime FFI)\n"); @@ -310,6 +317,9 @@ static Options parse_args(int argc, char **argv) { .borrow_check = 1, // Borrow checking ON by default (advisory warnings) .borrow_strict = 0, .borrow_error = 0, + .lint = 1, // Lint pass ON by default (advisory warnings) + .lint_strict = 0, + .lint_error = 0, .check_only = 0, .static_link = 0, .stack_check = 1, // Stack overflow checking ON by default @@ -373,6 +383,14 @@ static Options parse_args(int argc, char **argv) { } else if (strcmp(argv[i], "--borrow-error") == 0) { opts.borrow_check = 1; opts.borrow_error = 1; + } else if (strcmp(argv[i], "--no-lint") == 0) { + opts.lint = 0; + } else if (strcmp(argv[i], "--lint-strict") == 0) { + opts.lint = 1; + opts.lint_strict = 1; + } else if (strcmp(argv[i], "--lint-error") == 0) { + opts.lint = 1; + opts.lint_error = 1; } else if (strcmp(argv[i], "--static") == 0) { opts.static_link = 1; // default since 2.6.0; kept as an explicit no-op } else if (strcmp(argv[i], "--dynamic") == 0) { @@ -910,6 +928,38 @@ int main(int argc, char **argv) { } resolve_program(statements, stmt_count); + // Static lint / diagnostics (advisory by default). Flags well-typed, + // memory-safe code that is almost certainly a mistake: unreachable code, + // dead branches, self-assignment, modulo-by-zero, and (strict) unused + // variables. Runs on the source as written — before the optimizer can + // fold away dead branches — and is non-fatal unless --lint-error is given. + if (opts.lint) { + if (opts.verbose) { + printf("Linting...\n"); + } + LintContext *lc = lint_new(opts.input_file); + if (lc) { + lc->strict = opts.lint_strict; + lc->errors_are_fatal = opts.lint_error; + int lint_errors = lint_program(lc, statements, stmt_count); + if (opts.verbose && lc->warning_count == 0 && lc->error_count == 0) { + printf("Lint passed\n"); + } + lint_free(lc); + + if (lint_errors > 0) { + fprintf(stderr, "%d lint error%s found\n", + lint_errors, lint_errors > 1 ? "s" : ""); + for (int i = 0; i < stmt_count; i++) { + stmt_free(statements[i]); + } + free(statements); + free(source); + return 1; + } + } + } + // Optimize AST (constant folding, boolean simplification, strength reduction) // This runs before type checking to simplify patterns for analysis if (opts.verbose) { diff --git a/tests/borrow/run_borrow_tests.sh b/tests/borrow/run_borrow_tests.sh index 18df051d..001a41bd 100755 --- a/tests/borrow/run_borrow_tests.sh +++ b/tests/borrow/run_borrow_tests.sh @@ -67,7 +67,8 @@ for test_file in *.hml; do [ -f "$flags_file" ] && extra_flags="$(cat "$flags_file")" # Compile to C only; capture stderr diagnostics, discard stdout/C output. - actual=$("$HEMLOCKC" $extra_flags -c --emit-c "$TMP_C" "$test_file" 2>&1 1>/dev/null) + # Disable the lint pass so only borrow-checker diagnostics are compared. + actual=$("$HEMLOCKC" --no-lint $extra_flags -c --emit-c "$TMP_C" "$test_file" 2>&1 1>/dev/null) expected=$(cat "$expected_file") if [ "$actual" = "$expected" ]; then diff --git a/tests/lint/dead_branch.expected b/tests/lint/dead_branch.expected new file mode 100644 index 00000000..4766ed27 --- /dev/null +++ b/tests/lint/dead_branch.expected @@ -0,0 +1,2 @@ +dead_branch.hml:2: warning: condition is always false; this branch never executes +dead_branch.hml:5: warning: condition is always true; the else branch never executes diff --git a/tests/lint/dead_branch.hml b/tests/lint/dead_branch.hml new file mode 100644 index 00000000..5b4d1e48 --- /dev/null +++ b/tests/lint/dead_branch.hml @@ -0,0 +1,11 @@ +fn f(): i32 { + if (false) { + return 1; + } + if (true) { + return 2; + } else { + return 3; + } +} +print(f()); diff --git a/tests/lint/modulo_zero.expected b/tests/lint/modulo_zero.expected new file mode 100644 index 00000000..255dace9 --- /dev/null +++ b/tests/lint/modulo_zero.expected @@ -0,0 +1 @@ +modulo_zero.hml:3: warning: modulo by zero: this operation traps at runtime diff --git a/tests/lint/modulo_zero.hml b/tests/lint/modulo_zero.hml new file mode 100644 index 00000000..fd5f62e3 --- /dev/null +++ b/tests/lint/modulo_zero.hml @@ -0,0 +1,5 @@ +fn r(): i32 { + let n = 10; + return n % 0; +} +print(r()); diff --git a/tests/lint/neg_clean.expected b/tests/lint/neg_clean.expected new file mode 100644 index 00000000..e69de29b diff --git a/tests/lint/neg_clean.hml b/tests/lint/neg_clean.hml new file mode 100644 index 00000000..8ad7116e --- /dev/null +++ b/tests/lint/neg_clean.hml @@ -0,0 +1,5 @@ +fn add(a: i32, b: i32): i32 { + let sum = a + b; + return sum; +} +print(add(2, 3)); diff --git a/tests/lint/neg_index_sideeffect.expected b/tests/lint/neg_index_sideeffect.expected new file mode 100644 index 00000000..e69de29b diff --git a/tests/lint/neg_index_sideeffect.hml b/tests/lint/neg_index_sideeffect.hml new file mode 100644 index 00000000..77ad1bbf --- /dev/null +++ b/tests/lint/neg_index_sideeffect.hml @@ -0,0 +1,8 @@ +let calls = 0; +fn next(): i32 { + calls = calls + 1; + return calls % 2; +} +let a = [10, 20]; +a[next()] = a[next()]; +print(a[0]); diff --git a/tests/lint/neg_while_true.expected b/tests/lint/neg_while_true.expected new file mode 100644 index 00000000..e69de29b diff --git a/tests/lint/neg_while_true.hml b/tests/lint/neg_while_true.hml new file mode 100644 index 00000000..8cbe1b1d --- /dev/null +++ b/tests/lint/neg_while_true.hml @@ -0,0 +1,9 @@ +fn count(): i32 { + let i = 0; + while (true) { + i = i + 1; + if (i >= 3) { break; } + } + return i; +} +print(count()); diff --git a/tests/lint/run_lint_tests.sh b/tests/lint/run_lint_tests.sh new file mode 100755 index 00000000..4ae0eff9 --- /dev/null +++ b/tests/lint/run_lint_tests.sh @@ -0,0 +1,101 @@ +#!/bin/bash + +# Hemlock Static Lint Test Runner +# +# For each tests/lint/.hml there is a .expected file holding the +# lint diagnostics the compiler should emit (without the leading path, i.e. +# lines of the form ".hml:LINE: warning: ...", in order). An optional +# .flags file supplies extra hemlockc flags (e.g. --lint-strict). +# +# The compiler is invoked so it only emits C (no linking); stdout is discarded +# and stderr (the diagnostics) is captured and compared. Borrow checking is +# disabled so only lint diagnostics appear. The compiler is run from inside +# this directory so the path prefix is just ".hml". +# +# Convention: fixtures named neg_*.hml are "negative" cases whose .expected +# file is empty — they assert the linter produces NO diagnostic and so guard +# against false positives. All other fixtures assert an exact diagnostic. + +RED='\033[0;31m' +GREEN='\033[0;32m' +BLUE='\033[0;34m' +NC='\033[0m' + +PASS_COUNT=0 +FAIL_COUNT=0 + +echo "======================================" +echo " Hemlock Lint Test Suite" +echo "======================================" +echo "" + +# Detect project root +if [ -f "../../Makefile" ]; then + PROJECT_ROOT="$(cd ../.. && pwd)" +elif [ -f "Makefile" ]; then + PROJECT_ROOT="$(pwd)" +else + echo -e "${RED}✗ Cannot find Makefile${NC}" + exit 1 +fi + +HEMLOCKC="$PROJECT_ROOT/hemlockc" +TEST_DIR="$PROJECT_ROOT/tests/lint" + +if [ ! -x "$HEMLOCKC" ]; then + echo -e "${BLUE}Building compiler...${NC}" + if ! make -C "$PROJECT_ROOT" compiler > /tmp/lint_build.log 2>&1; then + echo -e "${RED}✗ Build failed${NC}" + cat /tmp/lint_build.log + exit 1 + fi +fi + +cd "$TEST_DIR" || exit 1 +TMP_C=$(mktemp) +trap "rm -f $TMP_C" EXIT + +for test_file in *.hml; do + [ -f "$test_file" ] || continue + test_name="${test_file%.hml}" + expected_file="${test_name}.expected" + flags_file="${test_name}.flags" + + [ -f "$expected_file" ] || continue + + extra_flags="" + [ -f "$flags_file" ] && extra_flags="$(cat "$flags_file")" + + # Compile to C only; capture stderr diagnostics, discard stdout/C output. + # Disable borrow checking so only lint diagnostics are compared. + actual=$("$HEMLOCKC" --no-borrow-check $extra_flags -c --emit-c "$TMP_C" "$test_file" 2>&1 1>/dev/null) + expected=$(cat "$expected_file") + + if [ "$actual" = "$expected" ]; then + echo -e "${GREEN}✓${NC} $test_name" + ((PASS_COUNT++)) + else + echo -e "${RED}✗${NC} $test_name" + echo " Expected:" + echo "$expected" | sed 's/^/ /' + echo " Actual:" + echo "$actual" | sed 's/^/ /' + ((FAIL_COUNT++)) + fi +done + +echo "" +echo "======================================" +echo " Summary" +echo "======================================" +echo -e "${GREEN}Passed:${NC} $PASS_COUNT" +echo -e "${RED}Failed:${NC} $FAIL_COUNT" +echo "======================================" + +if [ $FAIL_COUNT -eq 0 ]; then + echo -e "${GREEN}All lint tests passed! 🎉${NC}" + exit 0 +else + echo -e "${RED}Some lint tests failed.${NC}" + exit 1 +fi diff --git a/tests/lint/self_assign.expected b/tests/lint/self_assign.expected new file mode 100644 index 00000000..3abff64b --- /dev/null +++ b/tests/lint/self_assign.expected @@ -0,0 +1,2 @@ +self_assign.hml:3: warning: self-assignment: 'x = x' has no effect +self_assign.hml:5: warning: self-assignment: assigning property '.a' to itself has no effect diff --git a/tests/lint/self_assign.hml b/tests/lint/self_assign.hml new file mode 100644 index 00000000..0774a6de --- /dev/null +++ b/tests/lint/self_assign.hml @@ -0,0 +1,7 @@ +fn main_fn() { + let x = 5; + x = x; + let obj = { a: 1 }; + obj.a = obj.a; +} +main_fn(); diff --git a/tests/lint/unreachable.expected b/tests/lint/unreachable.expected new file mode 100644 index 00000000..4e116ae8 --- /dev/null +++ b/tests/lint/unreachable.expected @@ -0,0 +1 @@ +unreachable.hml:4: warning: unreachable code: control always leaves via the return on line 3 diff --git a/tests/lint/unreachable.hml b/tests/lint/unreachable.hml new file mode 100644 index 00000000..496cef0a --- /dev/null +++ b/tests/lint/unreachable.hml @@ -0,0 +1,8 @@ +fn classify(n: i32): string { + if (n > 0) { + return "positive"; + print("dead"); + } + return "other"; +} +print(classify(1)); diff --git a/tests/lint/unused.expected b/tests/lint/unused.expected new file mode 100644 index 00000000..53c96007 --- /dev/null +++ b/tests/lint/unused.expected @@ -0,0 +1 @@ +unused.hml:3: warning: variable 'unused' is declared but never used diff --git a/tests/lint/unused.flags b/tests/lint/unused.flags new file mode 100644 index 00000000..a1e2b140 --- /dev/null +++ b/tests/lint/unused.flags @@ -0,0 +1 @@ +--lint-strict \ No newline at end of file diff --git a/tests/lint/unused.hml b/tests/lint/unused.hml new file mode 100644 index 00000000..026d0d20 --- /dev/null +++ b/tests/lint/unused.hml @@ -0,0 +1,7 @@ +fn f(): i32 { + let used = 1; + let unused = 2; + let _ignored = 3; + return used; +} +print(f()); From e4658ad919de0f149575b0d9c6d5bcb32926543c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 14:10:50 +0000 Subject: [PATCH 2/3] lint: fix CI test sweep + add self-compare, dup-field, dup-case checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01WKLiroxGKnQhJBa43UbHJT --- docs/advanced/lint.md | 8 +- include/compiler/lint.h | 5 + src/backends/compiler/lint.c | 105 ++++++++++++++++++++ tests/lint/dup_case.expected | 1 + tests/lint/dup_case.hml | 9 ++ tests/lint/dup_field.expected | 1 + tests/lint/dup_field.hml | 2 + tests/lint/neg_nan_idiom.expected | 0 tests/lint/neg_nan_idiom.hml | 8 ++ tests/lint/neg_spread_and_computed.expected | 0 tests/lint/neg_spread_and_computed.hml | 13 +++ tests/lint/self_compare.expected | 4 + tests/lint/self_compare.hml | 8 ++ tests/run_full_parity.sh | 3 +- tests/run_tests.sh | 5 +- 15 files changed, 168 insertions(+), 4 deletions(-) create mode 100644 tests/lint/dup_case.expected create mode 100644 tests/lint/dup_case.hml create mode 100644 tests/lint/dup_field.expected create mode 100644 tests/lint/dup_field.hml create mode 100644 tests/lint/neg_nan_idiom.expected create mode 100644 tests/lint/neg_nan_idiom.hml create mode 100644 tests/lint/neg_spread_and_computed.expected create mode 100644 tests/lint/neg_spread_and_computed.hml create mode 100644 tests/lint/self_compare.expected create mode 100644 tests/lint/self_compare.hml diff --git a/docs/advanced/lint.md b/docs/advanced/lint.md index e1937ed7..959f6bde 100644 --- a/docs/advanced/lint.md +++ b/docs/advanced/lint.md @@ -42,13 +42,19 @@ about library code you did not write. | **dead branch** | an `if`/`while` condition is a constant (`if (false)`, `while (0)`) | on | | **redundant branch** | an `if (true) { … } else { … }` whose `else` can never run | on | | **self-assignment** | `x = x`, `obj.f = obj.f`, or `a[i] = a[i]` — a no-op | on | +| **self-comparison** | `x < x` / `x > x` (always false), `x && x` / `x \|\| x` (redundant) | on | | **modulo by zero** | `x % 0` with a literal zero divisor — traps at runtime | on | +| **duplicate field** | `{ a: 1, a: 2 }` — the later value silently wins | on | +| **duplicate case** | a repeated literal `switch` case — the second is unreachable | on | | **unused variable** | a `let`/`const` that is never read in its scope | strict only | Every diagnostic is something the compiler is **certain** about. The analysis is intentionally conservative — for example, self-assignment is only reported when both sides are side-effect-free, so `a[next()] = a[next()]` is left alone, and -`while (true)` (a deliberate idiom) is never flagged. There are no false +`while (true)` (a deliberate idiom) is never flagged. Self-comparison exempts +`==`, `!=`, `<=`, and `>=` entirely: for floats every comparison with NaN is +false, so `x == x` is *not* always true and `x != x` is the canonical NaN +check — only `<` and `>` (false even for NaN) are reported. There are no false positives to silence. ### Examples diff --git a/include/compiler/lint.h b/include/compiler/lint.h index 1fe6fd48..523ceb94 100644 --- a/include/compiler/lint.h +++ b/include/compiler/lint.h @@ -12,7 +12,12 @@ * or after an if/else where both arms always diverge) * - dead branches (`if`/`while` conditions that are constant) * - self-assignment (`x = x`, `obj.f = obj.f`, `a[i] = a[i]`) + * - self-comparison (`x < x`, `x > x`, `x && x`, `x || x` — always + * false / redundant; ==//!=/<=/>= are exempt since + * NaN makes them type-dependent) * - modulo by zero (`x % 0` with a literal zero — traps at runtime) + * - duplicate fields (`{ a: 1, a: 2 }` — the later value silently wins) + * - duplicate cases (repeated literal switch case — unreachable) * - unused variables (strict mode: a let/const that is never read) * * Only the program's own top-level module is analysed; imported modules are diff --git a/src/backends/compiler/lint.c b/src/backends/compiler/lint.c index 7141e93d..6a4f0927 100644 --- a/src/backends/compiler/lint.c +++ b/src/backends/compiler/lint.c @@ -401,6 +401,53 @@ static void lint_check_self_assign(LintContext *ctx, Expr *e) { } } +/* A bare literal (its self-comparison is constant-folder territory, and + * language tests exercise operators on literals deliberately). */ +static int lint_is_literal(Expr *e) { + if (!e) return 0; + switch (e->type) { + case EXPR_NUMBER: + case EXPR_STRING: + case EXPR_BOOL: + case EXPR_RUNE: + return 1; + default: + return 0; + } +} + +/* + * Comparison/combination of a variable with itself: `x < x` / `x > x` are + * always false, `x && x` / `x || x` are redundant. Only fires when both sides + * are pure and structurally identical (lint_pure_equal), so `a[next()] < + * a[next()]` is left alone, and not when both sides are bare literals (`1 < + * 1` is the constant folder's job and appears deliberately in operator + * tests). + * + * `==`, `!=`, `<=`, `>=` are deliberately NOT flagged: for floats, every + * comparison with NaN is false, so `x == x` / `x <= x` are NOT always true — + * `x != x` is the canonical NaN check — and the linter cannot see types. + * `<` and `>` are safe: false for NaN too, so "always false" always holds. + */ +static void lint_check_self_compare(LintContext *ctx, Expr *e) { + if (!lint_pure_equal(e->as.binary.left, e->as.binary.right)) return; + if (lint_is_literal(e->as.binary.left)) return; + switch (e->as.binary.op) { + case OP_LESS: + case OP_GREATER: + lint_warn(ctx, lint_eline(ctx, e), + "comparison of an expression with itself is always false"); + break; + case OP_AND: + case OP_OR: + lint_warn(ctx, lint_eline(ctx, e), + "logical operator with identical operands is redundant"); + break; + default: + break; + } +} + /* A constant `if`/`while` condition: the branch/loop is dead or redundant. */ static void lint_check_const_condition(LintContext *ctx, Expr *cond, int has_else, int is_while, int line) { @@ -421,6 +468,61 @@ static void lint_check_const_condition(LintContext *ctx, Expr *cond, } } +/* Duplicate field names in an object literal: `{ a: 1, a: 2 }` — the later + * value silently wins, so the first is a certain mistake. Spread entries + * (field_names[i] == NULL) are skipped. */ +static void lint_check_dup_fields(LintContext *ctx, Expr *e) { + int n = e->as.object_literal.num_fields; + for (int i = 0; i < n; i++) { + const char *name = e->as.object_literal.field_names[i]; + if (!name) continue; /* spread */ + for (int j = i + 1; j < n; j++) { + const char *other = e->as.object_literal.field_names[j]; + if (other && strcmp(name, other) == 0) { + lint_warn(ctx, lint_eline(ctx, e), + "duplicate field '%s' in object literal; the later value wins", + name); + break; /* one report per name */ + } + } + } +} + +/* A literal usable for exact duplicate-case comparison. */ +static int lint_is_case_literal(Expr *e) { + if (!e) return 0; + switch (e->type) { + case EXPR_NUMBER: + case EXPR_STRING: + case EXPR_BOOL: + case EXPR_RUNE: + return 1; + default: + return 0; + } +} + +/* Duplicate literal case values in a switch: the second case can never be + * reached. Only literal values are compared — identifiers or computed cases + * are left alone since their values are not known here. */ +static void lint_check_dup_cases(LintContext *ctx, Stmt *s) { + int n = s->as.switch_stmt.num_cases; + for (int i = 0; i < n; i++) { + Expr *a = s->as.switch_stmt.case_values[i]; + if (!lint_is_case_literal(a)) continue; /* default or computed */ + for (int j = i + 1; j < n; j++) { + Expr *b = s->as.switch_stmt.case_values[j]; + if (b && lint_is_case_literal(b) && lint_pure_equal(a, b)) { + lint_warn(ctx, b->line > 0 ? b->line : s->line, + "duplicate case value in switch; this case is unreachable" + " (first occurrence on line %d)", + a->line > 0 ? a->line : s->line); + break; /* one report per value */ + } + } + } +} + /* ========== RECURSIVE WALK ========== */ static void lint_expr(LintContext *ctx, Expr *e); @@ -435,6 +537,7 @@ static void lint_expr(LintContext *ctx, Expr *e) { if (e->as.binary.op == OP_MOD && lint_is_int_zero(e->as.binary.right)) lint_warn(ctx, lint_eline(ctx, e), "modulo by zero: this operation traps at runtime"); + lint_check_self_compare(ctx, e); lint_expr(ctx, e->as.binary.left); lint_expr(ctx, e->as.binary.right); break; @@ -481,6 +584,7 @@ static void lint_expr(LintContext *ctx, Expr *e) { lint_expr(ctx, e->as.array_literal.elements[i]); break; case EXPR_OBJECT_LITERAL: + lint_check_dup_fields(ctx, e); for (int i = 0; i < e->as.object_literal.num_fields; i++) lint_expr(ctx, e->as.object_literal.field_values[i]); break; @@ -609,6 +713,7 @@ static void lint_stmt(LintContext *ctx, Stmt *s) { break; case STMT_SWITCH: lint_expr(ctx, s->as.switch_stmt.expr); + lint_check_dup_cases(ctx, s); for (int i = 0; i < s->as.switch_stmt.num_cases; i++) { lint_expr(ctx, s->as.switch_stmt.case_values[i]); lint_stmt(ctx, s->as.switch_stmt.case_bodies[i]); diff --git a/tests/lint/dup_case.expected b/tests/lint/dup_case.expected new file mode 100644 index 00000000..7fa448eb --- /dev/null +++ b/tests/lint/dup_case.expected @@ -0,0 +1 @@ +dup_case.hml:5: warning: duplicate case value in switch; this case is unreachable (first occurrence on line 3) diff --git a/tests/lint/dup_case.hml b/tests/lint/dup_case.hml new file mode 100644 index 00000000..79203b8c --- /dev/null +++ b/tests/lint/dup_case.hml @@ -0,0 +1,9 @@ +fn name(n: i32): string { + switch (n) { + case 1: return "one"; + case 2: return "two"; + case 1: return "uno"; + default: return "other"; + } +} +print(name(1)); diff --git a/tests/lint/dup_field.expected b/tests/lint/dup_field.expected new file mode 100644 index 00000000..2170a173 --- /dev/null +++ b/tests/lint/dup_field.expected @@ -0,0 +1 @@ +dup_field.hml:1: warning: duplicate field 'a' in object literal; the later value wins diff --git a/tests/lint/dup_field.hml b/tests/lint/dup_field.hml new file mode 100644 index 00000000..368d045b --- /dev/null +++ b/tests/lint/dup_field.hml @@ -0,0 +1,2 @@ +let obj = { a: 1, b: 2, a: 3 }; +print(obj.a); diff --git a/tests/lint/neg_nan_idiom.expected b/tests/lint/neg_nan_idiom.expected new file mode 100644 index 00000000..e69de29b diff --git a/tests/lint/neg_nan_idiom.hml b/tests/lint/neg_nan_idiom.hml new file mode 100644 index 00000000..0f43b11b --- /dev/null +++ b/tests/lint/neg_nan_idiom.hml @@ -0,0 +1,8 @@ +fn is_nan(x: f64): bool { + return x != x; +} +fn same(x: f64): bool { + return x == x && x <= x && x >= x; +} +print(is_nan(0.0 / 0.0)); +print(same(1.5)); diff --git a/tests/lint/neg_spread_and_computed.expected b/tests/lint/neg_spread_and_computed.expected new file mode 100644 index 00000000..e69de29b diff --git a/tests/lint/neg_spread_and_computed.hml b/tests/lint/neg_spread_and_computed.hml new file mode 100644 index 00000000..afaba5c3 --- /dev/null +++ b/tests/lint/neg_spread_and_computed.hml @@ -0,0 +1,13 @@ +let base = { a: 1 }; +let obj = { ...base, a: 2 }; +let k1 = 1; +let k2 = 1; +fn pick(n: i32): string { + switch (n) { + case k1: return "first"; + case k2: return "second"; + default: return "other"; + } +} +print(obj.a); +print(pick(1)); diff --git a/tests/lint/self_compare.expected b/tests/lint/self_compare.expected new file mode 100644 index 00000000..f467e5ae --- /dev/null +++ b/tests/lint/self_compare.expected @@ -0,0 +1,4 @@ +self_compare.hml:2: warning: comparison of an expression with itself is always false +self_compare.hml:3: warning: comparison of an expression with itself is always false +self_compare.hml:4: warning: logical operator with identical operands is redundant +self_compare.hml:5: warning: logical operator with identical operands is redundant diff --git a/tests/lint/self_compare.hml b/tests/lint/self_compare.hml new file mode 100644 index 00000000..e0692d03 --- /dev/null +++ b/tests/lint/self_compare.hml @@ -0,0 +1,8 @@ +fn f(x: i32, flag: bool): bool { + if (x < x) { print("never"); } + if (x > x) { print("never"); } + let r = flag && flag; + let s = flag || flag; + return r || s; +} +print(f(1, true)); diff --git a/tests/run_full_parity.sh b/tests/run_full_parity.sh index fc619d42..5380eba4 100755 --- a/tests/run_full_parity.sh +++ b/tests/run_full_parity.sh @@ -104,7 +104,8 @@ echo "" # Categories to skip entirely (known incompatible or special tests) # - compiler/parity/ast_serialize/lsp: special test categories -SKIP_CATEGORIES="compiler parity contracts ast_serialize lsp" +# - lint: compiler-diagnostic fixtures; some trap at runtime by design +SKIP_CATEGORIES="compiler parity contracts ast_serialize lsp lint" # Tests with non-deterministic output (race conditions, timing, etc.) # These test correctness but not output matching diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 3c6c29a9..bfa1a1a2 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -109,8 +109,9 @@ format_time() { echo -e "${BLUE}Running tests...${NC}" echo "" -# Find all test files (excluding compiler, parity, contracts, and formatter directories which have their own test runners) -TEST_FILES=$(find "$TEST_DIR" -name "*.hml" -not -path "*/compiler/*" -not -path "*/parity/*" -not -path "*/contracts/*" -not -path "*/formatter/*" | sort) +# Find all test files (excluding compiler, parity, contracts, formatter, and lint directories which have their own test runners) +# lint/ fixtures are compiler-diagnostic cases; some trap at runtime by design (e.g. modulo-by-zero) +TEST_FILES=$(find "$TEST_DIR" -name "*.hml" -not -path "*/compiler/*" -not -path "*/parity/*" -not -path "*/contracts/*" -not -path "*/formatter/*" -not -path "*/lint/*" | sort) CURRENT_CATEGORY="" for test_file in $TEST_FILES; do From adf3e6d8e38128a3b744c549205dc5d24c2d4633 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 20:46:33 +0000 Subject: [PATCH 3/3] compiler: reject unknown named arguments + lint infinite loops that never fall through MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01WKLiroxGKnQhJBa43UbHJT --- docs/advanced/lint.md | 2 +- include/compiler/lint.h | 3 +- src/backends/compiler/lint.c | 74 +++++++++++++++++-- src/backends/compiler/type_check_expr.c | 17 ++++- .../type_check/run_type_check_tests.sh | 28 +++++++ tests/lint/loop_no_break.expected | 1 + tests/lint/loop_no_break.hml | 7 ++ tests/lint/neg_loop_break.expected | 0 tests/lint/neg_loop_break.hml | 23 ++++++ 9 files changed, 146 insertions(+), 9 deletions(-) create mode 100644 tests/lint/loop_no_break.expected create mode 100644 tests/lint/loop_no_break.hml create mode 100644 tests/lint/neg_loop_break.expected create mode 100644 tests/lint/neg_loop_break.hml diff --git a/docs/advanced/lint.md b/docs/advanced/lint.md index 959f6bde..a411c98d 100644 --- a/docs/advanced/lint.md +++ b/docs/advanced/lint.md @@ -38,7 +38,7 @@ about library code you did not write. | Diagnostic | When | Default | |------------|------|---------| -| **unreachable code** | a statement follows one that always diverges (`return`/`break`/`continue`/`throw`/`panic()`, or an `if`/`else` whose arms all diverge) | on | +| **unreachable code** | a statement follows one that always diverges (`return`/`break`/`continue`/`throw`/`panic()`, an `if`/`else` whose arms all diverge, or an infinite `loop`/`while (true)` with no `break` bound to it) | on | | **dead branch** | an `if`/`while` condition is a constant (`if (false)`, `while (0)`) | on | | **redundant branch** | an `if (true) { … } else { … }` whose `else` can never run | on | | **self-assignment** | `x = x`, `obj.f = obj.f`, or `a[i] = a[i]` — a no-op | on | diff --git a/include/compiler/lint.h b/include/compiler/lint.h index 523ceb94..f97224d2 100644 --- a/include/compiler/lint.h +++ b/include/compiler/lint.h @@ -9,7 +9,8 @@ * ship a binary: * * - unreachable code (statements after return/break/continue/throw/panic, - * or after an if/else where both arms always diverge) + * after an if/else where both arms always diverge, or + * after an infinite loop with no break bound to it) * - dead branches (`if`/`while` conditions that are constant) * - self-assignment (`x = x`, `obj.f = obj.f`, `a[i] = a[i]`) * - self-comparison (`x < x`, `x > x`, `x && x`, `x || x` — always diff --git a/src/backends/compiler/lint.c b/src/backends/compiler/lint.c index 6a4f0927..04607181 100644 --- a/src/backends/compiler/lint.c +++ b/src/backends/compiler/lint.c @@ -129,11 +129,59 @@ static int lint_pure_equal(Expr *a, Expr *b) { } } +/* + * Does `s` contain a break that would exit a loop enclosing it? `depth` is + * the number of loop/switch levels between that enclosing loop and `s` (0 = + * directly inside it): an unlabeled break only escapes when depth is 0, since + * deeper ones bind to the nested loop/switch instead. A labeled break escapes + * when it names `label`, at any depth. Used to decide whether an infinite + * loop can ever fall through to the statement after it. + */ +static int lint_has_escaping_break(Stmt *s, const char *label, int depth) { + if (!s) return 0; + switch (s->type) { + case STMT_BREAK: + if (s->as.break_stmt.label) + return label && strcmp(s->as.break_stmt.label, label) == 0; + return depth == 0; + case STMT_BLOCK: + for (int i = 0; i < s->as.block.count; i++) + if (lint_has_escaping_break(s->as.block.statements[i], label, depth)) + return 1; + return 0; + case STMT_IF: + return lint_has_escaping_break(s->as.if_stmt.then_branch, label, depth) || + lint_has_escaping_break(s->as.if_stmt.else_branch, label, depth); + case STMT_TRY: + return lint_has_escaping_break(s->as.try_stmt.try_block, label, depth) || + lint_has_escaping_break(s->as.try_stmt.catch_block, label, depth) || + lint_has_escaping_break(s->as.try_stmt.finally_block, label, depth); + case STMT_WHILE: + return lint_has_escaping_break(s->as.while_stmt.body, label, depth + 1); + case STMT_LOOP: + return lint_has_escaping_break(s->as.loop_stmt.body, label, depth + 1); + case STMT_FOR: + return lint_has_escaping_break(s->as.for_loop.body, label, depth + 1); + case STMT_FOR_IN: + return lint_has_escaping_break(s->as.for_in.body, label, depth + 1); + case STMT_SWITCH: + /* An unlabeled break inside a switch binds to the switch. */ + for (int i = 0; i < s->as.switch_stmt.num_cases; i++) + if (lint_has_escaping_break(s->as.switch_stmt.case_bodies[i], label, depth + 1)) + return 1; + return 0; + default: + return 0; + } +} + /* * Does control unconditionally leave the program point after this statement * (so anything textually following it in the same block is unreachable)? - * Only cases the compiler is certain about are claimed; loops, switch, and - * try are treated as falling through to avoid false "unreachable" reports. + * Only cases the compiler is certain about are claimed; conditional loops, + * switch, and try are treated as falling through to avoid false reports. + * An infinite loop (`loop`, or `while (true)`) falls through only via a + * break bound to it — with none, everything after it is unreachable. */ static int lint_stmt_diverges(Stmt *s) { if (!s) return 0; @@ -153,6 +201,16 @@ static int lint_stmt_diverges(Stmt *s) { return s->as.if_stmt.else_branch && lint_stmt_diverges(s->as.if_stmt.then_branch) && lint_stmt_diverges(s->as.if_stmt.else_branch); + case STMT_LOOP: + return !lint_has_escaping_break(s->as.loop_stmt.body, + s->as.loop_stmt.label, 0); + case STMT_WHILE: { + int truthy; + if (lint_const_truth(s->as.while_stmt.condition, &truthy) && truthy) + return !lint_has_escaping_break(s->as.while_stmt.body, + s->as.while_stmt.label, 0); + return 0; + } default: return 0; } @@ -653,9 +711,15 @@ static void lint_check_unreachable(LintContext *ctx, Stmt **stmts, int count) { for (int i = 0; i < count; i++) { Stmt *s = stmts[i]; if (diverged_at >= 0 && !lint_is_hoisted_decl(s)) { - lint_warn(ctx, s->line, - "unreachable code: control always leaves via the %s on line %d", - lint_diverge_word(stmts[diverged_at]), stmts[diverged_at]->line); + Stmt *d = stmts[diverged_at]; + if (d->type == STMT_LOOP || d->type == STMT_WHILE) + lint_warn(ctx, s->line, + "unreachable code: the infinite loop on line %d has no break and never falls through", + d->line); + else + lint_warn(ctx, s->line, + "unreachable code: control always leaves via the %s on line %d", + lint_diverge_word(d), d->line); return; } if (diverged_at < 0 && lint_stmt_diverges(s)) diverged_at = i; diff --git a/src/backends/compiler/type_check_expr.c b/src/backends/compiler/type_check_expr.c index c9b54d78..89d9f1a5 100644 --- a/src/backends/compiler/type_check_expr.c +++ b/src/backends/compiler/type_check_expr.c @@ -467,8 +467,21 @@ void type_check_expr(TypeCheckContext *ctx, Expr *expr) { } } - // If parameter not found, skip type checking (runtime will catch it) - if (param_idx < 0) continue; + // A named argument that matches no parameter is a + // guaranteed runtime throw in the interpreter + // ("Unknown parameter name"), while the compiled + // binary silently binds it positionally — reject it + // here so a typo like `f(nam: ...)` can't compile + // into divergent behavior. Only when the signature + // carries parameter names; otherwise stay silent. + if (param_idx < 0) { + if (sig->param_names) { + type_error(ctx, expr->line, + "unknown named argument '%s' to '%s': no parameter with that name", + arg_name, name); + } + continue; + } } if (!sig->param_types[param_idx]) continue; diff --git a/tests/compiler/type_check/run_type_check_tests.sh b/tests/compiler/type_check/run_type_check_tests.sh index 1e361608..7737f0b3 100755 --- a/tests/compiler/type_check/run_type_check_tests.sh +++ b/tests/compiler/type_check/run_type_check_tests.sh @@ -313,6 +313,34 @@ else ((FAILED++)) fi +# Test 21: Unknown named argument is rejected +echo "Test 21: Unknown named argument" +cat > /tmp/test_unknown_named.hml << 'EOF' +fn process(name: string, count: i32): string { return name; } +let x = process(nam: "Alice", count: 5); +EOF +if $HEMLOCKC --check /tmp/test_unknown_named.hml 2>&1 | grep -q "unknown named argument 'nam'"; then + echo " PASSED: Caught unknown named argument" + ((PASSED++)) +else + echo " FAILED: Did not catch unknown named argument" + ((FAILED++)) +fi + +# Test 22: Valid named arguments still accepted +echo "Test 22: Valid named arguments pass" +cat > /tmp/test_valid_named.hml << 'EOF' +fn process(name: string, count: i32): string { return name; } +let x = process(name: "Alice", count: 5); +EOF +if $HEMLOCKC --check /tmp/test_valid_named.hml 2>&1 | grep -q "unknown named argument"; then + echo " FAILED: Valid named argument incorrectly flagged" + ((FAILED++)) +else + echo " PASSED: Valid named arguments accepted" + ((PASSED++)) +fi + echo "" echo "=== Type Check Test Results ===" echo "Passed: $PASSED" diff --git a/tests/lint/loop_no_break.expected b/tests/lint/loop_no_break.expected new file mode 100644 index 00000000..500f462f --- /dev/null +++ b/tests/lint/loop_no_break.expected @@ -0,0 +1 @@ +loop_no_break.hml:5: warning: unreachable code: the infinite loop on line 2 has no break and never falls through diff --git a/tests/lint/loop_no_break.hml b/tests/lint/loop_no_break.hml new file mode 100644 index 00000000..efd9f178 --- /dev/null +++ b/tests/lint/loop_no_break.hml @@ -0,0 +1,7 @@ +fn spin(): i32 { + loop { + print("tick"); + } + return 1; +} +print(spin()); diff --git a/tests/lint/neg_loop_break.expected b/tests/lint/neg_loop_break.expected new file mode 100644 index 00000000..e69de29b diff --git a/tests/lint/neg_loop_break.hml b/tests/lint/neg_loop_break.hml new file mode 100644 index 00000000..72513819 --- /dev/null +++ b/tests/lint/neg_loop_break.hml @@ -0,0 +1,23 @@ +fn counted(): i32 { + let i = 0; + loop { + i = i + 1; + if (i >= 3) { break; } + } + return i; +} +fn labeled(): i32 { + outer: loop { + loop { break outer; } + } + return 2; +} +fn conditional(): i32 { + while (true) { + if (1 > 2) { break; } + return 3; + } + return 4; +} +print(counted()); +print(labeled());