Skip to content

Commit

Permalink
Fix bad source locations in switch capture errors
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@mlugg
mlugg committed May 30, 2023
1 parent fb73c7a commit b6f22fc
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 81 deletions.
146 changes: 101 additions & 45 deletions src/Module.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2671,12 +2671,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,
Expand Down Expand Up @@ -3157,6 +3168,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
Expand Down Expand Up @@ -3330,6 +3344,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,
Expand Down Expand Up @@ -6009,11 +6024,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,
Expand All @@ -6029,6 +6059,7 @@ pub const SwitchProngSrc = union(enum) {
gpa: Allocator,
decl: *Decl,
switch_node_offset: i32,
/// Ignored if `prong_src` is not `.range`
range_expand: RangeExpand,
) LazySrcLoc {
@setCold(true);
Expand All @@ -6048,7 +6079,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: {
Expand All @@ -6060,60 +6091,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) };
},
}
}
};

Expand Down
32 changes: 19 additions & 13 deletions src/Sema.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10098,7 +10098,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.
Expand Down Expand Up @@ -10215,7 +10215,13 @@ const SwitchProngAnalysis = struct {
if (operand_ty.zigTypeTag() != .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(sema.gpa, sema.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(sema.gpa, sema.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(sema.mod),
Expand Down Expand Up @@ -10679,7 +10685,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
Expand Down Expand Up @@ -11352,7 +11358,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,
Expand Down Expand Up @@ -11435,7 +11441,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,
Expand Down Expand Up @@ -11466,7 +11472,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,
Expand Down Expand Up @@ -11526,7 +11532,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,
Expand Down Expand Up @@ -11857,7 +11863,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,
Expand Down Expand Up @@ -11897,7 +11903,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,
Expand Down Expand Up @@ -11928,7 +11934,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,
Expand Down Expand Up @@ -11956,7 +11962,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,
Expand All @@ -11982,7 +11988,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,
Expand Down Expand Up @@ -12033,7 +12039,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,
Expand Down
21 changes: 0 additions & 21 deletions test/cases/compile_errors/capture_group_on_switch_prong_with_incompatible_payload_types.zig
View file

This file was deleted.

4 changes: 2 additions & 2 deletions test/cases/compile_errors/switch_on_union_with_no_attached_enum.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
Expand Down

0 comments on commit b6f22fc

Please sign in to comment.