Fix bad source locations in switch capture errors

mlugg · mlugg · commit 42dc7539c5b0 · 2023-06-13T12:55:27.000+01:00
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.
diff --git 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
@@ -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
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
@@ -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,
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -4,12 +4,12 @@ const Payload = union {`
`4`	`4`	`C: bool,`
`5`	`5`	`};`
`6`	`6`	`export fn entry() void {`
`7`		`- const a = Payload { .A = 1234 };`
	`7`	`+ const a = Payload{ .A = 1234 };`
`8`	`8`	`foo(&a);`
`9`	`9`	`}`
`10`	`10`	`fn foo(a: *const Payload) void {`
`11`	`11`	`switch (a.*) {`
`12`		`- Payload.A => {},`
	`12`	`+ .A => {},`
`13`	`13`	`else => unreachable,`
`14`	`14`	`}`
`15`	`15`	`}`