From bcb673d94ac09ec381e5b1ad1edf64b603ac68f1 Mon Sep 17 00:00:00 2001 From: mlugg Date: Sun, 28 May 2023 01:45:15 +0100 Subject: Sema: resolve union payload switch captures with peer type resolution This is a bit harder than it seems at first glance. Actually resolving the type is the easy part: the interesting thing is actually getting the capture value. We split this into three cases: * If all payload types are the same (as is required in status quo), we can just do what we already do: get the first field value. * If all payloads are in-memory coercible to the resolved type, we still fetch the first field, but we also emit a `bitcast` to convert to the resolved type. * Otherwise, we need to handle each case separately. We emit a nested `switch_br` which, for each possible case, gets the corresponding union field, and coerces it to the resolved type. As an optimization, the inner switch's 'else' prong is used for any peer which is in-memory coercible to the target type, and the bitcast approach described above is used. Pointer captures have the additional constraint that all payload types must be in-memory coercible to the resolved type. Resolves: #2812 --- .../switch_capture_incompatible_types.zig | 27 ++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 test/cases/compile_errors/switch_capture_incompatible_types.zig (limited to 'test/cases/compile_errors') diff --git a/test/cases/compile_errors/switch_capture_incompatible_types.zig b/test/cases/compile_errors/switch_capture_incompatible_types.zig new file mode 100644 index 0000000000..b6de7d5bf5 --- /dev/null +++ b/test/cases/compile_errors/switch_capture_incompatible_types.zig @@ -0,0 +1,27 @@ +export fn f() void { + const U = union(enum) { a: u32, b: *u8 }; + var u: U = undefined; + switch (u) { + .a, .b => |val| _ = val, + } +} + +export fn g() void { + const U = union(enum) { a: u64, b: u32 }; + var u: U = undefined; + switch (u) { + .a, .b => |*ptr| _ = ptr, + } +} + +// error +// backend=stage2 +// target=native +// +// :5:20: error: capture group with incompatible types +// :5:20: note: incompatible types: 'u32' and '*u8' +// :5:10: note: type 'u32' here +// :5:14: note: type '*u8' here +// :13:20: error: capture group with incompatible types +// :13:14: note: pointer type child 'u32' cannot cast into resolved pointer type child 'u64' +// :13:20: note: this coercion is only possible when capturing by value -- cgit v1.2.3 From 42dc7539c5b0a39e9b64c5ad92757945b0ca05ad Mon Sep 17 00:00:00 2001 From: mlugg Date: Sun, 28 May 2023 04:31:56 +0100 Subject: Fix bad source locations in switch capture errors To do this, I expanded SwitchProngSrc a bit. Several of the tags there aren't actually used by any current errors, but they're there for consistency and if we ever need them. Also delete a now-redundant test and fix another. --- src/Module.zig | 146 ++++++++++++++------- src/Sema.zig | 32 +++-- ...witch_prong_with_incompatible_payload_types.zig | 21 --- .../switch_on_union_with_no_attached_enum.zig | 4 +- 4 files changed, 122 insertions(+), 81 deletions(-) delete mode 100644 test/cases/compile_errors/capture_group_on_switch_prong_with_incompatible_payload_types.zig (limited to 'test/cases/compile_errors') diff --git a/src/Module.zig b/src/Module.zig index 73ab1c3277..d60f3919a5 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -2471,12 +2471,23 @@ pub const SrcLoc = struct { } } else unreachable; }, - .node_offset_switch_prong_capture => |node_off| { + .node_offset_switch_prong_capture, + .node_offset_switch_prong_tag_capture, + => |node_off| { const tree = try src_loc.file_scope.getTree(gpa); const case_node = src_loc.declRelativeToNodeIndex(node_off); const case = tree.fullSwitchCase(case_node).?; - const start_tok = case.payload_token.?; const token_tags = tree.tokens.items(.tag); + const start_tok = switch (src_loc.lazy) { + .node_offset_switch_prong_capture => case.payload_token.?, + .node_offset_switch_prong_tag_capture => blk: { + var tok = case.payload_token.?; + if (token_tags[tok] == .asterisk) tok += 1; + tok += 2; // skip over comma + break :blk tok; + }, + else => unreachable, + }; const end_tok = switch (token_tags[start_tok]) { .asterisk => start_tok + 1, else => start_tok, @@ -2957,6 +2968,9 @@ pub const LazySrcLoc = union(enum) { /// The source location points to the capture of a switch_prong. /// The Decl is determined contextually. node_offset_switch_prong_capture: i32, + /// The source location points to the tag capture of a switch_prong. + /// The Decl is determined contextually. + node_offset_switch_prong_tag_capture: i32, /// The source location points to the align expr of a function type /// expression, found by taking this AST node index offset from the containing /// Decl AST node, which points to a function type AST node. Next, navigate to @@ -3130,6 +3144,7 @@ pub const LazySrcLoc = union(enum) { .node_offset_switch_special_prong, .node_offset_switch_range, .node_offset_switch_prong_capture, + .node_offset_switch_prong_tag_capture, .node_offset_fn_type_align, .node_offset_fn_type_addrspace, .node_offset_fn_type_section, @@ -5867,11 +5882,26 @@ fn lockAndClearFileCompileError(mod: *Module, file: *File) void { } pub const SwitchProngSrc = union(enum) { + /// The item for a scalar prong. scalar: u32, + /// A given single item for a multi prong. multi: Multi, + /// A given range item for a multi prong. range: Multi, - multi_capture: u32, + /// The item for the special prong. special, + /// The main capture for a scalar prong. + scalar_capture: u32, + /// The main capture for a multi prong. + multi_capture: u32, + /// The main capture for the special prong. + special_capture, + /// The tag capture for a scalar prong. + scalar_tag_capture: u32, + /// The tag capture for a multi prong. + multi_tag_capture: u32, + /// The tag capture for the special prong. + special_tag_capture, pub const Multi = struct { prong: u32, @@ -5887,6 +5917,7 @@ pub const SwitchProngSrc = union(enum) { mod: *Module, decl: *Decl, switch_node_offset: i32, + /// Ignored if `prong_src` is not `.range` range_expand: RangeExpand, ) LazySrcLoc { @setCold(true); @@ -5907,7 +5938,7 @@ pub const SwitchProngSrc = union(enum) { var multi_i: u32 = 0; var scalar_i: u32 = 0; - for (case_nodes) |case_node| { + const case_node = for (case_nodes) |case_node| { const case = tree.fullSwitchCase(case_node).?; const is_special = special: { @@ -5919,60 +5950,85 @@ pub const SwitchProngSrc = union(enum) { }; if (is_special) { - if (prong_src != .special) continue; - return LazySrcLoc.nodeOffset( - decl.nodeIndexToRelative(case.ast.values[0]), - ); + switch (prong_src) { + .special, .special_capture, .special_tag_capture => break case_node, + else => continue, + } } const is_multi = case.ast.values.len != 1 or node_tags[case.ast.values[0]] == .switch_range; switch (prong_src) { - .scalar => |i| if (!is_multi and i == scalar_i) return LazySrcLoc.nodeOffset( - decl.nodeIndexToRelative(case.ast.values[0]), - ), - .multi_capture => |i| if (is_multi and i == multi_i) { - return LazySrcLoc{ .node_offset_switch_prong_capture = decl.nodeIndexToRelative(case_node) }; - }, - .multi => |s| if (is_multi and s.prong == multi_i) { - var item_i: u32 = 0; - for (case.ast.values) |item_node| { - if (node_tags[item_node] == .switch_range) continue; - - if (item_i == s.item) return LazySrcLoc.nodeOffset( - decl.nodeIndexToRelative(item_node), - ); - item_i += 1; - } else unreachable; - }, - .range => |s| if (is_multi and s.prong == multi_i) { - var range_i: u32 = 0; - for (case.ast.values) |range| { - if (node_tags[range] != .switch_range) continue; - - if (range_i == s.item) switch (range_expand) { - .none => return LazySrcLoc.nodeOffset( - decl.nodeIndexToRelative(range), - ), - .first => return LazySrcLoc.nodeOffset( - decl.nodeIndexToRelative(node_datas[range].lhs), - ), - .last => return LazySrcLoc.nodeOffset( - decl.nodeIndexToRelative(node_datas[range].rhs), - ), - }; - range_i += 1; - } else unreachable; - }, - .special => {}, + .scalar, + .scalar_capture, + .scalar_tag_capture, + => |i| if (!is_multi and i == scalar_i) break case_node, + + .multi_capture, + .multi_tag_capture, + => |i| if (is_multi and i == multi_i) break case_node, + + .multi, + .range, + => |m| if (is_multi and m.prong == multi_i) break case_node, + + .special, + .special_capture, + .special_tag_capture, + => {}, } + if (is_multi) { multi_i += 1; } else { scalar_i += 1; } } else unreachable; + + const case = tree.fullSwitchCase(case_node).?; + + switch (prong_src) { + .scalar, .special => return LazySrcLoc.nodeOffset( + decl.nodeIndexToRelative(case.ast.values[0]), + ), + .multi => |m| { + var item_i: u32 = 0; + for (case.ast.values) |item_node| { + if (node_tags[item_node] == .switch_range) continue; + if (item_i == m.item) return LazySrcLoc.nodeOffset( + decl.nodeIndexToRelative(item_node), + ); + item_i += 1; + } + unreachable; + }, + .range => |m| { + var range_i: u32 = 0; + for (case.ast.values) |range| { + if (node_tags[range] != .switch_range) continue; + if (range_i == m.item) switch (range_expand) { + .none => return LazySrcLoc.nodeOffset( + decl.nodeIndexToRelative(range), + ), + .first => return LazySrcLoc.nodeOffset( + decl.nodeIndexToRelative(node_datas[range].lhs), + ), + .last => return LazySrcLoc.nodeOffset( + decl.nodeIndexToRelative(node_datas[range].rhs), + ), + }; + range_i += 1; + } + unreachable; + }, + .scalar_capture, .multi_capture, .special_capture => { + return .{ .node_offset_switch_prong_capture = decl.nodeIndexToRelative(case_node) }; + }, + .scalar_tag_capture, .multi_tag_capture, .special_tag_capture => { + return .{ .node_offset_switch_prong_tag_capture = decl.nodeIndexToRelative(case_node) }; + }, + } } }; diff --git a/src/Sema.zig b/src/Sema.zig index 51acda3810..7c4968b0e5 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -10129,7 +10129,7 @@ const SwitchProngAnalysis = struct { prong_type: enum { normal, special }, prong_body: []const Zir.Inst.Index, capture: Zir.Inst.SwitchBlock.ProngInfo.Capture, - /// Must use the `scalar`, `special`, or `multi_capture` union field. + /// Must use the `scalar_capture`, `special_capture`, or `multi_capture` union field. raw_capture_src: Module.SwitchProngSrc, /// The set of all values which can reach this prong. May be undefined /// if the prong is special or contains ranges. @@ -10247,7 +10247,13 @@ const SwitchProngAnalysis = struct { if (operand_ty.zigTypeTag(mod) != .Union) { const zir_datas = sema.code.instructions.items(.data); const switch_node_offset = zir_datas[spa.switch_block_inst].pl_node.src_node; - const capture_src = raw_capture_src.resolve(mod, mod.declPtr(block.src_decl), switch_node_offset, .none); + const raw_tag_capture_src: Module.SwitchProngSrc = switch (raw_capture_src) { + .scalar_capture => |i| .{ .scalar_tag_capture = i }, + .multi_capture => |i| .{ .multi_tag_capture = i }, + .special_capture => .special_tag_capture, + else => unreachable, + }; + const capture_src = raw_tag_capture_src.resolve(mod, mod.declPtr(block.src_decl), switch_node_offset, .none); const msg = msg: { const msg = try sema.errMsg(block, capture_src, "cannot capture tag of non-union type '{}'", .{ operand_ty.fmt(mod), @@ -10712,7 +10718,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index, operand_is_r } }; - const operand = try sema.switchCond(block, src, raw_operand.val); + const operand = try sema.switchCond(block, operand_src, raw_operand.val); // AstGen guarantees that the instruction immediately preceding // switch_block(_ref) is a dbg_stmt @@ -11377,7 +11383,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index, operand_is_r .normal, body, info.capture, - .{ .scalar = @intCast(u32, scalar_i) }, + .{ .scalar_capture = @intCast(u32, scalar_i) }, &.{item}, if (info.is_inline) operand else .none, info.has_tag_capture, @@ -11460,7 +11466,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index, operand_is_r .special, special.body, special.capture, - .special, + .special_capture, undefined, // case_vals may be undefined for special prongs if (special.is_inline) operand else .none, special.has_tag_capture, @@ -11491,7 +11497,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index, operand_is_r .special, special.body, special.capture, - .special, + .special_capture, undefined, // case_vals may be undefined for special prongs .none, false, @@ -11551,7 +11557,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index, operand_is_r .normal, body, info.capture, - .{ .scalar = @intCast(u32, scalar_i) }, + .{ .scalar_capture = @intCast(u32, scalar_i) }, &.{item}, if (info.is_inline) item else .none, info.has_tag_capture, @@ -11885,7 +11891,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index, operand_is_r .special, special.body, special.capture, - .special, + .special_capture, &.{item_ref}, item_ref, special.has_tag_capture, @@ -11929,7 +11935,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index, operand_is_r .special, special.body, special.capture, - .special, + .special_capture, &.{item_ref}, item_ref, special.has_tag_capture, @@ -11960,7 +11966,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index, operand_is_r .special, special.body, special.capture, - .special, + .special_capture, &.{item_ref}, item_ref, special.has_tag_capture, @@ -11988,7 +11994,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index, operand_is_r .special, special.body, special.capture, - .special, + .special_capture, &.{Air.Inst.Ref.bool_true}, Air.Inst.Ref.bool_true, special.has_tag_capture, @@ -12014,7 +12020,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index, operand_is_r .special, special.body, special.capture, - .special, + .special_capture, &.{Air.Inst.Ref.bool_false}, Air.Inst.Ref.bool_false, special.has_tag_capture, @@ -12065,7 +12071,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index, operand_is_r .special, special.body, special.capture, - .special, + .special_capture, undefined, // case_vals may be undefined for special prongs .none, false, diff --git a/test/cases/compile_errors/capture_group_on_switch_prong_with_incompatible_payload_types.zig b/test/cases/compile_errors/capture_group_on_switch_prong_with_incompatible_payload_types.zig deleted file mode 100644 index cff9a58bc6..0000000000 --- a/test/cases/compile_errors/capture_group_on_switch_prong_with_incompatible_payload_types.zig +++ /dev/null @@ -1,21 +0,0 @@ -const Union = union(enum) { - A: usize, - B: isize, -}; -comptime { - var u = Union{ .A = 8 }; - switch (u) { - .A, .B => |e| { - _ = e; - unreachable; - }, - } -} - -// error -// backend=stage2 -// target=native -// -// :8:20: error: capture group with incompatible types -// :8:10: note: type 'usize' here -// :8:14: note: type 'isize' here diff --git a/test/cases/compile_errors/switch_on_union_with_no_attached_enum.zig b/test/cases/compile_errors/switch_on_union_with_no_attached_enum.zig index 4d8742d32e..d9bc2abb91 100644 --- a/test/cases/compile_errors/switch_on_union_with_no_attached_enum.zig +++ b/test/cases/compile_errors/switch_on_union_with_no_attached_enum.zig @@ -4,12 +4,12 @@ const Payload = union { C: bool, }; export fn entry() void { - const a = Payload { .A = 1234 }; + const a = Payload{ .A = 1234 }; foo(&a); } fn foo(a: *const Payload) void { switch (a.*) { - Payload.A => {}, + .A => {}, else => unreachable, } } -- cgit v1.2.3