From 619e54c81382d849f0bfcbe7e5c38f9b34390639 Mon Sep 17 00:00:00 2001 From: Justus Klausecker Date: Fri, 22 May 2026 17:33:25 +0200 Subject: [PATCH 1/4] AstGen: add missing `pointer modifier invalid on discard` failure for switch This: ``` fn foo() void { if ({}) |*_| {} else |err| switch (err) {} } ``` now correctly produces the same compile error in `switchExpr` as this: ``` fn foo() void { if ({}) |*_| {} else |err| (switch (err) {}) } ``` does in `ifExpr`. --- lib/std/zig/AstGen.zig | 1 + test/cases/compile_errors/capture_by_ref_discard.zig | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/lib/std/zig/AstGen.zig b/lib/std/zig/AstGen.zig index c865bd92fe1d3d69cf188cf295ba637ef70a0b9f..1408f3c6f037e6e802ce1944764722955faa1fd8 100644 --- a/lib/std/zig/AstGen.zig +++ b/lib/std/zig/AstGen.zig @@ -7453,6 +7453,7 @@ fn switchExpr( const ident_name = try astgen.identAsString(ident_token); const ident_name_str = tree.tokenSlice(ident_token); if (mem.eql(u8, "_", ident_name_str)) { + if (non_err_is_ref != .no) return astgen.failTok(payload_token, "pointer modifier invalid on discard", .{}); break :scope &scratch_scope.base; } non_err_capture = if (non_err_is_ref != .no) .by_ref else .by_val; diff --git a/test/cases/compile_errors/capture_by_ref_discard.zig b/test/cases/compile_errors/capture_by_ref_discard.zig index 1779936d34fc04ef7f29a275886b1e8e47635d10..c407ede6c9dc4ba9a1db6aea344e5b34d06d5871 100644 --- a/test/cases/compile_errors/capture_by_ref_discard.zig +++ b/test/cases/compile_errors/capture_by_ref_discard.zig @@ -16,9 +16,14 @@ export fn d() void { while (null) |*_| {} } +export fn e() void { + if (0) |*_| {} else |err| switch (err) {} +} + // error // // :2:16: error: pointer modifier invalid on discard // :7:18: error: pointer modifier invalid on discard // :12:16: error: pointer modifier invalid on discard // :16:19: error: pointer modifier invalid on discard +// :20:13: error: pointer modifier invalid on discard -- 2.54.0 From d3056114f6f17bb4723cccfa4dd578ddafb909a9 Mon Sep 17 00:00:00 2001 From: Justus Klausecker Date: Fri, 24 Jul 2026 16:50:07 +0200 Subject: [PATCH 2/4] Sema: disallow unreachable `else` prong for tagged unions with nonexhaustive tag types --- src/Sema.zig | 2 +- ...n_with_nonexhaustive_tag_is_exhaustive.zig | 56 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 test/cases/compile_errors/switch_on_union_with_nonexhaustive_tag_is_exhaustive.zig diff --git a/src/Sema.zig b/src/Sema.zig index 6dafdeb0ee6c53fb6f116272e28c641239573cff..a52b11b506a71f7d4c4c734dbded00d5f5996a40 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -11375,7 +11375,7 @@ fn validateSwitchBlock( if (has_else) { if (all_tags_handled) { - if (item_ty.isNonexhaustiveEnum(zcu)) { + if (operand_ty.isNonexhaustiveEnum(zcu)) { if (has_under) return sema.fail( block, else_prong_src, diff --git a/test/cases/compile_errors/switch_on_union_with_nonexhaustive_tag_is_exhaustive.zig b/test/cases/compile_errors/switch_on_union_with_nonexhaustive_tag_is_exhaustive.zig new file mode 100644 index 0000000000000000000000000000000000000000..fc289fb55fb2c7fb0db1dd990fea16f431509595 --- /dev/null +++ b/test/cases/compile_errors/switch_on_union_with_nonexhaustive_tag_is_exhaustive.zig @@ -0,0 +1,56 @@ +const E = enum(u8) { + a, + b, + _, +}; +const U = union(E) { + a, + b, +}; +fn foo() U { + return undefined; +} + +export fn entry1() void { + const u = foo(); + switch (u) { + .a => {}, + } +} +export fn entry2() void { + const u = foo(); + switch (u) { + .a => {}, + .b => {}, + else => {}, + } +} +export fn entry3() void { + const u = foo(); + switch (u) { + .a => {}, + .b => {}, + _ => {}, + } +} +export fn entry4() void { + const u = foo(); + switch (u) { + .a => {}, + else => {}, + _ => {}, + } +} + +// error +// +// :16:5: error: switch must handle all possibilities +// :3:5: note: unhandled enumeration value: 'b' +// :1:11: note: enum 'tmp.E' declared here +// :25:14: error: unreachable else prong; all cases already handled +// :30:5: error: '_' prong only allowed when switching on non-exhaustive enums +// :33:9: note: '_' prong here +// :30:5: note: consider using 'else' +// :38:5: error: '_' prong only allowed when switching on non-exhaustive enums +// :41:9: note: '_' prong here +// :38:5: note: consider using 'else' -- 2.54.0 From e48779fe1f6127d50008277fb8662566769975da Mon Sep 17 00:00:00 2001 From: Justus Klausecker Date: Fri, 22 May 2026 17:32:38 +0200 Subject: [PATCH 3/4] Sema: improve invalid switch type compile errors Makes them more similar to other existing compile errors. --- src/Sema.zig | 58 ++++++---- .../compile_errors/switch_on_invalid_type.zig | 103 ++++++++++++++++++ .../switch_on_non_packed_struct.zig | 25 ----- 3 files changed, 139 insertions(+), 47 deletions(-) create mode 100644 test/cases/compile_errors/switch_on_invalid_type.zig delete mode 100644 test/cases/compile_errors/switch_on_non_packed_struct.zig diff --git a/src/Sema.zig b/src/Sema.zig index a52b11b506a71f7d4c4c734dbded00d5f5996a40..e1035ea5d58e77d94e08725975cda95982c10ab0 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -11186,14 +11186,8 @@ fn validateSwitchBlock( operand_ty.assertHasLayout(zcu); const union_obj = ip.loadUnionType(operand_ty.toIntern()); switch (union_obj.tag_usage) { - .tagged => { - break :item_ty .fromInterned(union_obj.enum_tag_type); - }, - .none => { - if (union_obj.layout == .@"packed") { - break :item_ty operand_ty; - } - }, + .tagged => break :item_ty .fromInterned(union_obj.enum_tag_type), + .none => if (union_obj.layout == .@"packed") break :item_ty operand_ty, .safety => {}, } return sema.failWithOwnedErrorMsg(block, msg: { @@ -11208,27 +11202,47 @@ fn validateSwitchBlock( .@"struct" => { operand_ty.assertHasLayout(zcu); - const layout = operand_ty.containerLayout(zcu); - if (layout == .@"packed") { - break :item_ty operand_ty; - } + if (operand_ty.containerLayout(zcu) == .@"packed") break :item_ty operand_ty; return sema.failWithOwnedErrorMsg(block, msg: { - const msg = try sema.errMsg(operand_src, "switch on struct with {t} layout", .{layout}); + const msg = try sema.errMsg(operand_src, "switch on non-packed struct", .{}); errdefer msg.destroy(sema.gpa); - if (operand_ty.srcLocOrNull(zcu)) |struct_src| { - try sema.errNote(struct_src, msg, "consider 'packed struct' here", .{}); - } + try sema.addDeclaredHereNote(msg, operand_ty); break :msg msg; }); }, - .pointer => { - if (!operand_ty.isSlice(zcu)) { - break :item_ty operand_ty; - } - }, + .pointer => if (!operand_ty.isSlice(zcu)) break :item_ty operand_ty, - else => {}, + .optional => return sema.failWithOwnedErrorMsg(block, msg: { + const msg = try sema.errMsg(operand_src, "switch on optional type '{f}'", .{ + operand_ty.fmt(pt), + }); + errdefer msg.destroy(gpa); + try sema.errNote(operand_src, msg, "consider using '.?', 'orelse', or 'if'", .{}); + break :msg msg; + }), + + .error_union => return sema.failWithOwnedErrorMsg(block, msg: { + const msg = try sema.errMsg(operand_src, "switch on error union type '{f}'", .{ + operand_ty.fmt(pt), + }); + errdefer msg.destroy(gpa); + try sema.errNote(operand_src, msg, "consider using 'try', 'catch', or 'if'", .{}); + break :msg msg; + }), + + .noreturn, + .float, + .comptime_float, + .array, + .vector, + .undefined, + .null, + .@"opaque", + .frame, + .@"anyframe", + .spirv, + => {}, } return sema.fail(block, operand_src, "switch on type '{f}'", .{operand_ty.fmt(pt)}); }; diff --git a/test/cases/compile_errors/switch_on_invalid_type.zig b/test/cases/compile_errors/switch_on_invalid_type.zig new file mode 100644 index 0000000000000000000000000000000000000000..8d10d75a66c4b4f082bc83628cbc741090ff8428 --- /dev/null +++ b/test/cases/compile_errors/switch_on_invalid_type.zig @@ -0,0 +1,103 @@ +const AutoUnion = union { a: u8 }; +export fn entry1() void { + switch (@as(AutoUnion, .{ .a = 123 })) { + else => {}, + } +} + +const ExternUnion = union { a: u8 }; +export fn entry2() void { + switch (@as(ExternUnion, .{ .a = 123 })) { + else => {}, + } +} + +const AutoStruct = struct { a: u8 }; +export fn entry3() void { + switch (@as(AutoStruct, .{ .a = 123 })) { + else => {}, + } +} + +const ExternStruct = extern struct { a: u8 }; +export fn entry4() void { + switch (@as(ExternStruct, .{ .a = 123 })) { + else => {}, + } +} + +export fn entry5() void { + switch (@as([]const u16, &.{ 1, 2, 3 })) { + else => {}, + } +} + +export fn entry6() void { + switch (@as([3]u16, .{ 1, 2, 3 })) { + else => {}, + } +} + +export fn entry7() void { + switch (@as(@Vector(3, u16), .{ 1, 2, 3 })) { + else => {}, + } +} + +export fn entry8() void { + switch (@as(?u16, 123)) { + else => {}, + } +} + +export fn entry9() void { + switch (@as(anyerror!u16, 123)) { + else => {}, + } +} + +export fn entry10() void { + switch (@as(f32, 123)) { + else => {}, + } +} + +export fn entry11() void { + switch (@as(comptime_float, 123)) { + else => {}, + } +} + +export fn entry12() void { + switch (undefined) { + else => {}, + } +} + +export fn entry13() void { + switch (null) { + else => {}, + } +} + +// error +// +// :3:13: error: switch on union with no attached enum +// :1:19: note: consider 'union(enum)' here +// :10:13: error: switch on union with no attached enum +// :8:21: note: consider 'union(enum)' here +// :17:13: error: switch on non-packed struct +// :15:20: note: struct declared here +// :24:13: error: switch on non-packed struct +// :22:29: note: struct declared here +// :30:13: error: switch on type '[]const u16' +// :36:13: error: switch on type '[3]u16' +// :42:13: error: switch on type '@Vector(3, u16)' +// :48:13: error: switch on optional type '?u16' +// :48:13: note: consider using '.?', 'orelse', or 'if' +// :54:13: error: switch on error union type 'anyerror!u16' +// :54:13: note: consider using 'try', 'catch', or 'if' +// :60:13: error: switch on type 'f32' +// :66:13: error: switch on type 'comptime_float' +// :72:13: error: switch on type '@TypeOf(undefined)' +// :78:13: error: switch on type '@TypeOf(null)' diff --git a/test/cases/compile_errors/switch_on_non_packed_struct.zig b/test/cases/compile_errors/switch_on_non_packed_struct.zig deleted file mode 100644 index ef17bb3ca533361c38f1cb9caae5b391c1f4ab54..0000000000000000000000000000000000000000 --- a/test/cases/compile_errors/switch_on_non_packed_struct.zig +++ /dev/null @@ -1,25 +0,0 @@ -const Auto = struct { - a: u8, -}; -export fn entry1(a: u8) void { - const s: Auto = .{ .a = a }; - switch (s) { - else => {}, - } -} - -const Extern = extern struct { - a: u8, -}; -export fn entry2(s: Extern) void { - switch (s) { - else => {}, - } -} - -// error -// -// :6:13: error: switch on struct with auto layout -// :1:14: note: consider 'packed struct' here -// :15:13: error: switch on struct with extern layout -// :11:23: note: consider 'packed struct' here -- 2.54.0 From c1682a01c1c3994b3a30173197954ba65992bee1 Mon Sep 17 00:00:00 2001 From: Justus Klausecker Date: Fri, 24 Jul 2026 14:34:00 +0200 Subject: [PATCH 4/4] Sema: improve switch duplicate item/range errors Now reports which values are duplicated and the overlap of duplicate ranges. --- src/RangeSet.zig | 41 ++- src/Sema.zig | 243 ++++++++++-------- .../duplicate_boolean_switch_value.zig | 4 +- .../duplicate_error_in_switch.zig | 2 +- ...expression-duplicate_enumeration_prong.zig | 3 +- ...te_enumeration_prong_when_else_present.zig | 3 +- ...witch_expression-duplicate_error_prong.zig | 4 +- ...uplicate_error_prong_when_else_present.zig | 4 +- ...duplicate_or_overlapping_integer_value.zig | 16 -- .../switch_expression-duplicate_type.zig | 2 +- ...expression-duplicate_type_struct_alias.zig | 3 +- .../switch_with_overlapping_case_ranges.zig | 42 ++- 12 files changed, 200 insertions(+), 167 deletions(-) delete mode 100644 test/cases/compile_errors/switch_expression-duplicate_or_overlapping_integer_value.zig diff --git a/src/RangeSet.zig b/src/RangeSet.zig index 3033e8510394cbef68b815601f4eea545fb4feae..291b8ee4e5f001ac715113d63ea30d3f6e0bb6d8 100644 --- a/src/RangeSet.zig +++ b/src/RangeSet.zig @@ -1,6 +1,6 @@ const RangeSet = @This(); -ranges: std.MultiArrayList(Range), +list: std.MultiArrayList(Range), pub const Range = struct { first: Value, @@ -8,41 +8,36 @@ pub const Range = struct { src: LazySrcLoc, }; -pub const empty: RangeSet = .{ .ranges = .empty }; +pub const empty: RangeSet = .{ .list = .empty }; pub fn deinit(self: *RangeSet, allocator: Allocator) void { - self.ranges.deinit(allocator); + self.list.deinit(allocator); self.* = undefined; } -pub fn ensureUnusedCapacity(self: *RangeSet, allocator: Allocator, additional_count: usize) Allocator.Error!void { - return self.ranges.ensureUnusedCapacity(allocator, additional_count); +pub fn ensureUnusedCapacity(set: *RangeSet, allocator: Allocator, additional_count: usize) Allocator.Error!void { + return set.list.ensureUnusedCapacity(allocator, additional_count); } -pub fn addAssumeCapacity(set: *RangeSet, new: Range, ty: Type, zcu: *Zcu) ?LazySrcLoc { +pub fn addAssumeCapacity(set: *RangeSet, new: Range, ty: Type, zcu: *Zcu) ?Range { assert(new.first.typeOf(zcu).eql(ty)); assert(new.last.typeOf(zcu).eql(ty)); assert(new.first.compareScalar(.lte, new.last, ty, zcu)); - const idx = std.sort.lowerBound(Value, set.ranges.items(.last), @as(SearchCtx, .{ + const idx = std.sort.lowerBound(Value, set.list.items(.last), @as(SearchCtx, .{ .val = new.first, .zcu = zcu, }), compare); - if (idx != set.ranges.len and // `new.first` is *not* greater than all `old.last` - new.last.compareScalar(.gte, set.ranges.items(.first)[idx], ty, zcu)) + if (idx != set.list.len and // `new.first` is *not* greater than all `old.last` + new.last.compareScalar(.gte, set.list.items(.first)[idx], ty, zcu)) { - return set.ranges.items(.src)[idx]; // `new` overlaps with existing range. + return set.list.get(idx); // `new` overlaps with existing range. } - set.ranges.insertAssumeCapacity(idx, new); + set.list.insertAssumeCapacity(idx, new); return null; } -pub fn add(set: *RangeSet, allocator: Allocator, new: Range, ty: Type, zcu: *Zcu) Allocator.Error!?LazySrcLoc { - try set.ensureUnusedCapacity(allocator, 1); - return set.addAssumeCapacity(new, ty, zcu); -} - pub fn spans( set: *RangeSet, allocator: Allocator, @@ -53,13 +48,13 @@ pub fn spans( ) Allocator.Error!bool { assert(first.typeOf(zcu).eql(ty)); assert(last.typeOf(zcu).eql(ty)); - if (set.ranges.len == 0) return false; + if (set.list.len == 0) return false; - assert(std.sort.isSorted(Value, set.ranges.items(.first), @as(SortCtx, .{ .ty = ty, .zcu = zcu }), lessThan)); - assert(std.sort.isSorted(Value, set.ranges.items(.last), @as(SortCtx, .{ .ty = ty, .zcu = zcu }), lessThan)); + assert(std.sort.isSorted(Value, set.list.items(.first), @as(SortCtx, .{ .ty = ty, .zcu = zcu }), lessThan)); + assert(std.sort.isSorted(Value, set.list.items(.last), @as(SortCtx, .{ .ty = ty, .zcu = zcu }), lessThan)); - if (!set.ranges.items(.first)[0].eql(first, ty, zcu) or - !set.ranges.items(.last)[set.ranges.len - 1].eql(last, ty, zcu)) + if (!set.list.items(.first)[0].eql(first, ty, zcu) or + !set.list.items(.last)[set.list.len - 1].eql(last, ty, zcu)) { return false; } @@ -75,8 +70,8 @@ pub fn spans( // look for gaps for ( - set.ranges.items(.first)[1..], - set.ranges.items(.last)[0 .. set.ranges.len - 1], + set.list.items(.first)[1..], + set.list.items(.last)[0 .. set.list.len - 1], ) |cur_first, prev_last| { // prev_last + 1 == cur_first counter.copy(prev_last.toBigInt(&space, zcu)); diff --git a/src/Sema.zig b/src/Sema.zig index e1035ea5d58e77d94e08725975cda95982c10ab0..129b9104d385499e8443b4fc45dcf6c2e26497e8 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -10760,7 +10760,7 @@ fn finishSwitchBr( .@"enum" => if (else_is_named_only or !item_ty.isNonexhaustiveEnum(zcu) or tagged_union_originally) { - try branch_hints.ensureUnusedCapacity(gpa, @intCast(validated_switch.seen_enum_fields.len)); + try branch_hints.ensureUnusedCapacity(gpa, @intCast(validated_switch.seen.enum_fields.len)); break :check_enumerable .{ undefined, undefined }; }, .error_set => if (!operand_ty.isAnyError(zcu)) { @@ -10881,13 +10881,13 @@ fn finishSwitchBr( try branch_hints.append(gpa, prong_hint); try cases_extra.ensureUnusedCapacity(gpa, @typeInfo(Air.SwitchBr.Case).@"struct".field_names.len + - (validated_switch.seen_enum_fields.len + 1 - zir_switch.totalItemsLen()) + // +1 because totalItemsLen includes the _ + (validated_switch.seen.enum_fields.len + 1 - zir_switch.totalItemsLen()) + // +1 because totalItemsLen includes the _ case_block.instructions.items.len); const extra_case = cases_extra.addManyAsArrayAssumeCapacity( @typeInfo(Air.SwitchBr.Case).@"struct".field_names.len, ); var items_len: u32 = 0; - for (validated_switch.seen_enum_fields, 0..) |seen_field, field_i| { + for (validated_switch.seen.enum_fields, 0..) |seen_field, field_i| { if (seen_field != null) continue; const item_val = try pt.enumValueFieldIndex(item_ty, @intCast(field_i)); const item_ref: Air.Inst.Ref = .fromValue(item_val); @@ -10920,7 +10920,7 @@ fn finishSwitchBr( } if (tagged_union_originally) { const union_obj = zcu.typeToUnion(operand_ty).?; - for (validated_switch.seen_enum_fields, 0..) |seen_field, field_i| { + for (validated_switch.seen.enum_fields, 0..) |seen_field, field_i| { if (seen_field != null) continue; const field_ty: Type = .fromInterned(union_obj.field_types.get(ip)[field_i]); if (!field_ty.isNoReturn(zcu)) break :analyze_body true; @@ -11004,17 +11004,21 @@ fn finishSwitchBr( } const ValidatedSwitchBlock = struct { - seen_enum_fields: []const ?LazySrcLoc, - seen_errors: std.AutoHashMapUnmanaged(InternPool.NullTerminatedString, LazySrcLoc), - seen_ranges: std.MultiArrayList(RangeSet.Range).Slice, - true_src: ?LazySrcLoc, - false_src: ?LazySrcLoc, - void_src: ?LazySrcLoc, - + seen: Seen, case_vals: []const Air.Inst.Ref, else_case: Zir.UnwrappedSwitchBlock.Case.Else, else_err_ty: ?Type, + const Seen = struct { + enum_fields: []?LazySrcLoc, + errors: std.AutoHashMapUnmanaged(InternPool.NullTerminatedString, LazySrcLoc), + sparse_values: std.AutoHashMapUnmanaged(InternPool.Index, LazySrcLoc), + ranges: RangeSet, + true_src: ?LazySrcLoc, + false_src: ?LazySrcLoc, + void_src: ?LazySrcLoc, + }; + fn iterateUnhandledItems( validated_switch: *const ValidatedSwitchBlock, /// May be `undefined` if `item_ty` isn't an `error_set`. @@ -11023,28 +11027,26 @@ const ValidatedSwitchBlock = struct { min_int: Value, ) UnhandledIterator { return .{ + .error_names = error_names, + .seen = &validated_switch.seen, + .next_idx = 0, .next_val = min_int, - .error_names = error_names, - .seen_enum_fields = validated_switch.seen_enum_fields, - .seen_errors = &validated_switch.seen_errors, - .seen_ranges = validated_switch.seen_ranges, - .seen_true = validated_switch.true_src != null, - .seen_false = validated_switch.false_src != null, - .seen_void = validated_switch.void_src != null, + .handled_true = validated_switch.seen.true_src != null, + .handled_false = validated_switch.seen.false_src != null, + .handled_void = validated_switch.seen.void_src != null, }; } const UnhandledIterator = struct { + error_names: InternPool.NullTerminatedString.Slice, + seen: *const Seen, + next_idx: u32, next_val: ?Value, - error_names: InternPool.NullTerminatedString.Slice, - seen_enum_fields: []const ?LazySrcLoc, - seen_errors: *const std.AutoHashMapUnmanaged(InternPool.NullTerminatedString, LazySrcLoc), - seen_ranges: std.MultiArrayList(RangeSet.Range).Slice, - seen_true: bool, - seen_false: bool, - seen_void: bool, + handled_true: bool, + handled_false: bool, + handled_void: bool, fn next(it: *UnhandledIterator, sema: *Sema, item_ty: Type) CompileError!?Value { const pt = sema.pt; @@ -11052,7 +11054,7 @@ const ValidatedSwitchBlock = struct { const ip = &zcu.intern_pool; switch (item_ty.zigTypeTag(zcu)) { .@"enum" => { - for (it.seen_enum_fields[it.next_idx..], it.next_idx..) |seen_field, field_i| { + for (it.seen.enum_fields[it.next_idx..], it.next_idx..) |seen_field, field_i| { if (seen_field != null) continue; it.next_idx = @intCast(field_i + 1); return try pt.enumValueFieldIndex(item_ty, @intCast(field_i)); @@ -11061,7 +11063,7 @@ const ValidatedSwitchBlock = struct { }, .error_set => { for (it.error_names.get(ip)[it.next_idx..], it.next_idx..) |err_name, name_i| { - if (it.seen_errors.contains(err_name)) continue; + if (it.seen.errors.contains(err_name)) continue; it.next_idx = @intCast(name_i + 1); return .fromInterned(try pt.intern(.{ .err = .{ .ty = item_ty.toIntern(), @@ -11077,14 +11079,14 @@ const ValidatedSwitchBlock = struct { .@"union", .@"struct" => item_ty.backingIntType(zcu), else => unreachable, }; - while (it.next_idx < it.seen_ranges.len and - cur_val.eql(it.seen_ranges.items(.first)[it.next_idx], int_ty, zcu)) + while (it.next_idx < it.seen.ranges.list.len and + cur_val.eql(it.seen.ranges.list.items(.first)[it.next_idx], int_ty, zcu)) { defer it.next_idx += 1; const incr = try arith.incrementDefinedInt( sema, int_ty, - it.seen_ranges.items(.last)[it.next_idx], + it.seen.ranges.list.items(.last)[it.next_idx], ); if (incr.overflow) { it.next_val = null; @@ -11101,19 +11103,19 @@ const ValidatedSwitchBlock = struct { }; }, .bool => { - if (!it.seen_true) { - it.seen_true = true; + if (!it.handled_true) { + it.handled_true = true; return .true; } - if (!it.seen_false) { - it.seen_false = true; + if (!it.handled_false) { + it.handled_false = true; return .false; } return null; }, .void => { - if (!it.seen_void) { - it.seen_void = true; + if (!it.handled_void) { + it.handled_void = true; return .void; } return null; @@ -11267,13 +11269,15 @@ fn validateSwitchBlock( var case_vals: std.ArrayList(Air.Inst.Ref) = try .initCapacity(arena, zir_switch.item_infos.len); // Duplicate checking variables later also used for `inline else`. - var seen_enum_fields: []?LazySrcLoc = &.{}; - var seen_errors: std.AutoHashMapUnmanaged(InternPool.NullTerminatedString, LazySrcLoc) = .empty; - var seen_sparse_values: std.AutoHashMapUnmanaged(InternPool.Index, LazySrcLoc) = .empty; - var range_set: RangeSet = .empty; - var true_src: ?LazySrcLoc = null; - var false_src: ?LazySrcLoc = null; - var void_src: ?LazySrcLoc = null; + var seen: ValidatedSwitchBlock.Seen = .{ + .enum_fields = &.{}, + .errors = .empty, + .sparse_values = .empty, + .ranges = .empty, + .true_src = null, + .false_src = null, + .void_src = null, + }; var else_err_ty: ?Type = null; @@ -11281,20 +11285,20 @@ fn validateSwitchBlock( switch (item_ty.zigTypeTag(zcu)) { .@"enum" => { - seen_enum_fields = try arena.alloc(?LazySrcLoc, item_ty.enumFieldCount(zcu)); - @memset(seen_enum_fields, null); - // `range_set` is used for non-exhaustive enum values that do not + seen.enum_fields = try arena.alloc(?LazySrcLoc, item_ty.enumFieldCount(zcu)); + @memset(seen.enum_fields, null); + // `seen.ranges` is used for non-exhaustive enum values that do not // correspond to any tags. Since this is rare, we only allocate on // demand in `validateSwitchItem`. }, .error_set => { - try seen_errors.ensureUnusedCapacity(arena, zir_switch.totalItemsLen()); + try seen.errors.ensureUnusedCapacity(arena, zir_switch.totalItemsLen()); }, .int, .comptime_int, .@"union", .@"struct" => { - try range_set.ensureUnusedCapacity(arena, zir_switch.totalItemsLen()); + try seen.ranges.ensureUnusedCapacity(arena, zir_switch.totalItemsLen()); }, .enum_literal, .@"fn", .pointer, .type => { - try seen_sparse_values.ensureUnusedCapacity(arena, zir_switch.totalItemsLen()); + try seen.sparse_values.ensureUnusedCapacity(arena, zir_switch.totalItemsLen()); }, .bool, .void => {}, @@ -11337,7 +11341,7 @@ fn validateSwitchBlock( case_vals.appendAssumeCapacity(.none); } else { const item, extra_index = try sema.resolveSwitchItem(block, item_src, item_ty, item_info, extra_index, switch_inst, prong_info.is_comptime_unreach); - try sema.validateSwitchItemOrRange(block, item_src, item.val, null, item_ty, seen_enum_fields, &seen_errors, &seen_sparse_values, &range_set, &true_src, &false_src, &void_src); + try sema.validateSwitchItemOrRange(block, item_src, item.val, null, item_ty, &seen); case_vals.appendAssumeCapacity(item.ref); } } @@ -11352,7 +11356,7 @@ fn validateSwitchBlock( const last_src = block.src(.{ .switch_case_item_range_last = range_offset }); const first_item, extra_index = try sema.resolveSwitchItem(block, first_src, item_ty, range_info[0], extra_index, switch_inst, prong_info.is_comptime_unreach); const last_item, extra_index = try sema.resolveSwitchItem(block, last_src, item_ty, range_info[1], extra_index, switch_inst, prong_info.is_comptime_unreach); - try sema.validateSwitchItemOrRange(block, range_src, first_item.val, last_item.val, item_ty, seen_enum_fields, &seen_errors, &seen_sparse_values, &range_set, &true_src, &false_src, &void_src); + try sema.validateSwitchItemOrRange(block, range_src, first_item.val, last_item.val, item_ty, &seen); case_vals.appendSliceAssumeCapacity(&.{ first_item.ref, last_item.ref }); } } @@ -11383,7 +11387,7 @@ fn validateSwitchBlock( // Validate for missing special prongs. switch (item_ty.zigTypeTag(zcu)) { .@"enum" => { - const all_tags_handled = for (seen_enum_fields) |seen_src| { + const all_tags_handled = for (seen.enum_fields) |seen_src| { if (seen_src == null) break false; } else true; @@ -11411,7 +11415,7 @@ fn validateSwitchBlock( .{}, ); errdefer msg.destroy(sema.gpa); - for (seen_enum_fields, 0..) |seen_src, i| { + for (seen.enum_fields, 0..) |seen_src, i| { if (seen_src != null) continue; const field_name = item_ty.enumFieldName(i, zcu); @@ -11463,7 +11467,7 @@ fn validateSwitchBlock( var seen_errors_from_set: u32 = 0; for (error_names.get(ip)) |error_name| { - if (seen_errors.contains(error_name)) { + if (seen.errors.contains(error_name)) { seen_errors_from_set += 1; } else if (!has_else) { const msg = maybe_msg orelse blk: { @@ -11505,7 +11509,7 @@ fn validateSwitchBlock( var names: InferredErrorSet.NameMap = .{}; try names.ensureUnusedCapacity(sema.arena, error_names.len); for (error_names.get(ip)) |error_name| { - if (seen_errors.contains(error_name)) continue; + if (seen.errors.contains(error_name)) continue; names.putAssumeCapacityNoClobber(error_name, {}); } // No need to keep the hash map metadata correct; here we @@ -11523,7 +11527,7 @@ fn validateSwitchBlock( }; const min_int = try int_ty.minInt(pt, int_ty); const max_int = try int_ty.maxInt(pt, int_ty); - if (try range_set.spans(arena, min_int, max_int, int_ty, zcu)) { + if (try seen.ranges.spans(arena, min_int, max_int, int_ty, zcu)) { if (has_else) { return sema.fail( block, @@ -11556,8 +11560,8 @@ fn validateSwitchBlock( }, .bool, .void => |type_tag| { const all_values_handled = switch (type_tag) { - .bool => true_src != null and false_src != null, - .void => void_src != null, + .bool => seen.true_src != null and seen.false_src != null, + .void => seen.void_src != null, else => unreachable, }; if (has_else) { @@ -11584,13 +11588,7 @@ fn validateSwitchBlock( } return .{ - .seen_enum_fields = seen_enum_fields, - .seen_errors = seen_errors, - .seen_ranges = range_set.ranges.slice(), - .true_src = true_src, - .false_src = false_src, - .void_src = void_src, - + .seen = seen, .case_vals = case_vals.items, .else_case = else_case, .else_err_ty = else_err_ty, @@ -11768,7 +11766,7 @@ fn resolveSwitchBlock( .{ else_case.index, else_case.body, else_case.capture, else_case.has_tag_capture, else_case.is_inline }; if (err_set) try sema.maybeErrorUnwrapComptime(child_block, body, cond_ref); if (tagged_union_originally) { - for (validated_switch.seen_enum_fields, 0..) |maybe_seen, field_i| { + for (validated_switch.seen.enum_fields, 0..) |maybe_seen, field_i| { if (maybe_seen != null) continue; if (!operand_ty.unionFieldTypeByIndex(field_i, zcu).isNoReturn(zcu)) break; } else { @@ -12573,13 +12571,7 @@ fn validateSwitchItemOrRange( item_val: Value, opt_last_val: ?Value, item_ty: Type, - seen_enum_fields: []?LazySrcLoc, - seen_errors: *std.AutoHashMapUnmanaged(InternPool.NullTerminatedString, LazySrcLoc), - seen_sparse_values: *std.AutoHashMapUnmanaged(InternPool.Index, LazySrcLoc), - range_set: *RangeSet, - true_src: *?LazySrcLoc, - false_src: *?LazySrcLoc, - void_src: *?LazySrcLoc, + seen: *ValidatedSwitchBlock.Seen, ) CompileError!void { const pt = sema.pt; const zcu = pt.zcu; @@ -12588,88 +12580,117 @@ fn validateSwitchItemOrRange( .@"enum" => { const int = ip.indexToKey(item_val.toIntern()).enum_tag.int; if (ip.loadEnumType(item_ty.toIntern()).tagValueIndex(ip, int)) |field_index| { - const maybe_prev_src = seen_enum_fields[field_index]; - seen_enum_fields[field_index] = item_src; + const maybe_prev_src = seen.enum_fields[field_index]; + seen.enum_fields[field_index] = item_src; break :maybe_prev_src maybe_prev_src; } else { - break :maybe_prev_src try range_set.add(sema.arena, .{ + try seen.ranges.ensureUnusedCapacity(sema.arena, 1); + break :maybe_prev_src if (seen.ranges.addAssumeCapacity(.{ .first = .fromInterned(int), .last = .fromInterned(int), .src = item_src, - }, .fromInterned(ip.typeOf(int)), zcu); + }, .fromInterned(ip.typeOf(int)), zcu)) |prev| prev.src else null; } }, .error_set => { const error_name = ip.indexToKey(item_val.toIntern()).err.name; - break :maybe_prev_src if (seen_errors.fetchPutAssumeCapacity(error_name, item_src)) |prev| + break :maybe_prev_src if (seen.errors.fetchPutAssumeCapacity(error_name, item_src)) |prev| prev.value else null; }, .int, .comptime_int => { - if (opt_last_val) |last_val| { - const first_val = item_val; + const first_val = item_val; + const last_val: Value = last_val: { + const last_val = opt_last_val orelse break :last_val item_val; if (try first_val.compareAll(.gt, last_val, item_ty, pt)) { return sema.fail(block, item_src, "range start value is greater than the end value", .{}); } - break :maybe_prev_src range_set.addAssumeCapacity(.{ - .first = first_val, - .last = last_val, - .src = item_src, - }, item_ty, zcu); - } else { - break :maybe_prev_src range_set.addAssumeCapacity(.{ - .first = item_val, - .last = item_val, - .src = item_src, - }, item_ty, zcu); + break :last_val last_val; + }; + if (seen.ranges.addAssumeCapacity(.{ + .first = first_val, + .last = last_val, + .src = item_src, + }, item_ty, zcu)) |prev_range| { + const overlap_start = first_val.numberMax(prev_range.first, zcu); + const overlap_end = last_val.numberMin(prev_range.last, zcu); + if (overlap_start.eql(overlap_end, item_ty, zcu)) { + return sema.failWithOwnedErrorMsg(block, msg: { + const msg = try sema.errMsg(item_src, "duplicate switch value '{f}'", .{ + overlap_start.fmtValueSema(pt, sema), + }); + errdefer msg.destroy(sema.gpa); + if (prev_range.first.eql(prev_range.last, item_ty, zcu)) { + try sema.errNote(prev_range.src, msg, "previous value here", .{}); + } else { + try sema.errNote(prev_range.src, msg, "previous value inside range here", .{}); + } + break :msg msg; + }); + } + assert(!prev_range.first.eql(prev_range.last, item_ty, zcu)); + return sema.failWithOwnedErrorMsg(block, msg: { + const msg = try sema.errMsg(item_src, "duplicate switch ranges", .{}); + errdefer msg.destroy(sema.gpa); + if (first_val.eql(prev_range.first, item_ty, zcu) and + last_val.eql(prev_range.last, item_ty, zcu)) + { + try sema.errNote(prev_range.src, msg, "previous range here", .{}); + } else { + try sema.errNote(prev_range.src, msg, "overlaps with previous range here", .{}); + try sema.errNote(prev_range.src, msg, "ranges overlap from '{f}' to '{f}'", .{ + overlap_start.fmtValueSema(pt, sema), overlap_end.fmtValueSema(pt, sema), + }); + } + break :msg msg; + }); } + break :maybe_prev_src null; }, .@"union", .@"struct" => { const backing_int_val = ip.indexToKey(item_val.toIntern()).bitpack.backing_int_val; - break :maybe_prev_src range_set.addAssumeCapacity(.{ + break :maybe_prev_src if (seen.ranges.addAssumeCapacity(.{ .first = .fromInterned(backing_int_val), .last = .fromInterned(backing_int_val), .src = item_src, - }, item_ty.backingIntType(zcu), zcu); + }, item_ty.backingIntType(zcu), zcu)) |prev| prev.src else null; }, .enum_literal, .@"fn", .pointer, .type => { - break :maybe_prev_src if (seen_sparse_values.fetchPutAssumeCapacity(item_val.toIntern(), item_src)) |prev| + break :maybe_prev_src if (seen.sparse_values.fetchPutAssumeCapacity(item_val.toIntern(), item_src)) |prev| prev.value else null; }, .bool => { if (item_val.toBool()) { - if (true_src.*) |prev_src| break :maybe_prev_src prev_src; - true_src.* = item_src; + if (seen.true_src) |prev_src| break :maybe_prev_src prev_src; + seen.true_src = item_src; } else { - if (false_src.*) |prev_src| break :maybe_prev_src prev_src; - false_src.* = item_src; + if (seen.false_src) |prev_src| break :maybe_prev_src prev_src; + seen.false_src = item_src; } break :maybe_prev_src null; }, .void => { - if (void_src.*) |prev_src| break :maybe_prev_src prev_src; - void_src.* = item_src; + if (seen.void_src) |prev_src| break :maybe_prev_src prev_src; + seen.void_src = item_src; break :maybe_prev_src null; }, else => unreachable, // should have already checked for invalid types }; if (maybe_prev_src) |prev_src| { return sema.failWithOwnedErrorMsg(block, msg: { - const msg = try sema.errMsg( - item_src, - "duplicate switch value", - .{}, - ); + const msg = try sema.errMsg(item_src, "duplicate switch value '{f}'", .{ + item_val.fmtValueSema(pt, sema), + }); errdefer msg.destroy(sema.gpa); - try sema.errNote( - prev_src, - msg, - "previous value here", - .{}, - ); + try sema.errNote(prev_src, msg, "previous value here", .{}); + if (item_ty.zigTypeTag(zcu) == .type) { + try sema.addDeclaredHereNote(msg, item_val.toType()); + } else { + try sema.addDeclaredHereNote(msg, item_ty); + } break :msg msg; }); } diff --git a/test/cases/compile_errors/duplicate_boolean_switch_value.zig b/test/cases/compile_errors/duplicate_boolean_switch_value.zig index d3b7dba6e88e77e75b01861d691d02da4f9847e6..700851ec77b1c9dc6d10eb0cd9f37f1aa0a69bcb 100644 --- a/test/cases/compile_errors/duplicate_boolean_switch_value.zig +++ b/test/cases/compile_errors/duplicate_boolean_switch_value.zig @@ -17,7 +17,7 @@ comptime { // error // -// :5:9: error: duplicate switch value +// :5:9: error: duplicate switch value 'true' // :3:9: note: previous value here -// :13:9: error: duplicate switch value +// :13:9: error: duplicate switch value 'false' // :11:9: note: previous value here diff --git a/test/cases/compile_errors/duplicate_error_in_switch.zig b/test/cases/compile_errors/duplicate_error_in_switch.zig index 91f6f13c7ea158195da1aa16993f7982f9ef010f..6a2da7672e1d0851b975858e414c333313fb93fd 100644 --- a/test/cases/compile_errors/duplicate_error_in_switch.zig +++ b/test/cases/compile_errors/duplicate_error_in_switch.zig @@ -16,5 +16,5 @@ fn foo(x: i32) !void { // error // -// :5:9: error: duplicate switch value +// :5:9: error: duplicate switch value 'error.Foo' // :3:9: note: previous value here diff --git a/test/cases/compile_errors/switch_expression-duplicate_enumeration_prong.zig b/test/cases/compile_errors/switch_expression-duplicate_enumeration_prong.zig index 754451321c91d75d0d465e18d475ff08bae9865a..5c69bd922f2011cc900c50e891e4ee387a49b47f 100644 --- a/test/cases/compile_errors/switch_expression-duplicate_enumeration_prong.zig +++ b/test/cases/compile_errors/switch_expression-duplicate_enumeration_prong.zig @@ -20,5 +20,6 @@ export fn entry() usize { // error // -// :13:15: error: duplicate switch value +// :13:15: error: duplicate switch value '.Two' // :10:15: note: previous value here +// :1:16: note: enum declared here diff --git a/test/cases/compile_errors/switch_expression-duplicate_enumeration_prong_when_else_present.zig b/test/cases/compile_errors/switch_expression-duplicate_enumeration_prong_when_else_present.zig index 3cba599968fd15a1abdc9b64e1fd4fb984bd363b..24627b6282194defe031d9db2693026b8c381984 100644 --- a/test/cases/compile_errors/switch_expression-duplicate_enumeration_prong_when_else_present.zig +++ b/test/cases/compile_errors/switch_expression-duplicate_enumeration_prong_when_else_present.zig @@ -21,5 +21,6 @@ export fn entry() usize { // error // -// :13:15: error: duplicate switch value +// :13:15: error: duplicate switch value '.Two' // :10:15: note: previous value here +// :1:16: note: enum declared here diff --git a/test/cases/compile_errors/switch_expression-duplicate_error_prong.zig b/test/cases/compile_errors/switch_expression-duplicate_error_prong.zig index 1ee6add616cc0c0546a0b9f3833ed13072eb0d65..3df559cf03a73bae9f06d6c84642952e805a1a5e 100644 --- a/test/cases/compile_errors/switch_expression-duplicate_error_prong.zig +++ b/test/cases/compile_errors/switch_expression-duplicate_error_prong.zig @@ -25,7 +25,7 @@ export fn entry() usize { // error // -// :8:9: error: duplicate switch value +// :8:9: error: duplicate switch value 'error.Foo' // :5:9: note: previous value here -// :16:9: error: duplicate switch value +// :16:9: error: duplicate switch value 'error.Foo' // :13:9: note: previous value here diff --git a/test/cases/compile_errors/switch_expression-duplicate_error_prong_when_else_present.zig b/test/cases/compile_errors/switch_expression-duplicate_error_prong_when_else_present.zig index 38ae0099b3e4a240a4bd8940e472b5d64985089f..d21e144704131ac2b6921e7cc0337c053487b96f 100644 --- a/test/cases/compile_errors/switch_expression-duplicate_error_prong_when_else_present.zig +++ b/test/cases/compile_errors/switch_expression-duplicate_error_prong_when_else_present.zig @@ -27,7 +27,7 @@ export fn entry() usize { // error // -// :8:9: error: duplicate switch value +// :8:9: error: duplicate switch value 'error.Foo' // :5:9: note: previous value here -// :17:9: error: duplicate switch value +// :17:9: error: duplicate switch value 'error.Foo' // :14:9: note: previous value here diff --git a/test/cases/compile_errors/switch_expression-duplicate_or_overlapping_integer_value.zig b/test/cases/compile_errors/switch_expression-duplicate_or_overlapping_integer_value.zig deleted file mode 100644 index d970393450d1399b985ead356f5f21a138561850..0000000000000000000000000000000000000000 --- a/test/cases/compile_errors/switch_expression-duplicate_or_overlapping_integer_value.zig +++ /dev/null @@ -1,16 +0,0 @@ -fn foo(x: u8) u8 { - return switch (x) { - 0...100 => @as(u8, 0), - 101...200 => 1, - 201, 203...207 => 2, - 206...255 => 3, - }; -} -export fn entry() usize { - return @sizeOf(@TypeOf(&foo)); -} - -// error -// -// :6:12: error: duplicate switch value -// :5:17: note: previous value here diff --git a/test/cases/compile_errors/switch_expression-duplicate_type.zig b/test/cases/compile_errors/switch_expression-duplicate_type.zig index 4b553989808cb04e52dba2b2207bced6b8f090bc..7b8f277480b71bf708eba135896f5d61f48ad1b5 100644 --- a/test/cases/compile_errors/switch_expression-duplicate_type.zig +++ b/test/cases/compile_errors/switch_expression-duplicate_type.zig @@ -13,5 +13,5 @@ export fn entry() usize { // error // -// :6:9: error: duplicate switch value +// :6:9: error: duplicate switch value 'u32' // :4:9: note: previous value here diff --git a/test/cases/compile_errors/switch_expression-duplicate_type_struct_alias.zig b/test/cases/compile_errors/switch_expression-duplicate_type_struct_alias.zig index 0b67e2107d452df6a4e19a11a3ed7d8f6c8e6f64..f2919239c82c3a68d459b686c33a35f027f36f32 100644 --- a/test/cases/compile_errors/switch_expression-duplicate_type_struct_alias.zig +++ b/test/cases/compile_errors/switch_expression-duplicate_type_struct_alias.zig @@ -17,5 +17,6 @@ export fn entry() usize { // error // -// :10:9: error: duplicate switch value +// :10:9: error: duplicate switch value 'tmp.Test' // :8:9: note: previous value here +// :1:14: note: struct declared here diff --git a/test/cases/compile_errors/switch_with_overlapping_case_ranges.zig b/test/cases/compile_errors/switch_with_overlapping_case_ranges.zig index 619875c7970ba869cb84ac43cdd1f0789724ec4f..6d7ffc0e1c9728c0c9ea5ac788263945d156c823 100644 --- a/test/cases/compile_errors/switch_with_overlapping_case_ranges.zig +++ b/test/cases/compile_errors/switch_with_overlapping_case_ranges.zig @@ -28,13 +28,43 @@ export fn entry4(x: u8) void { } } +export fn entry5(x: u8) void { + switch (x) { + 0...255 => {}, + 4...120 => {}, + } +} + +export fn entry6(x: u8) void { + switch (x) { + 0...130 => {}, + 120...255 => {}, + } +} + +export fn entry7(x: u8) void { + switch (x) { + 2 => {}, + 0...255 => {}, + } +} + // error // -// :4:10: error: duplicate switch value -// :3:10: note: previous value here -// :11:10: error: duplicate switch value -// :10:13: note: previous value here -// :17:10: error: duplicate switch value +// :4:10: error: duplicate switch ranges +// :3:10: note: overlaps with previous range here +// :3:10: note: ranges overlap from '1' to '2' +// :11:10: error: duplicate switch value '5' +// :10:13: note: previous value inside range here +// :17:10: error: duplicate switch value '5' // :18:9: note: previous value here -// :27:10: error: duplicate switch value +// :27:10: error: duplicate switch value '6' // :26:9: note: previous value here +// :34:10: error: duplicate switch ranges +// :33:10: note: overlaps with previous range here +// :33:10: note: ranges overlap from '4' to '120' +// :41:12: error: duplicate switch ranges +// :40:10: note: overlaps with previous range here +// :40:10: note: ranges overlap from '120' to '130' +// :48:10: error: duplicate switch value '2' +// :47:9: note: previous value here -- 2.54.0