From f0deef1d79db272fa80ef0323b4382ee1936a3e4 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 26 Nov 2021 23:17:01 -0700 Subject: [PATCH] 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. --- src/Sema.zig | 33 +++++++++++++++---------------- src/codegen/llvm.zig | 7 ++++++- test/behavior.zig | 12 +++++------ test/behavior/optional.zig | 23 +++++++++++++++++++++ test/behavior/optional_stage1.zig | 23 --------------------- 5 files changed, 51 insertions(+), 47 deletions(-) diff --git a/src/Sema.zig b/src/Sema.zig index 694a69ac2a..fc743c3ba9 100644 --- a/src/Sema.zig +++ b/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, } }, }); diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index b75cc52856..21fa0187e3 100644 --- a/src/codegen/llvm.zig +++ b/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); diff --git a/test/behavior.zig b/test/behavior.zig index c6d090b28a..2da66e841b 100644 --- a/test/behavior.zig +++ b/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. diff --git a/test/behavior/optional.zig b/test/behavior/optional.zig index 4fdea98494..d6d6249c6e 100644 --- a/test/behavior/optional.zig +++ b/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(); +} diff --git a/test/behavior/optional_stage1.zig b/test/behavior/optional_stage1.zig index a20c59f5e9..04c51f8d9d 100644 --- a/test/behavior/optional_stage1.zig +++ b/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| {