Skip to content

Commit 313848c

Browse files
committed
Fix a bug where single-item StaticStringMaps can match strings where the expected key is a starting subset of the input key. Add a test. Implement #20055 - use the fact we have already grouped the keys by length to reduce the number of comparisons, and use the eql function to compile-error on redundant entries, determined by whether the eql function believes they are identical. Remove tests which now compile-error.
1 parent baad55a commit 313848c

File tree

1 file changed

+39
-46
lines changed

1 file changed

+39
-46
lines changed

lib/std/static_array_map.zig

Lines changed: 39 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -129,14 +129,10 @@ pub fn StaticArrayMapWithEql(
129129

130130
/// Returns the value for the key if any, else null.
131131
pub fn get(comptime self: Self, key: []const T) ?V {
132-
return switch (self.kvs.len) {
133-
0 => null,
134-
1 => blk: {
135-
const equal = std.mem.eql(T, self.kvs[0].key, key);
136-
break :blk if (equal) self.kvs[0].value else null;
137-
},
138-
else => self.filterLength(key),
139-
};
132+
switch (comptime self.kvs.len) {
133+
0 => return null,
134+
else => return self.filterLength(key),
135+
}
140136
}
141137

142138
/// The list of all the keys in the map.
@@ -166,13 +162,30 @@ pub fn StaticArrayMapWithEql(
166162
/// Filters the input key by length, then compares it to the possible matches.
167163
/// Because we know the length at comptime, we can compare the strings faster.
168164
fn filterLength(comptime self: Self, key: []const T) ?V {
169-
// Provide 2000 branches per key/value pair to compile.
165+
// separateLength is hungry - provide 2000 branches per key/value pair
170166
@setEvalBranchQuota(2000 * self.kvs.len);
171167
const kvs_by_lengths = comptime self.separateLength();
168+
172169
inline for (kvs_by_lengths) |kvs_by_len| {
173170
const len = kvs_by_len.length;
174171
if (key.len == len) {
175-
inline for (kvs_by_len.kvs) |kv| {
172+
inline for (kvs_by_len.kvs, 0..) |kv, idx| {
173+
174+
// Out of keys with the same length, check for duplicates
175+
@setEvalBranchQuota(10 * len * idx);
176+
comptime for (kvs_by_len.kvs[0..idx]) |prev_kv| {
177+
if (eql(kv.key[0..len].*, prev_kv.key[0..len].*)) {
178+
if (T == u8 and std.unicode.utf8ValidateSlice(kv.key)) {
179+
@compileError("duplicate key \"" ++ kv.key ++ "\"");
180+
} else {
181+
@compileError(std.fmt.comptimePrint(
182+
"duplicate key: {any}",
183+
.{kv.key},
184+
));
185+
}
186+
}
187+
};
188+
176189
if (eql(kv.key[0..len].*, key[0..len].*)) {
177190
return kv.value;
178191
}
@@ -200,6 +213,7 @@ pub fn StaticArrayMapWithEql(
200213
}
201214
}
202215

216+
// Add keys with the same length to the set.
203217
var added_kvs: []const Kv = &.{};
204218
for (self.kvs[index..]) |add_kv| {
205219
if (add_kv.key.len == length) {
@@ -361,42 +375,6 @@ test "empty" {
361375
try testing.expect(null == m2.get("anything"));
362376
}
363377

364-
test "redundant entries" {
365-
const slice = [_]TestKV{
366-
.{ "redundant", .D },
367-
.{ "theNeedle", .A },
368-
.{ "redundant", .B },
369-
.{ "re" ++ "dundant", .C },
370-
.{ "redun" ++ "dant", .E },
371-
};
372-
const map = TestMap.initComptime(slice);
373-
374-
// No promises about which one you get:
375-
try testing.expect(null != map.get("redundant"));
376-
377-
// Default map is not case sensitive:
378-
try testing.expect(null == map.get("REDUNDANT"));
379-
380-
try testing.expectEqual(TestEnum.A, map.get("theNeedle").?);
381-
}
382-
383-
test "redundant insensitive" {
384-
const slice = [_]TestKV{
385-
.{ "redundant", .D },
386-
.{ "theNeedle", .A },
387-
.{ "redundanT", .B },
388-
.{ "RE" ++ "dundant", .C },
389-
.{ "redun" ++ "DANT", .E },
390-
};
391-
392-
const map = TestMapIgnoreCase.initComptime(slice);
393-
394-
// No promises about which result you'll get ...
395-
try testing.expect(null != map.get("REDUNDANT"));
396-
try testing.expect(null != map.get("ReDuNdAnT"));
397-
try testing.expectEqual(TestEnum.A, map.get("theNeedle").?);
398-
}
399-
400378
test "comptime-only value" {
401379
const map = StaticStringMap(type).initComptime(.{
402380
.{ "a", struct {
@@ -482,3 +460,18 @@ test "array elements that are padded" {
482460
try testing.expectEqual(1, map.get(&.{ 0, 1, 127, 3, 4, 5 }));
483461
try testing.expectEqual(2, map.get(&.{ 0, 1, 2, 126, 4, 5 }));
484462
}
463+
464+
test "single string StaticStringMap" {
465+
const map = StaticStringMap(void).initComptime(.{.{"o kama pona"}});
466+
467+
try testing.expectEqual(true, map.has("o kama pona"));
468+
try testing.expectEqual(false, map.has("o kama ike"));
469+
try testing.expectEqual(false, map.has("o kama pona ala"));
470+
}
471+
472+
test "empty StaticStringMap" {
473+
const map = StaticStringMap(void).initComptime(.{});
474+
try testing.expectEqual(false, map.has(&.{}));
475+
try testing.expectEqual(null, map.get(&.{}));
476+
try testing.expectEqual(false, map.has("anything really"));
477+
}

0 commit comments

Comments
 (0)