diff --git a/TODO.md b/TODO.md index 7f0bcf0..00604de 100644 --- a/TODO.md +++ b/TODO.md @@ -603,7 +603,55 @@ - structurally identical anonymous struct payloads share type identity, so sum composition merges matching variants and still rejects same-name variants with different payload shapes -24. dynamic heap allocation +24. bug fixes & interop/indexing oversights (surfaced by the raylib testbed) + - the bouncing-shapes testbed (`testbed/game`) drove most of the language at once and + exposed several gaps; grouped here as the next polish pass. it currently compiles only by + keeping three non-interconverting integer "worlds" (`usize` for indices, the `int` + constraint for `c_int` args, `c_float` for physics) and routing around the items below + - `return match ...` doesn't work (and `yield match ...` probably doesn't either): a `match` + is not accepted directly as a `return`/`yield` operand, so the value must be bound to a + local first, or a statement-`match` with per-arm `return` used — the testbed's `next_kind` + had to do the latter + - signed-index array access must be a diagnostic, not a miscompile: `arr[i]` with a signed + index (`i32`/`int`, including the `int` constraint) currently lowers to malformed LLVM IR + (the indexed slot's address and the stored value get swapped: `store , ptr` with a + value where a `ptr` is expected) for both reads and writes; only `usize` and compile-time + literal indices work. settle the rule — index expressions require an unsigned + (`usize`-coercible) type, as in Rust/Zig — and report a clear checker error instead of + emitting bad IR + - native scalar types should coerce to their C equivalents in general: `f32 -> c_float`, + `f64 -> c_double`, and the integer cases (`i32 -> c_int`, `u8 -> c_uchar`, + `usize -> the matching C width`, …) at call/return/assignment boundaries, following C's own + same-width/family conversions. today only the `int` constraint coerces to `c_int`; + concrete-width native scalars (`i32`, `usize`, `f32`) are stranded, which is why the testbed + had to store physics directly in `c_float` and maintain parallel `usize`/`int` counters + - explicit scalar type casting: there is no cast syntax yet (`c_float(x)` is rejected — that + spelling is distinct-type construction, not a numeric cast), so any conversion the coercion + rules above don't cover is currently impossible. settle a cast spelling; this removes most + of the manual world-juggling the testbed needed + - c scalars must be comparable with numeric literals: `x < 0.0` and `x != 0.0` where + `x : c_float` currently error ("comparison requires compatible numeric operands") even + though `x + 1.0` / `x * 2.0` already accept the literal. a comparison operand literal should + adopt the other operand's concrete (c) scalar type exactly as arithmetic does — otherwise + every zero/bound needs a `c_float` constant alias (the testbed added `ZF`, `SPINMAX`, etc.) + - array sizes should accept compile-time-constant expressions, not just integer literals: + `[CAP]T` with `CAP :: 64` (and `[N + 1]T`, …) should resolve through the same constant + folding used for global initializers. runtime *variable* lengths stay rejected by design — + a runtime-length array is a slice + allocation (milestone 25), not an array type (Zig/Rust + parity: array lengths are comptime-known). the testbed had to hard-code `[64]` and keep a + separate `CAP usize :: 64` for the bounds checks + - peer-type resolution for string literals in value-`match` / value-`if` arms (Zig parity, + not a brolang-specific limitation): arms yielding differently-sized string literals + (`@[6;0]u8` for "circle" vs `@[8;0]u8` for "triangle") currently fail to unify, forcing a + per-arm `DrawText("…")` workaround instead of `tag :: match k { … }; DrawText(tag, …)`. + verified Zig 0.16 behavior: `switch`/`if` arms of string literals peer-resolve to a single + sentinel slice `[:0]const u8` (preserving the common `:0`), which then coerces directly to a + C string `[*c]const u8`. brolang should (a) peer-type-resolve same-sentinel string literals + to a sentinel byte slice `[;0]u8`, and (b) allow a zero-terminated byte slice to convert to + `*c_char` (extending milestone 3.5's pointer-view → `*c_char` rule to the matching sentinel + slice). with both, a value-`match` label passes straight to a `?*c_char` parameter + +25. dynamic heap allocation - see below for direction - notes below are too big in scope for a first pass and the language is not mature enough to support it yet - this first pass should focus on just basic heap allocation, so we have something to work with diff --git a/compiler/ast/ast.odin b/compiler/ast/ast.odin index 135c028..d2b9e45 100644 --- a/compiler/ast/ast.odin +++ b/compiler/ast/ast.odin @@ -160,6 +160,7 @@ Stmt :: struct { label: symbol.Id, type: Type_Syntax, immutable: bool, + value_control_flow: bool, pointer_capture: bool, // Assignments store the lvalue in `target`, the right-hand side in `expr`, // and the source operator in `assignment_op`. `Set` is ordinary `=`; diff --git a/compiler/checker/checker.odin b/compiler/checker/checker.odin index 5e8bf28..b3ff851 100644 --- a/compiler/checker/checker.odin +++ b/compiler/checker/checker.odin @@ -4417,7 +4417,7 @@ build_block :: proc( if typed { expected = type_from_syntax(statement.type) } - value, value_type := build_value_source(ctx, &body, statement.body, expected, statement.span, statement.label) + value, value_type := build_value_source(ctx, &body, statement.body, expected, statement.span, statement.label, statement.value_control_flow) if _, found := find_build_local(ctx.locals^[duplicate_start:], statement.name); found { id := source.addf( checker.diagnostics, statement.span, @@ -4628,7 +4628,7 @@ build_block :: proc( } else if statement.expr == ast.INVALID_EXPR { // `target = { ... yield v }`: build the value block against the // target's type (build_value_block coerces internally). - value, _ = build_value_source(ctx, &body, statement.body, target_type, statement.span, statement.label) + value, _ = build_value_source(ctx, &body, statement.body, target_type, statement.span, statement.label, statement.value_control_flow) } else { value = build_expr( checker, statement.expr, ctx.locals^[:], ctx.global_reads, ctx.calls, @@ -4648,7 +4648,7 @@ build_block :: proc( if statement.name == checker.sink_symbol { value: hir.Expr_Id if statement.expr == ast.INVALID_EXPR { - value, _ = build_value_source(ctx, &body, statement.body, types.INVALID, statement.span, statement.label) + value, _ = build_value_source(ctx, &body, statement.body, types.INVALID, statement.span, statement.label, statement.value_control_flow) } else { value = build_expr(checker, statement.expr, ctx.locals^[:], ctx.global_reads, ctx.calls, types.INVALID, ctx.pkg, ctx.file) } @@ -4692,7 +4692,7 @@ build_block :: proc( } value: hir.Expr_Id if statement.expr == ast.INVALID_EXPR { - value, _ = build_value_source(ctx, &body, statement.body, local.type, statement.span, statement.label) + value, _ = build_value_source(ctx, &body, statement.body, local.type, statement.span, statement.label, statement.value_control_flow) } else { value = build_expr( checker, statement.expr, ctx.locals^[:], ctx.global_reads, ctx.calls, @@ -5430,10 +5430,10 @@ build_value_block :: proc( return value, value_type } -// build_value_source feeds a declaration/assignment RHS into the right value builder: -// a labeled `blk: { ... }` block, a plain `{ ... }` block, an `if` whose branches yield, -// or a `for`/`while` whose iterations yield. All return the produced value and its type -// for the enclosing binding. `label` is the labeled-block label (INVALID otherwise). +// build_value_source feeds a declaration/assignment RHS into the right value builder. +// Braced blocks are plain value blocks and must end in their own `yield`; bare RHS +// control flow (`x :: match ...`) sets `value_control_flow` and can produce directly. +// `label` is the labeled-block label (INVALID otherwise). build_value_source :: proc( ctx: ^Build_Ctx, body: ^[dynamic]hir.Stmt_Id, @@ -5441,12 +5441,13 @@ build_value_source :: proc( expected: types.Type, span: source.Span, label := symbol.INVALID, + value_control_flow := false, ) -> (value: hir.Expr_Id, value_type: types.Type) { checker := ctx.checker if symbol.is_valid(label) { return build_value_labeled_block(ctx, body, body_stmts, label, expected, span) } - if len(body_stmts) == 1 { + if value_control_flow && len(body_stmts) == 1 { #partial switch checker.ast_module.statements[body_stmts[0]].kind { case .If: return build_value_if(ctx, body, body_stmts[0], expected, span) diff --git a/compiler/parser/parser.odin b/compiler/parser/parser.odin index 6cdc3c8..77e148e 100644 --- a/compiler/parser/parser.odin +++ b/compiler/parser/parser.odin @@ -1376,6 +1376,7 @@ parse_statement :: proc(parser: ^Parser) -> ast.Stmt_Id { name=name.symbol, type=type_syntax, immutable=immutable, + value_control_flow=true, target=ast.INVALID_EXPR, expr=ast.INVALID_EXPR, body=body, @@ -1443,6 +1444,7 @@ parse_statement :: proc(parser: ^Parser) -> ast.Stmt_Id { kind=.Assignment, span=span_from(parser.module.exprs[expr].span, previous(parser).span), target=expr, + value_control_flow=true, expr=ast.INVALID_EXPR, body=body, diagnostic=source.INVALID_DIAGNOSTIC, diff --git a/compiler_tests.odin b/compiler_tests.odin index c937cdc..a9e57fc 100644 --- a/compiler_tests.odin +++ b/compiler_tests.odin @@ -2459,9 +2459,16 @@ bad_catch func() i32 { _ = e } } +bad_nested_match func() i32 { + return fa() catch |e| { + match e { + .a: yield 3 + } + } +} main func() i32 { _ = bad_error() catch 0 - return bad_catch() + return bad_catch() + bad_nested_match() } ` source_file := source.Source{path="fallible_ergonomics.bro", text=text} @@ -2478,12 +2485,15 @@ main func() i32 { found_error := false found_yield := false + found_misplaced_yield := false for diagnostic in diagnostics.items { found_error = found_error || strings.contains(diagnostic.message, "'try' error channel cannot be widened") found_yield = found_yield || strings.contains(diagnostic.message, "a value block must end with an explicit 'yield'") + found_misplaced_yield = found_misplaced_yield || strings.contains(diagnostic.message, "'yield' is only valid as the final statement") } testing.expect(t, found_error) testing.expect(t, found_yield) + testing.expect(t, found_misplaced_yield) success_text := `A :: enum { a @@ -2540,10 +2550,11 @@ with_detail func(value i32) i32 ! DetailError { main func() i32 { e DetailError = .code{3} recovered :: with_detail(0) catch |err| { - match err { - .code |n|: yield n - .empty: yield 9 + result i32 :: match err { + .code |n|: n + .empty: 9 } + yield result } return accept(e) + accept(.code{4}) + accept(make_code(5)) + accept(.code{make_payload()}) + recovered - 24 } @@ -2617,17 +2628,19 @@ main func() i32 { a :: accept(e) b :: accept(.wrapped{line = 4, path = 3}) c :: with_payload(0) catch |err| { - match err { - .not_found |info|: yield info.path + info.line - .wrapped |info|: yield info.path + info.line - .scalar |n|: yield n - .empty: yield 0 + result i32 :: match err { + .not_found |info|: info.path + info.line + .wrapped |info|: info.path + info.line + .scalar |n|: n + .empty: 0 } + yield result } d :: inline_payload(0) catch |err| { - match err { - .inline_bad |info|: yield info.code + info.line + result i32 :: match err { + .inline_bad |info|: info.code + info.line } + yield result } return a + b + c + d - 59 } @@ -2672,17 +2685,19 @@ inline_union func(value i32) i32 ! union(enum) { } main func() i32 { a :: inline_enum(0) catch |e| { - if (e == .inline_bad) { + result i32 :: if (e == .inline_bad) { yield 10 } else { yield 11 } + yield result } b :: inline_union(0) catch |e| { - match e { - .inline_code |n|: yield n - .inline_empty: yield 12 + result i32 :: match e { + .inline_code |n|: n + .inline_empty: 12 } + yield result } return a + b - 16 } diff --git a/examples/programs/errors/main.bro b/examples/programs/errors/main.bro index d1f947e..dde83df 100644 --- a/examples/programs/errors/main.bro +++ b/examples/programs/errors/main.bro @@ -59,33 +59,36 @@ via_widen func(value i32) i32 ! BasicOrDetail { catch_basic func(value i32) i32 { return maybe(value) catch |e| { - if (e == .bad) { + result i32 :: if (e == .bad) { yield 21 } else { yield 22 } + yield result } } catch_detail func(value i32) i32 { return with_detail(value) catch |e| { - match e { - .code |n|: yield n + 30 - .info |info|: yield info.code + info.extra + 30 - .empty: yield 40 + result i32 :: match e { + .code |n|: n + 30 + .info |info|: info.code + info.extra + 30 + .empty: 40 } + yield result } } catch_widen func(value i32) i32 { return via_widen(value) catch |e| { - match e { - .bad: yield 50 - .worse: yield 51 - .code |n|: yield n - .info |info|: yield info.code + info.extra - .empty: yield 52 + result i32 :: match e { + .bad: 50 + .worse: 51 + .code |n|: n + .info |info|: info.code + info.extra + .empty: 52 } + yield result } } @@ -138,24 +141,27 @@ main func() i32 { l :: catch_widen(0) m :: catch_widen(-1) n :: inline_enum(0) catch |e| { - if (e == .inline_bad) { + result i32 :: if (e == .inline_bad) { yield 60 } else { yield 61 } + yield result } o :: inline_detail(0) catch |e| { - match e { - .inline_code |value|: yield value + 70 - .inline_empty: yield 80 + result i32 :: match e { + .inline_code |value|: value + 70 + .inline_empty: 80 } + yield result } p :: with_detail(3) catch |e| { - match e { - .code |value|: yield value - .info |info|: yield info.code + info.extra - .empty: yield 0 + result i32 :: match e { + .code |value|: value + .info |info|: info.code + info.extra + .empty: 0 } + yield result } acc = acc + a + b + c + d + e + f + g + h + i + j + k + l + m + n + o + p