From 5e18df9bc12ca119cec27e721ca0321302bbbbc1 Mon Sep 17 00:00:00 2001 From: hl-valdemar Date: Wed, 8 Jul 2026 18:45:55 +0200 Subject: [PATCH] warning diagnostics for unused locals --- .DS_Store | Bin 0 -> 6148 bytes README.md | 7 +- compiler/checker/checker.odin | 243 +++++++++++++----- compiler/source/source.odin | 53 +++- compiler_tests.odin | 158 +++++++++++- .../packages/qualified_shadow/app/main.bro | 2 +- examples/programs/conditional_unwrap/main.bro | 2 +- examples/programs/unused_locals/main.bro | 12 + std/mem/heap/heap.bro | 9 - std/mem/mem.bro | 4 +- 10 files changed, 378 insertions(+), 112 deletions(-) create mode 100644 .DS_Store create mode 100644 examples/programs/unused_locals/main.bro delete mode 100644 std/mem/heap/heap.bro diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..17cb7cf5dd8e27bf6a310611f3202a8f3e587b9c GIT binary patch literal 6148 zcmeH~J8r{33`B>q3j=9Xy4=PG$PE^TeF9$~g@H7I4cJNQJ$i zuh5DBZ2x(F0VV(&x+``bW@gM+xZ?xouhZ@Fe!aZ0;#J@&VrHyNnC;iLL0pQvfVyTmjO&;ssLc!1UOG})p;=82 zR;?Ceh}WZ?+UmMqI#RP8R>OzYoz15hnq@nzF`-!xQ4j$Um=RcIKKc27q(7SfXDv!a zKm`670b3tVhdp1a&emVA=k?dB`g+i*aXG_}p8zI)6mRKa+;6_1_R^8c3Qa!(fk8n8 H{*=Hsj(ic6 literal 0 HcmV?d00001 diff --git a/README.md b/README.md index 067453d..c86f193 100644 --- a/README.md +++ b/README.md @@ -143,8 +143,9 @@ package loader -> per-file lexer/parser/AST -> checker/HIR -> lower/IR -> opt -> ``` Source diagnostics do not block executable generation. When recovery is -possible, invalid code lowers to runtime diagnostic traps and the compiler -returns status `1`. Infrastructure or backend failures return status `2`. +possible, errors lower to runtime diagnostic traps; warnings do not trap. Any +source diagnostic makes the compiler return status `1`. Infrastructure or +backend failures return status `2`. Top-level function bodies are semantically checked lazily when a concrete specialization is demanded. @@ -193,5 +194,5 @@ feature ledger, and [TODO.md](TODO.md) for the implementation roadmap. Compiler exit statuses: - `0`: executable produced without source diagnostics -- `1`: executable produced with source diagnostics and embedded traps +- `1`: executable produced with source diagnostics; errors may embed traps, warnings do not - `2`: executable could not be produced diff --git a/compiler/checker/checker.odin b/compiler/checker/checker.odin index fb40632..7cdf006 100644 --- a/compiler/checker/checker.odin +++ b/compiler/checker/checker.odin @@ -80,6 +80,9 @@ Build_Ctx :: struct { local_types: []types.Type, locals: ^[dynamic]Build_Local, hir_locals: ^[dynamic]hir.Local, + local_spans: ^[dynamic]source.Span, + local_used: ^[dynamic]bool, + local_warnable: ^[dynamic]bool, global_reads: ^[dynamic]hir.Global_Id, calls: ^[dynamic]hir.Function_Id, problematic: ^bool, @@ -170,6 +173,94 @@ symbol_text :: proc(checker: ^Checker, id: symbol.Id) -> string { return symbol.resolve(checker.symbols, id) } +append_tracked_local :: proc( + locals: ^[dynamic]hir.Local, + spans: ^[dynamic]source.Span, + used: ^[dynamic]bool, + warnable: ^[dynamic]bool, + local: hir.Local, + span: source.Span, +) -> hir.Local_Id { + id := hir.local_id(len(locals^)) + append(locals, local) + append(spans, span) + append(used, false) + append(warnable, true) + return id +} + +append_build_local :: proc( + ctx: ^Build_Ctx, + name: symbol.Id, + type: types.Type, + mutable: bool, + span: source.Span, +) -> hir.Local_Id { + id := append_tracked_local( + ctx.hir_locals, + ctx.local_spans, + ctx.local_used, + ctx.local_warnable, + hir.Local{name=name, type=type, mutable=mutable}, + span, + ) + append(ctx.locals, Build_Local{name=name, type=type, mutable=mutable, id=id}) + return id +} + +ignore_tracked_locals :: proc(ctx: ^Build_Ctx, start: int) { + for i := start; i < len(ctx.local_warnable^); i += 1 { + ctx.local_warnable^[i] = false + } +} + +mark_local_used :: proc(checker: ^Checker, id: hir.Local_Id) { + ctx := checker.current_build_ctx + if ctx == nil || id == hir.INVALID_LOCAL { + return + } + index := int(id) + if index >= 0 && index < len(ctx.local_used^) { + ctx.local_used^[index] = true + } +} + +build_local_expr :: proc(checker: ^Checker, local: Build_Local, span: source.Span) -> hir.Expr_Id { + mark_local_used(checker, local.id) + return add_hir_expr(checker, hir.Expr{ + kind=.Local, span=span, type=local.type, target=hir.local_ref(local.id), + left=hir.INVALID_EXPR, right=hir.INVALID_EXPR, diagnostic=source.INVALID_DIAGNOSTIC, + }) +} + +record_unused_locals :: proc( + checker: ^Checker, + locals: []hir.Local, + spans: []source.Span, + used: []bool, + warnable: []bool, +) { + for local, index in locals { + if local.name == checker.sink_symbol || + !symbol.is_valid(local.name) || + index >= len(warnable) || + !warnable[index] || + index >= len(used) || + used[index] { + continue + } + span := source.Span{} + if index < len(spans) { + span = spans[index] + } + if local.parameter { + source.addf_warning(checker.diagnostics, span, "unused parameter '%s'", symbol_text(checker, local.name)) + } else { + source.addf_warning(checker.diagnostics, span, "unused local '%s'", symbol_text(checker, local.name)) + } + } +} + // type_label renders a type for a diagnostic, resolving a named type (enum/union/struct/ // distinct) to its declared source name; primitives and unnamed types fall back to // `types.name` (which prints `` for anonymous nodes). @@ -3698,10 +3789,7 @@ build_qualified_value_field :: proc( return hir.INVALID_EXPR, false, false } if local, ok := find_build_local(locals, expr.qualifier); ok { - base := add_hir_expr(checker, hir.Expr{ - kind=.Local, span=expr.span, type=local.type, target=hir.local_ref(local.id), - left=hir.INVALID_EXPR, right=hir.INVALID_EXPR, diagnostic=source.INVALID_DIAGNOSTIC, - }) + base := build_local_expr(checker, local, expr.span) value, ok := build_field_from_value(checker, expr, base, local.type) return value, true, ok } @@ -4285,9 +4373,7 @@ build_compound_expr :: proc( capture_start := len(ctx.locals^) error_type := types.fallible_error(channel_type, store) if symbol.is_valid(expr.name) && expr.name != checker.sink_symbol { - capture = hir.local_id(len(ctx.hir_locals^)) - append(ctx.hir_locals, hir.Local{name=expr.name, type=error_type, mutable=false}) - append(ctx.locals, Build_Local{name=expr.name, type=error_type, mutable=false, id=capture}) + capture = append_build_local(ctx, expr.name, error_type, false, expr.span) } handler: [dynamic]hir.Stmt_Id handler.allocator = checker.allocator @@ -4652,16 +4738,10 @@ build_expr :: proc( last = hir.INVALID_EXPR if !symbol.is_valid(expr.qualifier) { if local, ok := find_build_local(locals, expr.name); ok { - last = add_hir_expr(checker, hir.Expr{ - kind=.Local, span=expr.span, type=local.type, target=hir.local_ref(local.id), - left = hir.INVALID_EXPR, right = hir.INVALID_EXPR, diagnostic = source.INVALID_DIAGNOSTIC, - }) + last = build_local_expr(checker, local, expr.span) } } else if local, ok := find_build_local(locals, expr.qualifier); ok { - base := add_hir_expr(checker, hir.Expr{ - kind=.Local, span=expr.span, type=local.type, target=hir.local_ref(local.id), - left=hir.INVALID_EXPR, right=hir.INVALID_EXPR, diagnostic=source.INVALID_DIAGNOSTIC, - }) + base := build_local_expr(checker, local, expr.span) base_type := local.type item, has_item := types.container(base_type, &checker.module.types) field_name := symbol_text(checker, expr.name) @@ -4851,10 +4931,7 @@ build_expr :: proc( if !symbol.is_valid(expr.qualifier) { if local, ok := find_build_local(locals, expr.name); ok { if _, _, _, callable := types.function_pointer(local.type, &checker.module.types); callable { - callee = add_hir_expr(checker, hir.Expr{ - kind=.Local, span=expr.span, type=local.type, target=hir.local_ref(local.id), - left=hir.INVALID_EXPR, right=hir.INVALID_EXPR, diagnostic=source.INVALID_DIAGNOSTIC, - }) + callee = build_local_expr(checker, local, expr.span) } else { non_callable = true } @@ -5409,13 +5486,7 @@ build_block :: proc( ctx.problematic^ = true continue } - local_id := hir.local_id(len(ctx.hir_locals^)) - append(ctx.hir_locals, hir.Local{ - name = statement.name, type = value_type, mutable = !statement.immutable, - }) - append(ctx.locals, Build_Local{ - name = statement.name, type = value_type, mutable = !statement.immutable, id = local_id, - }) + local_id := append_build_local(ctx, statement.name, value_type, !statement.immutable, statement.span) append(&body, hir.stmt_id(len(checker.module.statements))) append(&checker.module.statements, hir.Stmt{ kind = .Declaration, span = statement.span, local = local_id, expr = value, @@ -5536,13 +5607,7 @@ build_block :: proc( ctx.problematic^ = true continue } - local_id := hir.local_id(len(ctx.hir_locals^)) - append(ctx.hir_locals, hir.Local{ - name = statement.name, type = value_type, mutable = !statement.immutable, - }) - append(ctx.locals, Build_Local{ - name = statement.name, type = value_type, mutable = !statement.immutable, id = local_id, - }) + local_id := append_build_local(ctx, statement.name, value_type, !statement.immutable, statement.span) append(&body, hir.stmt_id(len(checker.module.statements))) append(&checker.module.statements, hir.Stmt{ kind = .Declaration, span = statement.span, local = local_id, expr = value, @@ -5861,8 +5926,14 @@ build_block :: proc( // runs defers. if len(ctx.defers^) > 0 { if checker.module.exprs[value].kind != .Invalid { - tmp := hir.local_id(len(ctx.hir_locals^)) - append(ctx.hir_locals, hir.Local{name = checker.sink_symbol, type = ctx.result, mutable = false}) + tmp := append_tracked_local( + ctx.hir_locals, + ctx.local_spans, + ctx.local_used, + ctx.local_warnable, + hir.Local{name = checker.sink_symbol, type = ctx.result, mutable = false}, + source.Span{}, + ) append(&body, hir.stmt_id(len(checker.module.statements))) append(&checker.module.statements, hir.Stmt{ kind = .Declaration, span = statement.span, local = tmp, expr = value, @@ -5965,9 +6036,7 @@ build_block :: proc( diagnostic = id valid_unwrap = false } - local = hir.local_id(len(ctx.hir_locals^)) - append(ctx.hir_locals, hir.Local{name=capture, type=child, mutable=false}) - append(ctx.locals, Build_Local{name=capture, type=child, mutable=false, id=local}) + local = append_build_local(ctx, capture, child, false, statement.span) } if index < len(values) { append(&unwraps, hir.Conditional_Unwrap{expr=values[index], local=local}) @@ -6169,9 +6238,7 @@ build_block :: proc( valid_loop = false } } - item_local := hir.local_id(len(ctx.hir_locals^)) - append(ctx.hir_locals, hir.Local{name=statement.name, type=capture_type, mutable=false}) - append(ctx.locals, Build_Local{name=statement.name, type=capture_type, mutable=false, id=item_local}) + item_local := append_build_local(ctx, statement.name, capture_type, false, statement.span) index_local := hir.INVALID_LOCAL if symbol.is_valid(statement.index_name) { if statement.index_name == statement.name { @@ -6184,9 +6251,7 @@ build_block :: proc( diagnostic = id valid_loop = false } else { - index_local = hir.local_id(len(ctx.hir_locals^)) - append(ctx.hir_locals, hir.Local{name=statement.index_name, type=types.USIZE, mutable=false}) - append(ctx.locals, Build_Local{name=statement.index_name, type=types.USIZE, mutable=false, id=index_local}) + index_local = append_build_local(ctx, statement.index_name, types.USIZE, false, statement.span) } } if id := add_label_shadow_diagnostic(ctx, statement.span, statement.label); @@ -6512,8 +6577,14 @@ build_value_block :: proc( // rule as `return`. if len(ctx.defers^) > defer_start { if checker.module.exprs[value].kind != .Invalid { - tmp := hir.local_id(len(ctx.hir_locals^)) - append(ctx.hir_locals, hir.Local{name = checker.sink_symbol, type = value_type, mutable = false}) + tmp := append_tracked_local( + ctx.hir_locals, + ctx.local_spans, + ctx.local_used, + ctx.local_warnable, + hir.Local{name = checker.sink_symbol, type = value_type, mutable = false}, + source.Span{}, + ) append(body, hir.stmt_id(len(checker.module.statements))) append(&checker.module.statements, hir.Stmt{ kind = .Declaration, span = yield_stmt.span, local = tmp, expr = value, @@ -6568,9 +6639,14 @@ build_value_source :: proc( // new_value_slot allocates a fresh, un-nameable mutable local to hold a value-if/loop // result. Branches/iterations assign it; the construct's value is a read of it. new_value_slot :: proc(ctx: ^Build_Ctx, slot_type: types.Type) -> hir.Local_Id { - slot := hir.local_id(len(ctx.hir_locals^)) - append(ctx.hir_locals, hir.Local{name = ctx.checker.sink_symbol, type = slot_type, mutable = true}) - return slot + return append_tracked_local( + ctx.hir_locals, + ctx.local_spans, + ctx.local_used, + ctx.local_warnable, + hir.Local{name = ctx.checker.sink_symbol, type = slot_type, mutable = true}, + source.Span{}, + ) } // slot_read builds a `.Local` read of a result slot. @@ -6730,9 +6806,7 @@ emit_value_if :: proc( ) != source.INVALID_DIAGNOSTIC { ok = false } - local = hir.local_id(len(ctx.hir_locals^)) - append(ctx.hir_locals, hir.Local{name=capture, type=child, mutable=false}) - append(ctx.locals, Build_Local{name=capture, type=child, mutable=false, id=local}) + local = append_build_local(ctx, capture, child, false, if_stmt.span) } if index < len(values) { append(&unwrap_list, hir.Conditional_Unwrap{expr=values[index], local=local}) @@ -6964,8 +7038,14 @@ emit_match :: proc( target = hir.INVALID_REF, right = hir.INVALID_EXPR, diagnostic = source.INVALID_DIAGNOSTIC, }) } - subj_local := hir.local_id(len(ctx.hir_locals^)) - append(ctx.hir_locals, hir.Local{name = checker.sink_symbol, type = spill_type, mutable = false}) + subj_local := append_tracked_local( + ctx.hir_locals, + ctx.local_spans, + ctx.local_used, + ctx.local_warnable, + hir.Local{name = checker.sink_symbol, type = spill_type, mutable = false}, + source.Span{}, + ) append(out, hir.stmt_id(len(checker.module.statements))) append(&checker.module.statements, hir.Stmt{ kind = .Declaration, span = span, local = subj_local, expr = spill_value, @@ -6984,8 +7064,14 @@ emit_match :: proc( left = match_subject_location(checker, subj_local, subj_is_pointer, subject_type, ptr_type, span), target = hir.INVALID_REF, right = hir.INVALID_EXPR, diagnostic = source.INVALID_DIAGNOSTIC, }) - tag_local := hir.local_id(len(ctx.hir_locals^)) - append(ctx.hir_locals, hir.Local{name = checker.sink_symbol, type = tag_enum, mutable = false}) + tag_local := append_tracked_local( + ctx.hir_locals, + ctx.local_spans, + ctx.local_used, + ctx.local_warnable, + hir.Local{name = checker.sink_symbol, type = tag_enum, mutable = false}, + source.Span{}, + ) append(out, hir.stmt_id(len(checker.module.statements))) append(&checker.module.statements, hir.Stmt{ kind = .Declaration, span = span, local = tag_local, expr = tag_read, @@ -7295,9 +7381,7 @@ build_match_arm_body :: proc( target = hir.INVALID_REF, right = hir.INVALID_EXPR, diagnostic = source.INVALID_DIAGNOSTIC, }) } - cap_local := hir.local_id(len(ctx.hir_locals^)) - append(ctx.hir_locals, hir.Local{name = capture, type = cap_type, mutable = false}) - append(ctx.locals, Build_Local{name = capture, type = cap_type, mutable = false, id = cap_local}) + cap_local := append_build_local(ctx, capture, cap_type, false, span) append(&result, hir.stmt_id(len(checker.module.statements))) append(&checker.module.statements, hir.Stmt{ kind = .Declaration, span = span, local = cap_local, expr = cap_value, @@ -7473,6 +7557,7 @@ value_loop_element_type :: proc(ctx: ^Build_Ctx, loop_stmt: ast.Stmt) -> types.T return types.INVALID } capture_start := len(ctx.locals^) + local_start := len(ctx.hir_locals^) if loop_stmt.kind == .For { // Mirror the `.For` arm's capture-type computation just enough to type the probe. iterable := build_expr(checker, loop_stmt.expr, ctx.locals^[:], ctx.global_reads, ctx.calls, types.INVALID, ctx.pkg, ctx.file) @@ -7487,19 +7572,16 @@ value_loop_element_type :: proc(ctx: ^Build_Ctx, loop_stmt: ast.Stmt) -> types.T } } if symbol.is_valid(loop_stmt.name) { - id := hir.local_id(len(ctx.hir_locals^)) - append(ctx.hir_locals, hir.Local{name=loop_stmt.name, type=capture_type, mutable=false}) - append(ctx.locals, Build_Local{name=loop_stmt.name, type=capture_type, mutable=false, id=id}) + append_build_local(ctx, loop_stmt.name, capture_type, false, loop_stmt.span) } if symbol.is_valid(loop_stmt.index_name) { - id := hir.local_id(len(ctx.hir_locals^)) - append(ctx.hir_locals, hir.Local{name=loop_stmt.index_name, type=types.USIZE, mutable=false}) - append(ctx.locals, Build_Local{name=loop_stmt.index_name, type=types.USIZE, mutable=false, id=id}) + append_build_local(ctx, loop_stmt.index_name, types.USIZE, false, loop_stmt.span) } } probe := build_expr(checker, yield_expr, ctx.locals^[:], ctx.global_reads, ctx.calls, types.INVALID, ctx.pkg, ctx.file) result := checker.module.exprs[probe].type if checker.module.exprs[probe].kind != .Invalid else types.INVALID resize(ctx.locals, capture_start) + ignore_tracked_locals(ctx, local_start) return result } @@ -7545,6 +7627,7 @@ block_element_type :: proc(ctx: ^Build_Ctx, block_stmts: []ast.Stmt_Id) -> types } } scope_start := len(ctx.locals^) + local_start := len(ctx.hir_locals^) defer_start := len(ctx.defers^) lead := build_block(ctx, block_stmts[:lead_end], close = false) delete(lead, checker.allocator) @@ -7556,6 +7639,7 @@ block_element_type :: proc(ctx: ^Build_Ctx, block_stmts: []ast.Stmt_Id) -> types } resize(ctx.defers, defer_start) resize(ctx.locals, scope_start) + ignore_tracked_locals(ctx, local_start) return result } @@ -7848,6 +7932,12 @@ build_function :: proc(checker: ^Checker, id: Spec_Id) { locals.allocator = checker.allocator hir_locals: [dynamic]hir.Local hir_locals.allocator = checker.allocator + local_spans: [dynamic]source.Span + local_spans.allocator = checker.allocator + local_used: [dynamic]bool + local_used.allocator = checker.allocator + local_warnable: [dynamic]bool + local_warnable.allocator = checker.allocator params: [dynamic]hir.Local_Id params.allocator = checker.allocator body: [dynamic]hir.Stmt_Id @@ -7869,12 +7959,18 @@ build_function :: proc(checker: ^Checker, id: Spec_Id) { if param.comptime_value { continue } - local_id := hir.local_id(len(hir_locals)) param_type := types.INVALID if runtime_index < len(spec.args) { param_type = spec.args[runtime_index] } - append(&hir_locals, hir.Local{name = param.name, type = param_type, parameter = true}) + local_id := append_tracked_local( + &hir_locals, + &local_spans, + &local_used, + &local_warnable, + hir.Local{name = param.name, type = param_type, parameter = true}, + param.span, + ) append(&locals, Build_Local{name = param.name, type = param_type, id = local_id}) append(¶ms, local_id) runtime_index += 1 @@ -7905,6 +8001,9 @@ build_function :: proc(checker: ^Checker, id: Spec_Id) { }, ) delete(locals) + delete(local_spans) + delete(local_used) + delete(local_warnable) return } @@ -7939,6 +8038,9 @@ build_function :: proc(checker: ^Checker, id: Spec_Id) { local_types = local_types, locals = &locals, hir_locals = &hir_locals, + local_spans = &local_spans, + local_used = &local_used, + local_warnable = &local_warnable, global_reads = &global_reads, calls = &calls, problematic = &problematic, @@ -7976,6 +8078,8 @@ build_function :: proc(checker: ^Checker, id: Spec_Id) { problematic = true } + record_unused_locals(checker, hir_locals[:], local_spans[:], local_used[:], local_warnable[:]) + assert(spec.hir_id == hir.function_id(len(checker.module.functions))) append( &checker.module.functions, @@ -8006,6 +8110,9 @@ build_function :: proc(checker: ^Checker, id: Spec_Id) { delete(loop_is_loop) delete(yield_targets) delete(locals) + delete(local_spans) + delete(local_used) + delete(local_warnable) } expr_problematic :: proc(checker: ^Checker, expr_id: hir.Expr_Id) -> bool { @@ -8576,7 +8683,7 @@ check :: proc( propagate_problems(&checker) for import_item in ast_module.imports { if import_item.valid && !import_item.used { - source.addf(diagnostics, import_item.span, "unused import '%s'", symbol_text(&checker, import_item.alias)) + source.addf_warning(diagnostics, import_item.span, "unused import '%s'", symbol_text(&checker, import_item.alias)) } } return checker.module diff --git a/compiler/source/source.odin b/compiler/source/source.odin index 25f7ab0..6d82146 100644 --- a/compiler/source/source.odin +++ b/compiler/source/source.odin @@ -53,8 +53,14 @@ Store :: struct { } Diagnostic :: struct { - span: Span, - message: string, + span: Span, + message: string, + severity: Severity, +} + +Severity :: enum u8 { + Error, + Warning, } Diagnostics :: struct { @@ -66,8 +72,9 @@ Diagnostics :: struct { } Diagnostic_Key :: struct { - span: Span, - message: string, + span: Span, + message: string, + severity: Severity, } init_store :: proc(allocator := context.allocator) -> Store { @@ -144,31 +151,47 @@ destroy_diagnostics :: proc(diagnostics: ^Diagnostics) { delete(diagnostics.items) } -add :: proc(diagnostics: ^Diagnostics, span: Span, message: string) -> Diagnostic_Id { - key := Diagnostic_Key{span=span, message=message} +add_with_severity :: proc(diagnostics: ^Diagnostics, span: Span, message: string, severity: Severity) -> Diagnostic_Id { + key := Diagnostic_Key{span=span, message=message, severity=severity} if id, ok := diagnostics.index[key]; ok { return id } id := diagnostic_id(len(diagnostics.items)) cloned := fmt.aprintf("%s", message, allocator=diagnostics.allocator) - append(&diagnostics.items, Diagnostic{span=span, message=cloned}) - diagnostics.index[Diagnostic_Key{span=span, message=cloned}] = id + append(&diagnostics.items, Diagnostic{span=span, message=cloned, severity=severity}) + diagnostics.index[Diagnostic_Key{span=span, message=cloned, severity=severity}] = id return id } -addf :: proc(diagnostics: ^Diagnostics, span: Span, format: string, args: ..any) -> Diagnostic_Id { +add :: proc(diagnostics: ^Diagnostics, span: Span, message: string) -> Diagnostic_Id { + return add_with_severity(diagnostics, span, message, .Error) +} + +add_warning :: proc(diagnostics: ^Diagnostics, span: Span, message: string) -> Diagnostic_Id { + return add_with_severity(diagnostics, span, message, .Warning) +} + +addf_with_severity :: proc(diagnostics: ^Diagnostics, span: Span, severity: Severity, format: string, args: ..any) -> Diagnostic_Id { message := fmt.aprintf(format, ..args, allocator=diagnostics.allocator) - key := Diagnostic_Key{span=span, message=message} + key := Diagnostic_Key{span=span, message=message, severity=severity} if id, ok := diagnostics.index[key]; ok { delete(message, diagnostics.allocator) return id } id := diagnostic_id(len(diagnostics.items)) - append(&diagnostics.items, Diagnostic{span=span, message=message}) - diagnostics.index[Diagnostic_Key{span=span, message=message}] = id + append(&diagnostics.items, Diagnostic{span=span, message=message, severity=severity}) + diagnostics.index[Diagnostic_Key{span=span, message=message, severity=severity}] = id return id } +addf :: proc(diagnostics: ^Diagnostics, span: Span, format: string, args: ..any) -> Diagnostic_Id { + return addf_with_severity(diagnostics, span, .Error, format, ..args) +} + +addf_warning :: proc(diagnostics: ^Diagnostics, span: Span, format: string, args: ..any) -> Diagnostic_Id { + return addf_with_severity(diagnostics, span, .Warning, format, ..args) +} + line_and_column :: proc(source_file: ^Source, offset: Offset) -> (line, column: int) { if len(source_file.line_starts) > 0 { limit := min(int(offset), len(source_file.text)) @@ -222,15 +245,17 @@ format :: proc(diagnostics: ^Diagnostics, id: Diagnostic_Id, allocator := contex } diagnostic := diagnostics.items[index] source_file := source_for_span(diagnostics, diagnostic.span) + severity := "warning" if diagnostic.severity == .Warning else "error" if source_file == nil { - return fmt.aprintf(": error: %s", diagnostic.message, allocator=allocator) + return fmt.aprintf(": %s: %s", severity, diagnostic.message, allocator=allocator) } line, column := line_and_column(source_file, diagnostic.span.start) return fmt.aprintf( - "%s:%d:%d: error: %s", + "%s:%d:%d: %s: %s", source_file.path, line, column, + severity, diagnostic.message, allocator=allocator, ) diff --git a/compiler_tests.odin b/compiler_tests.odin index 3bd4701..aba0288 100644 --- a/compiler_tests.odin +++ b/compiler_tests.odin @@ -1102,12 +1102,12 @@ main func() void { @(test) string_literals_preserve_static_length_and_sentinel_through_pointer_views :: proc(t: ^testing.T) { - text := `take_sentinel_pointer func(value [*;0]u8) void {} -take_mut_sentinel_pointer func(value [*;0]mut u8) void {} -take_pointer func(value *u8) void {} -take_sentinel_slice func(value [;0]u8) void {} -take_mut_sentinel_slice func(value [;0]mut u8) void {} -take_slice func(value []u8) void {} + text := `take_sentinel_pointer func(_ [*;0]u8) void {} +take_mut_sentinel_pointer func(_ [*;0]mut u8) void {} +take_pointer func(_ *u8) void {} +take_sentinel_slice func(_ [;0]u8) void {} +take_mut_sentinel_slice func(_ [;0]mut u8) void {} +take_slice func(_ []u8) void {} take_c_string c_func(value *c_char) c_int take_c_sentinel c_func(value [*;0]c_char) c_int main func() void { @@ -1574,8 +1574,8 @@ main func() void { @(test) opaque_anyopaque_and_ptr_cast_compile_and_lower :: proc(t: ^testing.T) { text := `Handle :: opaque -take func(value ?*mut anyopaque) void {} -use_handle func(handle ?@mut Handle) void {} +take func(_ ?*mut anyopaque) void {} +use_handle func(_ ?@mut Handle) void {} main func() void { values [2]mut u8 = [1, 2] raw ?*mut anyopaque = (&values).ptr @@ -1833,6 +1833,7 @@ take_int func(value int) int { } main func() void { local i16 :: 1 + 2 + _ = local _ = 100 + (20 + 8) _ = return_i16() _ = take_i16(1 + 2) @@ -1985,6 +1986,69 @@ main func() void { testing.expect_value(t, hir_module.statements[main.body[2]].kind, hir.Stmt_Kind.Trap) } +@(test) +unused_locals_and_params_warn_without_traps :: proc(t: ^testing.T) { + text := `warn_only func(value i32, unused i32) i32 { + local i32 = 1 + write_only i32 = 2 + write_only = 3 + consumed i32 = value + _ = consumed + return value +} +main func() void { + _ = warn_only(1, 2) +} +` + source_file := source.Source{path="test.bro", text=text} + diagnostics := source.init_diagnostics(&source_file) + defer source.destroy_diagnostics(&diagnostics) + symbols := symbol.init_table() + defer symbol.destroy_table(&symbols) + stream := lexer.lex(&source_file, &diagnostics, &symbols) + defer delete(stream.items) + ast_module := parser.parse(&stream, &source_file, &diagnostics) + defer ast.destroy_module(&ast_module) + hir_module := checker.check(&ast_module, &diagnostics, &symbols) + defer hir.destroy_module(&hir_module) + + found_unused_param := false + found_unused_local := false + found_write_only := false + warning_count := 0 + error_count := 0 + for diagnostic in diagnostics.items { + if diagnostic.severity == source.Severity.Warning { + warning_count += 1 + } else { + error_count += 1 + } + found_unused_param = found_unused_param || strings.contains(diagnostic.message, "unused parameter 'unused'") + found_unused_local = found_unused_local || strings.contains(diagnostic.message, "unused local 'local'") + found_write_only = found_write_only || strings.contains(diagnostic.message, "unused local 'write_only'") + testing.expect(t, !strings.contains(diagnostic.message, "unused local 'consumed'")) + testing.expect(t, !strings.contains(diagnostic.message, "unused parameter 'value'")) + } + testing.expect_value(t, warning_count, 3) + testing.expect_value(t, error_count, 0) + testing.expect(t, found_unused_param) + testing.expect(t, found_unused_local) + testing.expect(t, found_write_only) + + found_function := false + for function in hir_module.functions { + if symbol.resolve(&symbols, function.name) != "warn_only" { + continue + } + found_function = true + testing.expect(t, !function.problematic) + for stmt_id in function.body { + testing.expect(t, hir_module.statements[stmt_id].kind != hir.Stmt_Kind.Trap) + } + } + testing.expect(t, found_function) +} + @(test) recursive_specialization_reaches_a_fixed_point :: proc(t: ^testing.T) { text := `a func(value int) i32 { @@ -2309,7 +2373,7 @@ zero func($T type) T { value T = undefined return value } -buffer func($T type, $N usize, value T) [N]T { +buffer func($T type, $N usize, _ T) [N]T { data [N]T = undefined return data } @@ -2656,6 +2720,16 @@ valid_program_compiles_and_runs :: proc(t: ^testing.T) { testing.expect_value(t, state.exit_code, 0) } +@(test) +unused_local_warnings_return_status_one_but_do_not_trap :: proc(t: ^testing.T) { + output := "/tmp/brolang-test-unused-locals" + defer _ = os.remove(output) + status := compiler_core.compile_package("examples/programs/unused_locals", output) + testing.expect_value(t, status, 1) + state := run_executable(output) + testing.expect_value(t, state.exit_code, 7) +} + @(test) build_command_is_recognized :: proc(t: ^testing.T) { testing.expect(t, is_build_command("build")) @@ -5721,6 +5795,30 @@ source_store_owns_buffers_indexes_lines_and_deduplicates_diagnostics :: proc(t: testing.expect(t, strings.contains(formatted, "owned.bro:2:1:")) } +@(test) +diagnostic_warnings_format_and_dedupe_by_severity :: proc(t: ^testing.T) { + source_file := source.Source{path="test.bro", text="one\n"} + diagnostics := source.init_diagnostics(&source_file) + defer source.destroy_diagnostics(&diagnostics) + span := source.Span{start=0, end=3} + + warning := source.add_warning(&diagnostics, span, "same") + warning_again := source.addf_warning(&diagnostics, span, "%s", "same") + err := source.add(&diagnostics, span, "same") + formatted_warning := source.format(&diagnostics, warning) + defer delete(formatted_warning) + formatted_error := source.format(&diagnostics, err) + defer delete(formatted_error) + + testing.expect_value(t, warning, warning_again) + testing.expect(t, warning != err) + testing.expect_value(t, len(diagnostics.items), 2) + testing.expect_value(t, diagnostics.items[warning].severity, source.Severity.Warning) + testing.expect_value(t, diagnostics.items[err].severity, source.Severity.Error) + testing.expect(t, strings.contains(formatted_warning, "test.bro:1:1: warning: same")) + testing.expect(t, strings.contains(formatted_error, "test.bro:1:1: error: same")) +} + @(test) maximum_signed_i64_literal_parses_exactly :: proc(t: ^testing.T) { source_file := source.Source{path="test.bro", text="value :: 9223372036854775807\nmain func() void {}\n"} @@ -6325,6 +6423,33 @@ unused_import_is_diagnosed_but_remains_executable :: proc(t: ^testing.T) { testing.expect_value(t, state.exit_code, 0) } +@(test) +unused_import_is_a_warning :: proc(t: ^testing.T) { + sources := source.init_store() + defer source.destroy_store(&sources) + diagnostics := source.init_store_diagnostics(&sources) + defer source.destroy_diagnostics(&diagnostics) + symbols := symbol.init_table() + defer symbol.destroy_table(&symbols) + module, loaded := loader.load("examples/packages/unused/app", &sources, &diagnostics, &symbols) + defer ast.destroy_module(&module) + hir_module := checker.check(&module, &diagnostics, &symbols) + defer hir.destroy_module(&hir_module) + + testing.expect(t, loaded) + found := false + for diagnostic, index in diagnostics.items { + if strings.contains(diagnostic.message, "unused import 'math'") { + found = true + testing.expect_value(t, diagnostic.severity, source.Severity.Warning) + formatted := source.format(&diagnostics, source.diagnostic_id(index)) + testing.expect(t, strings.contains(formatted, "warning: unused import 'math'")) + delete(formatted) + } + } + testing.expect(t, found) +} + @(test) unused_missing_package_does_not_trap :: proc(t: ^testing.T) { output := "/tmp/brolang-test-package-missing-unused" @@ -8309,6 +8434,9 @@ main func() void { point Point = undefined pointer @i32 = undefined maybe ?i32 = undefined + _ = point + _ = pointer + _ = maybe } ` source_file := source.Source{path="test.bro", text=text} @@ -9390,7 +9518,7 @@ main func() void {} @(test) contextual_inference_resolves_locals_like_globals :: proc(t: ^testing.T) { - text := `take_u16 func(v u16) void {} + text := `take_u16 func(_ u16) void {} get func() u16 { c :: 10 return c @@ -9403,6 +9531,8 @@ main func() void { b u16 :: a n :: 5 take_u16(n) + _ = z + _ = b _ = get() } ` @@ -9426,7 +9556,7 @@ main func() void { @(test) contextual_inference_demand_from_function_body_reaches_global :: proc(t: ^testing.T) { - text := `take_u16 func(v u16) void {} + text := `take_u16 func(_ u16) void {} G :: 10 main func() void { take_u16(G) @@ -9479,7 +9609,7 @@ contextual_inference_flows_through_compound_assignment :: proc(t: ^testing.T) { @(test) contextual_inference_resolves_open_global_arithmetic_across_uses :: proc(t: ^testing.T) { - text := `take_ci func(v c_int) void {} + text := `take_ci func(_ c_int) void {} W :: 800 Z :: 40 STEP :: 5 @@ -9547,8 +9677,8 @@ contextual_inference_rejects_local_constant_that_does_not_fit :: proc(t: ^testin @(test) contextual_inference_flows_through_numeric_arithmetic :: proc(t: ^testing.T) { - text := `take_u16 func(v u16) void {} -take_f32 func(v f32) void {} + text := `take_u16 func(_ u16) void {} +take_f32 func(_ f32) void {} G :: 10 H u16 :: G + 2 GF :: 1.5 diff --git a/examples/packages/qualified_shadow/app/main.bro b/examples/packages/qualified_shadow/app/main.bro index ca7b657..4910583 100644 --- a/examples/packages/qualified_shadow/app/main.bro +++ b/examples/packages/qualified_shadow/app/main.bro @@ -1,6 +1,6 @@ import "../math" -read func(value i8) int { +read func(_ i8) int { return math.value } diff --git a/examples/programs/conditional_unwrap/main.bro b/examples/programs/conditional_unwrap/main.bro index b0f92d6..900c6fb 100644 --- a/examples/programs/conditional_unwrap/main.bro +++ b/examples/programs/conditional_unwrap/main.bro @@ -33,7 +33,7 @@ main func() i32 { # optional pointer none -> skipped z ?@i32 = none - if z |q| { + if z |_| { total = total + 1000 } diff --git a/examples/programs/unused_locals/main.bro b/examples/programs/unused_locals/main.bro new file mode 100644 index 0000000..966570f --- /dev/null +++ b/examples/programs/unused_locals/main.bro @@ -0,0 +1,12 @@ +warn_only func(value i32, unused i32) i32 { + local i32 = 1 + write_only i32 = 2 + write_only = 3 + consumed i32 = value + _ = consumed + return value +} + +main func() i32 { + return warn_only(7, 9) +} diff --git a/std/mem/heap/heap.bro b/std/mem/heap/heap.bro deleted file mode 100644 index 5846b10..0000000 --- a/std/mem/heap/heap.bro +++ /dev/null @@ -1,9 +0,0 @@ -mem :: import ".." - -alloc func(size usize) ?*mut u8 { - return mem.alloc(mem.heap, size, 1) -} - -free func(memory ?*mut u8) void { - mem.free(mem.heap, memory, 0, 1) -} diff --git a/std/mem/mem.bro b/std/mem/mem.bro index 0d26107..4251ff2 100644 --- a/std/mem/mem.bro +++ b/std/mem/mem.bro @@ -8,10 +8,10 @@ Allocator :: struct { heap Allocator :: Allocator { context = none, - alloc = func(context ?*mut anyopaque, size usize, alignment usize) ?*mut u8 { + alloc = func(_ ?*mut anyopaque, size usize, _ usize) ?*mut u8 { return ptr_cast(u8, c.malloc(size)) }, - free = func(context ?*mut anyopaque, memory ?*mut u8, size usize, alignment usize) void { + free = func(_ ?*mut anyopaque, memory ?*mut u8, _ usize, _ usize) void { c.free(memory) }, }