fix catch handlers / match statements yielding past scope

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 <struct>, 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

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 `=`;

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:
+// 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,
+// Braced blocks are plain value blocks and must end in their own `yield`; bare RHS
-// or a `for`/`while` whose iterations yield. All return the produced value and its type
+// control flow (`x :: match ...`) sets `value_control_flow` and can produce directly.
-// for the enclosing binding. `label` is the labeled-block label (INVALID otherwise).
+// `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)

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,

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 {
+		result i32 :: match err {
-			.code |n|: yield n
+			.code |n|: n
-			.empty:    yield 9
+			.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 {
+		result i32 :: match err {
-			.not_found |info|: yield info.path + info.line
+			.not_found |info|: info.path + info.line
-			.wrapped |info|:   yield info.path + info.line
+			.wrapped |info|:   info.path + info.line
-			.scalar |n|:       yield n
+			.scalar |n|:       n
-			.empty:            yield 0
+			.empty:            0
 		}
+		yield result
 	}
 	d :: inline_payload(0) catch |err| {
-		match err {
+		result i32 :: match err {
-			.inline_bad |info|: yield info.code + info.line
+			.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 {
+		result i32 :: match e {
-			.inline_code |n|: yield n
+			.inline_code |n|: n
-			.inline_empty:   yield 12
+			.inline_empty:   12
 		}
+		yield result
 	}
 	return a + b - 16
 }

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 {
+		result i32 :: match e {
-			.code |n|:     yield n + 30
+			.code |n|:    n + 30
-			.info |info|:  yield info.code + info.extra + 30
+			.info |info|: info.code + info.extra + 30
-			.empty:        yield 40
+			.empty:       40
 		}
+		yield result
 	}
 }
 
 catch_widen func(value i32) i32 {
 	return via_widen(value) catch |e| {
-		match e {
+		result i32 :: match e {
-			.bad:         yield 50
+			.bad:         50
-			.worse:       yield 51
+			.worse:       51
-			.code |n|:    yield n
+			.code |n|:    n
-			.info |info|: yield info.code + info.extra
+			.info |info|: info.code + info.extra
-			.empty:       yield 52
+			.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 {
+		result i32 :: match e {
-			.inline_code |value|: yield value + 70
+			.inline_code |value|: value + 70
-			.inline_empty:        yield 80
+			.inline_empty:        80
 		}
+		yield result
 	}
 	p :: with_detail(3) catch |e| {
-		match e {
+		result i32 :: match e {
-			.code |value|: yield value
+			.code |value|: value
-			.info |info|:  yield info.code + info.extra
+			.info |info|:  info.code + info.extra
-			.empty:        yield 0
+			.empty:        0
 		}
+		yield result
 	}
 
 	acc = acc + a + b + c + d + e + f + g + h + i + j + k + l + m + n + o + p