Commit f0deef1d79

Andrew Kelley <andrew@ziglang.org>
2021-11-27 07:17:01
Sema: fix analyzeBlockBody logic
Previously, when a coercion needed to be inserted into a break instruction, the `br` AIR instruction would be rewritten so that the block operand was a sub-block that did the coercion. The problem is that the sub-block itself was never added to the parent block, resulting in the `br` instruction operand being a bad reference. Now, the `br` AIR instruction that needs to have coercion instructions added is replaced with the sub-block itself with type `noreturn`, and then the sub-block has the coercion instructions and a new `br` instruction that breaks from the original block. LLVM backend needed to be fixed to lower `noreturn` blocks without emitting an unused LLVM basic block.
1 parent d43ebf5
Changed files (5)
src/codegen/llvm.zig
@@ -2063,8 +2063,14 @@ pub const FuncGen = struct {
         const ty_pl = self.air.instructions.items(.data)[inst].ty_pl;
         const extra = self.air.extraData(Air.Block, ty_pl.payload);
         const body = self.air.extra[extra.end..][0..extra.data.body_len];
+        const inst_ty = self.air.typeOfIndex(inst);
         const parent_bb = self.context.createBasicBlock("Block");
 
+        if (inst_ty.isNoReturn()) {
+            try self.genBody(body);
+            return null;
+        }
+
         var break_bbs: BreakBasicBlocks = .{};
         defer break_bbs.deinit(self.gpa);
 
@@ -2084,7 +2090,6 @@ pub const FuncGen = struct {
         self.builder.positionBuilderAtEnd(parent_bb);
 
         // If the block does not return a value, we dont have to create a phi node.
-        const inst_ty = self.air.typeOfIndex(inst);
         if (!inst_ty.hasCodeGenBits()) return null;
 
         const raw_llvm_ty = try self.dg.llvmType(inst_ty);
src/Sema.zig
@@ -3182,29 +3182,28 @@ fn analyzeBlockBody(
         assert(coerce_block.instructions.items[coerce_block.instructions.items.len - 1] ==
             Air.refToIndex(coerced_operand).?);
 
-        // Convert the br operand to a block.
-        const br_operand_ty_ref = try sema.addType(br_operand_ty);
+        // Convert the br instruction to a block instruction that has the coercion
+        // and then a new br inside that returns the coerced instruction.
+        const sub_block_len = @intCast(u32, coerce_block.instructions.items.len + 1);
         try sema.air_extra.ensureUnusedCapacity(gpa, @typeInfo(Air.Block).Struct.fields.len +
-            coerce_block.instructions.items.len);
-        try sema.air_instructions.ensureUnusedCapacity(gpa, 2);
-        const sub_block_inst = @intCast(Air.Inst.Index, sema.air_instructions.len);
-        const sub_br_inst = sub_block_inst + 1;
-        sema.air_instructions.items(.data)[br].br.operand = Air.indexToRef(sub_block_inst);
-        sema.air_instructions.appendAssumeCapacity(.{
-            .tag = .block,
-            .data = .{ .ty_pl = .{
-                .ty = br_operand_ty_ref,
-                .payload = sema.addExtraAssumeCapacity(Air.Block{
-                    .body_len = @intCast(u32, coerce_block.instructions.items.len),
-                }),
-            } },
-        });
+            sub_block_len);
+        try sema.air_instructions.ensureUnusedCapacity(gpa, 1);
+        const sub_br_inst = @intCast(Air.Inst.Index, sema.air_instructions.len);
+
+        sema.air_instructions.items(.tag)[br] = .block;
+        sema.air_instructions.items(.data)[br] = .{ .ty_pl = .{
+            .ty = Air.Inst.Ref.noreturn_type,
+            .payload = sema.addExtraAssumeCapacity(Air.Block{
+                .body_len = sub_block_len,
+            }),
+        } };
         sema.air_extra.appendSliceAssumeCapacity(coerce_block.instructions.items);
         sema.air_extra.appendAssumeCapacity(sub_br_inst);
+
         sema.air_instructions.appendAssumeCapacity(.{
             .tag = .br,
             .data = .{ .br = .{
-                .block_inst = sub_block_inst,
+                .block_inst = merges.block_inst,
                 .operand = coerced_operand,
             } },
         });
test/behavior/optional.zig
@@ -136,3 +136,26 @@ test "unwrap function call with optional pointer return value" {
     try S.entry();
     comptime try S.entry();
 }
+
+test "nested orelse" {
+    const S = struct {
+        fn entry() !void {
+            try expect(func() == null);
+        }
+        fn maybe() ?Foo {
+            return null;
+        }
+        fn func() ?Foo {
+            const x = maybe() orelse
+                maybe() orelse
+                return null;
+            _ = x;
+            unreachable;
+        }
+        const Foo = struct {
+            field: i32,
+        };
+    };
+    try S.entry();
+    comptime try S.entry();
+}
test/behavior/optional_stage1.zig
@@ -3,29 +3,6 @@ const testing = std.testing;
 const expect = testing.expect;
 const expectEqual = testing.expectEqual;
 
-test "nested orelse" {
-    const S = struct {
-        fn entry() !void {
-            try expect(func() == null);
-        }
-        fn maybe() ?Foo {
-            return null;
-        }
-        fn func() ?Foo {
-            const x = maybe() orelse
-                maybe() orelse
-                return null;
-            _ = x;
-            unreachable;
-        }
-        const Foo = struct {
-            field: i32,
-        };
-    };
-    try S.entry();
-    comptime try S.entry();
-}
-
 test "assigning to an unwrapped optional field in an inline loop" {
     comptime var maybe_pos_arg: ?comptime_int = null;
     inline for ("ab") |x| {
test/behavior.zig
@@ -24,25 +24,25 @@ test {
     _ = @import("behavior/defer.zig");
     _ = @import("behavior/enum.zig");
     _ = @import("behavior/error.zig");
+    _ = @import("behavior/error.zig");
+    _ = @import("behavior/generics.zig");
     _ = @import("behavior/hasdecl.zig");
     _ = @import("behavior/hasfield.zig");
     _ = @import("behavior/if.zig");
     _ = @import("behavior/int128.zig");
+    _ = @import("behavior/member_func.zig");
     _ = @import("behavior/null.zig");
+    _ = @import("behavior/optional.zig");
     _ = @import("behavior/pointers.zig");
     _ = @import("behavior/ptrcast.zig");
     _ = @import("behavior/pub_enum.zig");
     _ = @import("behavior/struct.zig");
+    _ = @import("behavior/this.zig");
+    _ = @import("behavior/translate_c_macros.zig");
     _ = @import("behavior/truncate.zig");
     _ = @import("behavior/underscore.zig");
     _ = @import("behavior/usingnamespace.zig");
     _ = @import("behavior/while.zig");
-    _ = @import("behavior/this.zig");
-    _ = @import("behavior/member_func.zig");
-    _ = @import("behavior/translate_c_macros.zig");
-    _ = @import("behavior/generics.zig");
-    _ = @import("behavior/error.zig");
-    _ = @import("behavior/optional.zig");
 
     if (builtin.object_format != .c) {
         // Tests that pass for stage1 and stage2 but not the C backend.