From ec36b6b861d32558fc6caeccf1d3ef9c2555a633 Mon Sep 17 00:00:00 2001 From: hl-valdemar Date: Mon, 20 Jul 2026 15:28:00 +0200 Subject: [PATCH] fix optional-presence comparison --- compiler/checker/checker.odin | 63 +++++++++++++++++++++++++++------- compiler/checker/comptime.odin | 26 ++++++++++++++ compiler/lower/lower.odin | 19 ++++++++++ compiler_tests.odin | 16 +++++++-- std/testing/testing.bro | 31 +++++++++++++++-- 5 files changed, 138 insertions(+), 17 deletions(-) diff --git a/compiler/checker/checker.odin b/compiler/checker/checker.odin index 567e892..aff8e16 100644 --- a/compiler/checker/checker.odin +++ b/compiler/checker/checker.odin @@ -2730,8 +2730,9 @@ infer_call_comptime_values :: proc( for value_bound in bound { all_bound = all_bound && value_bound } - // Concrete arguments bind first. Numeric constants are contextual and therefore - // only contribute their default type after stronger evidence has had a chance. + // Concrete arguments bind first. Numeric constants and `none` are contextual and + // therefore only contribute after stronger evidence has had a chance to bind the + // parameter type. weak_passes := [2]bool{false, true} for weak in weak_passes { for arg_id, source_index in args { @@ -2742,7 +2743,9 @@ infer_call_comptime_values :: proc( if param_index < 0 || function.params[param_index].comptime_value { continue } - is_weak := is_numeric_constant_expr(checker, arg_id) + arg_expr := checker.ast_module.exprs[arg_id] + is_none := arg_expr.kind == .None + is_weak := is_numeric_constant_expr(checker, arg_id) || is_none if is_weak != weak { continue } @@ -2753,7 +2756,6 @@ infer_call_comptime_values :: proc( // keyed record must be checked against the specialized parameter type. // Its provisional structural type intentionally contains only the supplied // fields, so comparing that type here would reject omitted defaulted fields. - arg_expr := checker.ast_module.exprs[arg_id] if all_bound && arg_expr.kind == .Struct_Literal && !arg_expr.tuple && !symbol.is_valid(arg_expr.name) { continue } @@ -2766,9 +2768,22 @@ infer_call_comptime_values :: proc( } } } + actual := actual_args[param_index] + if is_none { + previous := checker.current_comptime_values + checker.current_comptime_values = values + contextual := type_from_syntax( + checker, function.params[param_index].type, function.pkg, function.file, + ) + checker.current_comptime_values = previous + if types.is_optional(contextual, &checker.module.types) { + actual = contextual + actual_args[param_index] = contextual + } + } matched = match_inferred_type_pattern( checker, function, prefix, function.params[param_index].type, - actual_args[param_index], values, bound, + actual, values, bound, checker.ast_module.exprs[arg_id].span, diagnose, ) && matched } @@ -4482,8 +4497,18 @@ infer_compound_expr :: proc( right := infer_nested_expr(checker, expr.right, locals, pkg, file, demanded, local_types, types.U64) return left if types.is_concrete_integer(left) && types.is_unsigned(right, checker.target) else types.INVALID case .Eq, .Ne, .Lt, .Le, .Gt, .Ge, .And, .Or: - _ = infer_nested_expr(checker, expr.left, locals, pkg, file, demanded, local_types) - _ = infer_nested_expr(checker, expr.right, locals, pkg, file, demanded, local_types) + left_expr := checker.ast_module.exprs[expr.left] + right_expr := checker.ast_module.exprs[expr.right] + if left_expr.kind == .None && right_expr.kind != .None { + right := infer_nested_expr(checker, expr.right, locals, pkg, file, demanded, local_types) + _ = infer_nested_expr(checker, expr.left, locals, pkg, file, demanded, local_types, right) + } else if right_expr.kind == .None && left_expr.kind != .None { + left := infer_nested_expr(checker, expr.left, locals, pkg, file, demanded, local_types) + _ = infer_nested_expr(checker, expr.right, locals, pkg, file, demanded, local_types, left) + } else { + _ = infer_nested_expr(checker, expr.left, locals, pkg, file, demanded, local_types) + _ = infer_nested_expr(checker, expr.right, locals, pkg, file, demanded, local_types) + } return types.BOOL case .Range: left := infer_nested_expr(checker, expr.left, locals, pkg, file, demanded, local_types) @@ -4519,7 +4544,7 @@ infer_compound_expr :: proc( } return types.array(store, element, u64(len(expr.args)), false) case .None: - return types.INVALID + return expected if types.is_optional(expected, store) else types.INVALID case .Undefined: return types.INVALID case .Enum_Literal: @@ -8124,8 +8149,9 @@ build_compound_expr :: proc( left=left, right=right, target=hir.INVALID_REF, diagnostic=source.INVALID_DIAGNOSTIC, }) case .Eq, .Ne, .Lt, .Le, .Gt, .Ge: - // Contextualize a bare integer-literal operand to the other operand's type - // so comparisons like `count > 0` or `0 < count` type-check. + // Contextualize literals whose type comes from their peer. This covers + // integer and enum literals as well as optional presence tests such as + // `value == none` and `none != value`. left_const := eval_constant(checker, expr.left) right_const := eval_constant(checker, expr.right) left, right: hir.Expr_Id @@ -8133,7 +8159,13 @@ build_compound_expr :: proc( right_expr := checker.ast_module.exprs[expr.right] left_numeric_const := left_const.kind == .Value || is_float_constant_expr(checker, expr.left) right_numeric_const := right_const.kind == .Value || is_float_constant_expr(checker, expr.right) - if right_expr.kind == .Enum_Literal && left_expr.kind != .Enum_Literal { + if right_expr.kind == .None && left_expr.kind != .None { + left = build_nested_expr(checker, expr.left, locals, global_reads, calls, types.INVALID, pkg, file) + right = build_nested_expr(checker, expr.right, locals, global_reads, calls, checker.module.exprs[left].type, pkg, file) + } else if left_expr.kind == .None && right_expr.kind != .None { + right = build_nested_expr(checker, expr.right, locals, global_reads, calls, types.INVALID, pkg, file) + left = build_nested_expr(checker, expr.left, locals, global_reads, calls, checker.module.exprs[right].type, pkg, file) + } else if right_expr.kind == .Enum_Literal && left_expr.kind != .Enum_Literal { left = build_nested_expr(checker, expr.left, locals, global_reads, calls, types.INVALID, pkg, file) right = build_nested_expr(checker, expr.right, locals, global_reads, calls, checker.module.exprs[left].type, pkg, file) } else if left_expr.kind == .Enum_Literal && right_expr.kind != .Enum_Literal { @@ -8157,7 +8189,14 @@ build_compound_expr :: proc( left_type := checker.module.exprs[left].type right_type := checker.module.exprs[right].type operand_type := types.INVALID - if types.is_enum(left_type, store) || types.is_enum(right_type, store) { + if left_expr.kind == .None || right_expr.kind == .None { + if expr.kind != .Eq && expr.kind != .Ne || + !types.is_optional(left_type, store) || !types.equal(left_type, right_type) { + id := source.add(checker.diagnostics, expr.span, "'none' only supports '==' and '!=' with an optional value") + return invalid_hir_expr(checker, expr.span, id, types.BOOL) + } + operand_type = left_type + } else if types.is_enum(left_type, store) || types.is_enum(right_type, store) { if !types.equal(left_type, right_type) || (expr.kind != .Eq && expr.kind != .Ne) { id := source.add(checker.diagnostics, expr.span, "enum values only support '==' and '!=' with the same enum type") return invalid_hir_expr(checker, expr.span, id, types.BOOL) diff --git a/compiler/checker/comptime.odin b/compiler/checker/comptime.odin index 51faec4..4242602 100644 --- a/compiler/checker/comptime.odin +++ b/compiler/checker/comptime.odin @@ -1287,6 +1287,20 @@ ct_eval_expr :: proc( types.is_concrete_integer(expected) { left_expected = expected } + left_expr := checker.ast_module.exprs[expr.left] + right_expr := checker.ast_module.exprs[expr.right] + if left_expr.kind == .None && right_expr.kind != .None && + (expr.kind == .Eq || expr.kind == .Ne) { + right, right_flow, right_ok := ct_eval_expr(state, expr.right, types.INVALID, depth+1) + if !right_ok || right_flow.kind != .Normal { + return INVALID_CT_VALUE, right_flow, right_ok + } + left, flow, ok := ct_eval_expr(state, expr.left, state.values[right].type, depth+1) + if !ok || flow.kind != .Normal { + return INVALID_CT_VALUE, flow, ok + } + return ct_eval_binary(state, expr.kind, left, right, expr.span) + } left, flow, ok := ct_eval_expr(state, expr.left, left_expected, depth+1) if !ok || flow.kind != .Normal { return INVALID_CT_VALUE, flow, ok @@ -2118,6 +2132,18 @@ ct_eval_binary :: proc(state: ^Ct_State, op: ast.Expr_Kind, left_id, right_id: C left := state.values[left_id] right := state.values[right_id] is_compare := op == .Eq || op == .Ne || op == .Lt || op == .Le || op == .Gt || op == .Ge + if left.kind == .None || right.kind == .None { + if (op != .Eq && op != .Ne) || + !types.is_optional(left.type, &state.checker.module.types) || + !types.equal(left.type, right.type) { + return INVALID_CT_VALUE, ct_flow(.Normal), ct_fail(state, .Not_Comptime, span, "'none' only supports '==' and '!=' with an optional value") + } + equal := left.kind == .None && right.kind == .None + if op == .Ne { + equal = !equal + } + return ct_add_value(state, Ct_Value{kind=.Bool, type=types.BOOL, integer=1 if equal else 0}), ct_flow(.Normal), true + } if left.kind == .Type || right.kind == .Type { if left.kind != .Type || right.kind != .Type || (op != .Eq && op != .Ne) { return INVALID_CT_VALUE, ct_flow(.Normal), ct_fail(state, .Not_Comptime, span, "type values only support '==' and '!=' with another type") diff --git a/compiler/lower/lower.odin b/compiler/lower/lower.odin index 8c70044..6c15c82 100644 --- a/compiler/lower/lower.odin +++ b/compiler/lower/lower.odin @@ -561,6 +561,25 @@ lower_compound_expr :: proc(state: ^State, expr_id: hir.Expr_Id) -> ir.Instructi diagnostic=source.INVALID_DIAGNOSTIC, }) case .Eq, .Ne, .Lt, .Le, .Gt, .Ge: + left_expr := state.hir_module.exprs[expr.left] + right_expr := state.hir_module.exprs[expr.right] + if left_expr.kind == .None || right_expr.kind == .None { + optional_expr := expr.right if left_expr.kind == .None else expr.left + optional := lower_nested_expr(state, optional_expr) + present := append_instruction(state, ir.Instruction{ + op=.Optional_Is_Some, span=expr.span, type=types.BOOL, + target=ir.INVALID_REF, a=optional, b=ir.INVALID_INSTRUCTION, + diagnostic=source.INVALID_DIAGNOSTIC, + }) + if expr.kind == .Ne { + return present + } + return append_instruction(state, ir.Instruction{ + op=.Not, span=expr.span, type=types.BOOL, + target=ir.INVALID_REF, a=present, b=ir.INVALID_INSTRUCTION, + diagnostic=source.INVALID_DIAGNOSTIC, + }) + } left := lower_nested_expr(state, expr.left) right := lower_nested_expr(state, expr.right) predicate := ir.Compare_Predicate.Eq diff --git a/compiler_tests.odin b/compiler_tests.odin index 7d27a3b..9158f32 100644 --- a/compiler_tests.odin +++ b/compiler_tests.odin @@ -13822,6 +13822,7 @@ enum_field_struct_and_contextual_anonymous_records_compile_and_run :: proc(t: ^t main_path := "/tmp/brolang-test-enum-field-struct/main.bro" output := "/tmp/brolang-test-enum-field-struct-output" text := `meta :: import "@std/meta" +testing :: import "@std/testing" TokenKind :: enum(u8) { ident = 3 @@ -13854,6 +13855,8 @@ Generated func($T type) type { GeneratedInt :: alias Generated(i32) ordered Names = {} +static_none ?i32 :: none +static_some ?i32 :: 1 Map func($E, $V type) type { match typeinfo!(E) { @@ -13894,6 +13897,8 @@ get func($E, $V type, map @Map(E, V), key E) ?V { main func() i32 { _ = ordered + if !$(static_none == none) or !$(none == static_none) or + $(static_some == none) or $(none == static_some) { return 23 } inferred :: {x = 40, name = "bro"} if inferred.x != 40 or inferred.name.len != 3 { return 1 } direct Direct = {x = 7} @@ -13937,10 +13942,17 @@ main func() i32 { }) if !map.present[0] or !map.present[1] or map.present[2] { return 9 } if map.values[0].len != 10 or map.values[1].len != 7 { return 18 } - if get(TokenKind, []u8, &map, TokenKind.ident) |value| { + ident :: get(TokenKind, []u8, &map, TokenKind.ident) + if ident == none or none == ident { return 19 } + if ident |value| { if value.len != 10 { return 19 } } else { return 20 } - if get(TokenKind, []u8, &map, TokenKind.eof) |_| { return 21 } + eof :: get(TokenKind, []u8, &map, TokenKind.eof) + if eof != none or none != eof { return 21 } + location testing.SourceLocation = {file = "test.bro", line = 1, column = 1} + testing.expect_equal(none, eof, location) catch |_| { return 24 } + testing.expect_equal(ident, ident, location) catch |_| { return 25 } + if eof |_| { return 22 } return 0 } ` diff --git a/std/testing/testing.bro b/std/testing/testing.bro index 3de655a..dc8fa44 100644 --- a/std/testing/testing.bro +++ b/std/testing/testing.bro @@ -1,4 +1,5 @@ import "@std/debug" +import "@std/mem" Error :: enum { expectation_failed @@ -18,9 +19,33 @@ expect func(condition bool, location SourceLocation) void ! Error { } expect_equal func($T type, expected, actual T, location SourceLocation) void ! Error { - if expected != actual { - debug.print("{s}:{d}:{d}: expected {}, found {}\n", {location.file, location.line, location.column, expected, actual}) - return .expectation_failed + match typeinfo!(T) { + .optional: { + if expected |expected_value| { + if actual |actual_value| { + try expect_equal(expected_value, actual_value, location) + return + } + debug.print("{s}:{d}:{d}: expected an optional value, found none\n", {location.file, location.line, location.column}) + return .expectation_failed + } + if actual |_| { + debug.print("{s}:{d}:{d}: expected none, found an optional value\n", {location.file, location.line, location.column}) + return .expectation_failed + } + } + .slice: { + if !mem.eql(expected, actual) { + debug.print("{s}:{d}:{d}: expected and actual slices differ\n", {location.file, location.line, location.column}) + return .expectation_failed + } + } + else: { + if expected != actual { + debug.print("{s}:{d}:{d}: expected {}, found {}\n", {location.file, location.line, location.column, expected, actual}) + return .expectation_failed + } + } } }