From 8c3b99a0bbc23fd48a85d445088108eb9d6e7dcf Mon Sep 17 00:00:00 2001 From: Justus Klausecker Date: Mon, 15 Jun 2026 01:27:09 +0200 Subject: [PATCH] Sema: make switch prong item duplicate validation go faster `RangeSet` used to do a linear search for any overlapping ranges on every insert, leading to O(n^2) comparisons. This had the 'advantage' that the entire set only needs to be sorted once, when checking whether the whole value range of a given type has been covered. It now instead keeps itself sorted at all times which means that we can use binary search and only perform O(n log n) comparisons. This turns out to be way faster for large amounts of switch prong items. --- src/RangeSet.zig | 75 +++++++++++-------- src/Sema.zig | 10 +-- .../switch_with_overlapping_case_ranges.zig | 38 ++++++++-- 3 files changed, 82 insertions(+), 41 deletions(-) diff --git a/src/RangeSet.zig b/src/RangeSet.zig index c42f2d40123d49d4e040f33d440d763526549ed0..3033e8510394cbef68b815601f4eea545fb4feae 100644 --- a/src/RangeSet.zig +++ b/src/RangeSet.zig @@ -1,6 +1,6 @@ const RangeSet = @This(); -ranges: std.ArrayList(Range), +ranges: std.MultiArrayList(Range), pub const Range = struct { first: Value, @@ -22,15 +22,19 @@ pub fn ensureUnusedCapacity(self: *RangeSet, allocator: Allocator, additional_co pub fn addAssumeCapacity(set: *RangeSet, new: Range, ty: Type, zcu: *Zcu) ?LazySrcLoc { assert(new.first.typeOf(zcu).eql(ty)); assert(new.last.typeOf(zcu).eql(ty)); + assert(new.first.compareScalar(.lte, new.last, ty, zcu)); - for (set.ranges.items) |range| { - if (new.last.compareScalar(.gte, range.first, ty, zcu) and - new.first.compareScalar(.lte, range.last, ty, zcu)) - { - return range.src; // They overlap. - } + const idx = std.sort.lowerBound(Value, set.ranges.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)) + { + return set.ranges.items(.src)[idx]; // `new` overlaps with existing range. } - set.ranges.appendAssumeCapacity(new); + set.ranges.insertAssumeCapacity(idx, new); return null; } @@ -39,15 +43,6 @@ pub fn add(set: *RangeSet, allocator: Allocator, new: Range, ty: Type, zcu: *Zcu return set.addAssumeCapacity(new, ty, zcu); } -const SortCtx = struct { - ty: Type, - zcu: *Zcu, -}; -/// Assumes a and b do not overlap -fn lessThan(ctx: SortCtx, a: Range, b: Range) bool { - return a.first.compareScalar(.lt, b.first, ctx.ty, ctx.zcu); -} - pub fn spans( set: *RangeSet, allocator: Allocator, @@ -58,35 +53,36 @@ pub fn spans( ) Allocator.Error!bool { assert(first.typeOf(zcu).eql(ty)); assert(last.typeOf(zcu).eql(ty)); - if (set.ranges.items.len == 0) return false; + if (set.ranges.len == 0) return false; - std.mem.sort(Range, set.ranges.items, SortCtx{ .ty = ty, .zcu = zcu }, lessThan); + 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)); - if (!set.ranges.items[0].first.eql(first, ty, zcu) or - !set.ranges.items[set.ranges.items.len - 1].last.eql(last, ty, zcu)) + if (!set.ranges.items(.first)[0].eql(first, ty, zcu) or + !set.ranges.items(.last)[set.ranges.len - 1].eql(last, ty, zcu)) { return false; } const limbs = try allocator.alloc( - std.math.big.Limb, - std.math.big.int.calcTwosCompLimbCount(ty.intInfo(zcu).bits), + math.big.Limb, + math.big.int.calcTwosCompLimbCount(ty.intInfo(zcu).bits), ); defer allocator.free(limbs); - var counter: std.math.big.int.Mutable = .init(limbs, 0); + var counter: math.big.int.Mutable = .init(limbs, 0); var space: InternPool.Key.Int.Storage.BigIntSpace = undefined; // look for gaps - for (set.ranges.items[1..], 0..) |cur, i| { - // i starts counting from the second item. - const prev = set.ranges.items[i]; - - // prev.last + 1 == cur.first - counter.copy(prev.last.toBigInt(&space, zcu)); + for ( + set.ranges.items(.first)[1..], + set.ranges.items(.last)[0 .. set.ranges.len - 1], + ) |cur_first, prev_last| { + // prev_last + 1 == cur_first + counter.copy(prev_last.toBigInt(&space, zcu)); counter.addScalar(counter.toConst(), 1); - const cur_start_int = cur.first.toBigInt(&space, zcu); + const cur_start_int = cur_first.toBigInt(&space, zcu); if (!cur_start_int.eql(counter.toConst())) { return false; } @@ -95,7 +91,24 @@ pub fn spans( return true; } +const SearchCtx = struct { + val: Value, + zcu: *const Zcu, +}; +fn compare(ctx: SearchCtx, other: Value) math.Order { + return ctx.val.order(other, ctx.zcu); +} + +const SortCtx = struct { + ty: Type, + zcu: *Zcu, +}; +fn lessThan(ctx: SortCtx, a: Value, b: Value) bool { + return a.compareScalar(.lt, b, ctx.ty, ctx.zcu); +} + const std = @import("std"); +const math = std.math; const assert = std.debug.assert; const Allocator = std.mem.Allocator; diff --git a/src/Sema.zig b/src/Sema.zig index 136abcc8981345f3066aee9bd1c471dc217452b8..23fc4a11cbf04a47a6d115a39c9517f0dc1008fc 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -10866,7 +10866,7 @@ fn finishSwitchBr( const ValidatedSwitchBlock = struct { seen_enum_fields: []const ?LazySrcLoc, seen_errors: std.AutoHashMapUnmanaged(InternPool.NullTerminatedString, LazySrcLoc), - seen_ranges: []const RangeSet.Range, + seen_ranges: std.MultiArrayList(RangeSet.Range).Slice, true_src: ?LazySrcLoc, false_src: ?LazySrcLoc, void_src: ?LazySrcLoc, @@ -10901,7 +10901,7 @@ const ValidatedSwitchBlock = struct { error_names: InternPool.NullTerminatedString.Slice, seen_enum_fields: []const ?LazySrcLoc, seen_errors: *const std.AutoHashMapUnmanaged(InternPool.NullTerminatedString, LazySrcLoc), - seen_ranges: []const RangeSet.Range, + seen_ranges: std.MultiArrayList(RangeSet.Range).Slice, seen_true: bool, seen_false: bool, seen_void: bool, @@ -10938,13 +10938,13 @@ const ValidatedSwitchBlock = struct { else => unreachable, }; while (it.next_idx < it.seen_ranges.len and - cur_val.eql(it.seen_ranges[it.next_idx].first, int_ty, zcu)) + cur_val.eql(it.seen_ranges.items(.first)[it.next_idx], int_ty, zcu)) { defer it.next_idx += 1; const incr = try arith.incrementDefinedInt( sema, int_ty, - it.seen_ranges[it.next_idx].last, + it.seen_ranges.items(.last)[it.next_idx], ); if (incr.overflow) { it.next_val = null; @@ -11432,7 +11432,7 @@ fn validateSwitchBlock( return .{ .seen_enum_fields = seen_enum_fields, .seen_errors = seen_errors, - .seen_ranges = range_set.ranges.items, + .seen_ranges = range_set.ranges.slice(), .true_src = true_src, .false_src = false_src, .void_src = void_src, 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 5b272e5c84a8d9295697add83d0627a7b22c5fc9..619875c7970ba869cb84ac43cdd1f0789724ec4f 100644 --- a/test/cases/compile_errors/switch_with_overlapping_case_ranges.zig +++ b/test/cases/compile_errors/switch_with_overlapping_case_ranges.zig @@ -1,12 +1,40 @@ -export fn entry() void { - var q: u8 = 0; - switch ((&q).*) { +export fn entry1(x: u8) void { + switch (x) { 1...2 => {}, 0...255 => {}, } } +export fn entry2(x: i8) void { + switch (x) { + -128...5 => {}, + 5...127 => {}, + } +} + +export fn entry3(x: u8) void { + switch (x) { + 0...5 => {}, + 5 => {}, + 6...255 => {}, + } +} + +export fn entry4(x: u8) void { + switch (x) { + 0...5 => {}, + 6 => {}, + 6...255 => {}, + } +} + // error // -// :5:10: error: duplicate switch value -// :4:10: note: previous value here +// :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 +// :18:9: note: previous value here +// :27:10: error: duplicate switch value +// :26:9: note: previous value here -- 2.54.0