From 00609e7edbbc949b12e29a8d9911d988b78d7e03 Mon Sep 17 00:00:00 2001 From: mlugg Date: Fri, 26 May 2023 04:10:51 +0100 Subject: Eliminate switch_capture and switch_capture_ref ZIR tags These tags are unnecessary, as this information can be more efficiently encoded within the switch_block instruction itself. We also use a neat little trick to avoid needing a dummy instruction (like is used for errdefer captures): since the switch_block itself cannot otherwise be referenced within a prong, we can repurpose its index within prongs to refer to the captured value. --- src/Module.zig | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) (limited to 'src/Module.zig') diff --git a/src/Module.zig b/src/Module.zig index 61f39a327a..73ab1c3277 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -5871,6 +5871,7 @@ pub const SwitchProngSrc = union(enum) { multi: Multi, range: Multi, multi_capture: u32, + special, pub const Multi = struct { prong: u32, @@ -5908,14 +5909,22 @@ pub const SwitchProngSrc = union(enum) { var scalar_i: u32 = 0; for (case_nodes) |case_node| { const case = tree.fullSwitchCase(case_node).?; - if (case.ast.values.len == 0) - continue; - if (case.ast.values.len == 1 and - node_tags[case.ast.values[0]] == .identifier and - mem.eql(u8, tree.tokenSlice(main_tokens[case.ast.values[0]]), "_")) - { - continue; + + const is_special = special: { + if (case.ast.values.len == 0) break :special true; + if (case.ast.values.len == 1 and node_tags[case.ast.values[0]] == .identifier) { + break :special mem.eql(u8, tree.tokenSlice(main_tokens[case.ast.values[0]]), "_"); + } + break :special false; + }; + + if (is_special) { + if (prong_src != .special) continue; + return LazySrcLoc.nodeOffset( + decl.nodeIndexToRelative(case.ast.values[0]), + ); } + const is_multi = case.ast.values.len != 1 or node_tags[case.ast.values[0]] == .switch_range; @@ -5956,6 +5965,7 @@ pub const SwitchProngSrc = union(enum) { range_i += 1; } else unreachable; }, + .special => {}, } if (is_multi) { multi_i += 1; -- 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 'src/Module.zig') 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