| author |  Andrew Kelley <andrew@ziglang.org> 2022-01-17 15:21:58-07:00 |
| committer | Andrew Kelley <andrew@ziglang.org> 2022-01-17 15:23:50-07:00 |
| log | f4e051e35d8019c9a8d99ccae8f2e9d8f032629a |
| tree | cbeebf38b85f5aaaaff50ea7f9f875c02ac63667 |
| parent | 79628d48a4429818bddef2e86e2d7073d1955302 |
Sema: fix comptime break semantics
Previously, breaking from an outer block at comptime would result in
incorrect control flow. Now there is a mechanism, `error.ComptimeBreak`,
similar to `error.ComptimeReturn`, to send comptime control flow further
up the stack, to its matching block.
This commit also introduces a new log scope. To use it, pass
`--debug-log sema_zir` and you will see 1 line per ZIR instruction
semantically analyzed. This is useful when you want to understand what
comptime control flow is doing while debugging the compiler.
One more `switch` test case is passing.4 files changed, 59 insertions(+), 24 deletions(-)
test- behavior
src/Module.zig+5
| ... | @@ -2486,6 +2486,9 @@ pub const CompileError = error{ | ... | @@ -2486,6 +2486,9 @@ pub const CompileError = error{ |
| 2486 | /// In a comptime scope, a return instruction was encountered. This error is only seen when | 2486 | /// In a comptime scope, a return instruction was encountered. This error is only seen when |
| 2487 | /// doing a comptime function call. | 2487 | /// doing a comptime function call. |
| 2488 | ComptimeReturn, | 2488 | ComptimeReturn, |
| 2489 | /// In a comptime scope, a break instruction was encountered. This error is only seen when | ||
| 2490 | /// evaluating a comptime block. | ||
| 2491 | ComptimeBreak, | ||
| 2489 | }; | 2492 | }; |
| 2490 | 2493 | ||
| 2491 | pub fn deinit(mod: *Module) void { | 2494 | pub fn deinit(mod: *Module) void { |
| ... | @@ -4446,6 +4449,7 @@ pub fn analyzeFnBody(mod: *Module, decl: *Decl, func: *Fn, arena: Allocator) Sem | ... | @@ -4446,6 +4449,7 @@ pub fn analyzeFnBody(mod: *Module, decl: *Decl, func: *Fn, arena: Allocator) Sem |
| 4446 | error.NeededSourceLocation => unreachable, | 4449 | error.NeededSourceLocation => unreachable, |
| 4447 | error.GenericPoison => unreachable, | 4450 | error.GenericPoison => unreachable, |
| 4448 | error.ComptimeReturn => unreachable, | 4451 | error.ComptimeReturn => unreachable, |
| 4452 | error.ComptimeBreak => unreachable, | ||
| 4449 | else => |e| return e, | 4453 | else => |e| return e, |
| 4450 | }; | 4454 | }; |
| 4451 | if (opt_opv) |opv| { | 4455 | if (opt_opv) |opv| { |
| ... | @@ -4478,6 +4482,7 @@ pub fn analyzeFnBody(mod: *Module, decl: *Decl, func: *Fn, arena: Allocator) Sem | ... | @@ -4478,6 +4482,7 @@ pub fn analyzeFnBody(mod: *Module, decl: *Decl, func: *Fn, arena: Allocator) Sem |
| 4478 | error.NeededSourceLocation => @panic("zig compiler bug: NeededSourceLocation"), | 4482 | error.NeededSourceLocation => @panic("zig compiler bug: NeededSourceLocation"), |
| 4479 | error.GenericPoison => @panic("zig compiler bug: GenericPoison"), | 4483 | error.GenericPoison => @panic("zig compiler bug: GenericPoison"), |
| 4480 | error.ComptimeReturn => @panic("zig compiler bug: ComptimeReturn"), | 4484 | error.ComptimeReturn => @panic("zig compiler bug: ComptimeReturn"), |
| 4485 | error.ComptimeBreak => @panic("zig compiler bug: ComptimeBreak"), | ||
| 4481 | else => |e| return e, | 4486 | else => |e| return e, |
| 4482 | }; | 4487 | }; |
| 4483 | 4488 |
src/Sema.zig+33-3
| ... | @@ -39,6 +39,9 @@ func: ?*Module.Fn, | ... | @@ -39,6 +39,9 @@ func: ?*Module.Fn, |
| 39 | fn_ret_ty: Type, | 39 | fn_ret_ty: Type, |
| 40 | branch_quota: u32 = 1000, | 40 | branch_quota: u32 = 1000, |
| 41 | branch_count: u32 = 0, | 41 | branch_count: u32 = 0, |
| 42 | /// Populated when returning `error.ComptimeBreak`. Used to communicate the | ||
| 43 | /// break instruction up the stack to find the corresponding Block. | ||
| 44 | comptime_break_inst: Zir.Inst.Index = undefined, | ||
| 42 | /// This field is updated when a new source location becomes active, so that | 45 | /// This field is updated when a new source location becomes active, so that |
| 43 | /// instructions which do not have explicitly mapped source locations still have | 46 | /// instructions which do not have explicitly mapped source locations still have |
| 44 | /// access to the source location set by the previous instruction which did | 47 | /// access to the source location set by the previous instruction which did |
| ... | @@ -486,8 +489,31 @@ pub fn deinit(sema: *Sema) void { | ... | @@ -486,8 +489,31 @@ pub fn deinit(sema: *Sema) void { |
| 486 | /// has no peers. | 489 | /// has no peers. |
| 487 | fn resolveBody(sema: *Sema, block: *Block, body: []const Zir.Inst.Index) CompileError!Air.Inst.Ref { | 490 | fn resolveBody(sema: *Sema, block: *Block, body: []const Zir.Inst.Index) CompileError!Air.Inst.Ref { |
| 488 | const break_inst = try sema.analyzeBody(block, body); | 491 | const break_inst = try sema.analyzeBody(block, body); |
| 489 | const operand_ref = sema.code.instructions.items(.data)[break_inst].@"break".operand; | 492 | const break_data = sema.code.instructions.items(.data)[break_inst].@"break"; |
| 490 | return sema.resolveInst(operand_ref); | 493 | // For comptime control flow, we need to detect when `analyzeBody` reports |
| 494 | // that we need to break from an outer block. In such case we | ||
| 495 | // use Zig's error mechanism to send control flow up the stack until | ||
| 496 | // we find the corresponding block to this break. | ||
| 497 | if (block.is_comptime) { | ||
| 498 | if (block.label) |label| { | ||
| 499 | if (label.zir_block != break_data.block_inst) { | ||
| 500 | sema.comptime_break_inst = break_inst; | ||
| 501 | return error.ComptimeBreak; | ||
| 502 | } | ||
| 503 | } | ||
| 504 | } | ||
| 505 | return sema.resolveInst(break_data.operand); | ||
| 506 | } | ||
| 507 | |||
| 508 | pub fn analyzeBody( | ||
| 509 | sema: *Sema, | ||
| 510 | block: *Block, | ||
| 511 | body: []const Zir.Inst.Index, | ||
| 512 | ) CompileError!Zir.Inst.Index { | ||
| 513 | return sema.analyzeBodyInner(block, body) catch |err| switch (err) { | ||
| 514 | error.ComptimeBreak => sema.comptime_break_inst, | ||
| 515 | else => |e| return e, | ||
| 516 | }; | ||
| 491 | } | 517 | } |
| 492 | 518 | ||
| 493 | /// ZIR instructions which are always `noreturn` return this. This matches the | 519 | /// ZIR instructions which are always `noreturn` return this. This matches the |
| ... | @@ -505,7 +531,7 @@ const always_noreturn: CompileError!Zir.Inst.Index = @as(Zir.Inst.Index, undefin | ... | @@ -505,7 +531,7 @@ const always_noreturn: CompileError!Zir.Inst.Index = @as(Zir.Inst.Index, undefin |
| 505 | /// instruction. In this case, the `Zir.Inst.Index` part of the return value will be | 531 | /// instruction. In this case, the `Zir.Inst.Index` part of the return value will be |
| 506 | /// the break instruction. This communicates both which block the break applies to, as | 532 | /// the break instruction. This communicates both which block the break applies to, as |
| 507 | /// well as the operand. No block scope needs to be created for this strategy. | 533 | /// well as the operand. No block scope needs to be created for this strategy. |
| 508 | pub fn analyzeBody( | 534 | fn analyzeBodyInner( |
| 509 | sema: *Sema, | 535 | sema: *Sema, |
| 510 | block: *Block, | 536 | block: *Block, |
| 511 | body: []const Zir.Inst.Index, | 537 | body: []const Zir.Inst.Index, |
| ... | @@ -541,6 +567,9 @@ pub fn analyzeBody( | ... | @@ -541,6 +567,9 @@ pub fn analyzeBody( |
| 541 | const result = while (true) { | 567 | const result = while (true) { |
| 542 | crash_info.setBodyIndex(i); | 568 | crash_info.setBodyIndex(i); |
| 543 | const inst = body[i]; | 569 | const inst = body[i]; |
| 570 | std.log.scoped(.sema_zir).debug("sema ZIR {s} %{d}", .{ | ||
| 571 | block.src_decl.src_namespace.file_scope.sub_file_path, inst, | ||
| 572 | }); | ||
| 544 | const air_inst: Air.Inst.Ref = switch (tags[inst]) { | 573 | const air_inst: Air.Inst.Ref = switch (tags[inst]) { |
| 545 | // zig fmt: off | 574 | // zig fmt: off |
| 546 | .alloc => try sema.zirAlloc(block, inst), | 575 | .alloc => try sema.zirAlloc(block, inst), |
| ... | @@ -4319,6 +4348,7 @@ fn analyzeCall( | ... | @@ -4319,6 +4348,7 @@ fn analyzeCall( |
| 4319 | const result = result: { | 4348 | const result = result: { |
| 4320 | _ = sema.analyzeBody(&child_block, fn_info.body) catch |err| switch (err) { | 4349 | _ = sema.analyzeBody(&child_block, fn_info.body) catch |err| switch (err) { |
| 4321 | error.ComptimeReturn => break :result inlining.comptime_result, | 4350 | error.ComptimeReturn => break :result inlining.comptime_result, |
| 4351 | error.ComptimeBreak => unreachable, // Can't break through a fn call. | ||
| 4322 | else => |e| return e, | 4352 | else => |e| return e, |
| 4323 | }; | 4353 | }; |
| 4324 | break :result try sema.analyzeBlockBody(block, call_src, &child_block, merges); | 4354 | break :result try sema.analyzeBlockBody(block, call_src, &child_block, merges); |
test/behavior/switch.zig+21
| ... | @@ -326,3 +326,24 @@ test "anon enum literal used in switch on union enum" { | ... | @@ -326,3 +326,24 @@ test "anon enum literal used in switch on union enum" { |
| 326 | }, | 326 | }, |
| 327 | } | 327 | } |
| 328 | } | 328 | } |
| 329 | |||
| 330 | test "switch all prongs unreachable" { | ||
| 331 | try testAllProngsUnreachable(); | ||
| 332 | comptime try testAllProngsUnreachable(); | ||
| 333 | } | ||
| 334 | |||
| 335 | fn testAllProngsUnreachable() !void { | ||
| 336 | try expect(switchWithUnreachable(1) == 2); | ||
| 337 | try expect(switchWithUnreachable(2) == 10); | ||
| 338 | } | ||
| 339 | |||
| 340 | fn switchWithUnreachable(x: i32) i32 { | ||
| 341 | while (true) { | ||
| 342 | switch (x) { | ||
| 343 | 1 => return 2, | ||
| 344 | 2 => break, | ||
| 345 | else => continue, | ||
| 346 | } | ||
| 347 | } | ||
| 348 | return 10; | ||
| 349 | } |
test/behavior/switch_stage1.zig-21
| ... | @@ -3,27 +3,6 @@ const expect = std.testing.expect; | ... | @@ -3,27 +3,6 @@ const expect = std.testing.expect; |
| 3 | const expectError = std.testing.expectError; | 3 | const expectError = std.testing.expectError; |
| 4 | const expectEqual = std.testing.expectEqual; | 4 | const expectEqual = std.testing.expectEqual; |
| 5 | 5 | ||
| 6 | test "switch all prongs unreachable" { | ||
| 7 | try testAllProngsUnreachable(); | ||
| 8 | comptime try testAllProngsUnreachable(); | ||
| 9 | } | ||
| 10 | |||
| 11 | fn testAllProngsUnreachable() !void { | ||
| 12 | try expect(switchWithUnreachable(1) == 2); | ||
| 13 | try expect(switchWithUnreachable(2) == 10); | ||
| 14 | } | ||
| 15 | |||
| 16 | fn switchWithUnreachable(x: i32) i32 { | ||
| 17 | while (true) { | ||
| 18 | switch (x) { | ||
| 19 | 1 => return 2, | ||
| 20 | 2 => break, | ||
| 21 | else => continue, | ||
| 22 | } | ||
| 23 | } | ||
| 24 | return 10; | ||
| 25 | } | ||
| 26 | |||
| 27 | fn return_a_number() anyerror!i32 { | 6 | fn return_a_number() anyerror!i32 { |
| 28 | return 1; | 7 | return 1; |
| 29 | } | 8 | } |