Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std.ComptimeStringMap gives compile error when giving an empty list #18212

Closed
FanShupei opened this issue Dec 6, 2023 · 3 comments · Fixed by #18234
Closed

std.ComptimeStringMap gives compile error when giving an empty list #18212

FanShupei opened this issue Dec 6, 2023 · 3 comments · Fixed by #18234
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@FanShupei
Copy link
Contributor

Zig Version

0.12.0-dev.1800+559e216f3

Steps to Reproduce and Observed Behavior

When compiling the following code,

const std = @import("std");
pub fn main() void {
    const dict = std.ComptimeStringMap(i32, .{});
    // const dict = std.ComptimeStringMap(i32, .{.{"nothing", 0}});
    std.debug.print("{?}\n", .{dict.get("anything")});
}

it gives the following compile error:

/home/fsp/.zvm/master/lib/std/comptime_string_map.zig:74:35: error: indexing into empty array is not allowed
        const min_len = sorted_kvs[0].key.len;
                        ~~~~~~~~~~^~~
/home/fsp/.zvm/master/lib/std/comptime_string_map.zig:14:36: note: called from here
    return ComptimeStringMapWithEql(V, kvs_list, defaultEql);
           ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
1.zig:3:39: note: called from here
    const dict = std.ComptimeStringMap(i32, .{});
                 ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
referenced by:
    callMain: /home/fsp/.zvm/master/lib/std/start.zig:575:17
    initEventLoopAndCallMain: /home/fsp/.zvm/master/lib/std/start.zig:519:34
    remaining reference traces hidden; use '-freference-trace' to see all reference traces

I believe the root cause is that ComptimeStringMapWithEql does not handle the special case that the list is empty.

Expected Behavior

When providing an empty list, std.ComptimeStringMap shall behave as an empty dict, instead of generating compile error.

@FanShupei FanShupei added the bug Observed behavior contradicts documented or intended behavior label Dec 6, 2023
@Rexicon226
Copy link
Contributor

what would be the purpose of an empty comptime string map?
if anything, it should be a compile error to not have anything in the tuple.

@FanShupei
Copy link
Contributor Author

I'm not thoughtful enough to see the expected behavior itself is controversial, but I insist the current behavior extremely confuses users. The expected behavior may be changed to the following:

Expected Behavior: If an empty list should be supported, it shall be behave as an empty dict. If it's intentionally disallowed, it should give a clear error message and be reflected in documentation.

Personally I think an empty string map should be allowed, for consistency, like Zig already has zero-length tuple and zero-length array. The behavior is intuitive and I haven't see any footgun for it.

I agree that deliberately construct an empty comptime map is quite meaningless. But the feature may be useful in metaproramming.

@xdBronch
Copy link
Contributor

xdBronch commented Dec 7, 2023

theres actually an unrelated PR open i stumbled across that fixes this #17731. you can see here that it defaults to 0 and will just always return null from the get function if the tuple is empty

@Vexu Vexu added the standard library This issue involves writing Zig code for the standard library. label Dec 7, 2023
@Vexu Vexu added this to the 0.12.0 milestone Dec 7, 2023
@Vexu Vexu linked a pull request Dec 7, 2023 that will close this issue
rootbeer added a commit to rootbeer/zig that referenced this issue Dec 9, 2023
Add tests for empty initialization, and some more corner cases (empty key,
very long key, duplicate keys).

Fixes ziglang#18212
rootbeer added a commit to rootbeer/zig that referenced this issue Dec 11, 2023
Add tests for empty initialization, and some more corner cases (empty key,
very long key, duplicate keys).

Fixes ziglang#18212
rootbeer added a commit to rootbeer/zig that referenced this issue Dec 26, 2023
Add tests for empty initialization, and some more corner cases (empty key,
very long key, duplicate keys).

Fixes ziglang#18212
rootbeer added a commit to rootbeer/zig that referenced this issue Jan 2, 2024
Add tests for empty initialization, and some more corner cases (empty key,
very long key, duplicate keys).

Fixes ziglang#18212
rootbeer added a commit to rootbeer/zig that referenced this issue Jan 3, 2024
Add tests for empty initialization, and some more corner cases (empty key,
very long key, duplicate keys).

Fixes ziglang#18212
rootbeer added a commit to rootbeer/zig that referenced this issue Jan 4, 2024
Add tests for empty initialization, and some more corner cases (empty key,
very long key, duplicate keys).

Fixes ziglang#18212
Vexu pushed a commit that referenced this issue Jan 5, 2024
Add tests for empty initialization, and some more corner cases (empty key,
very long key, duplicate keys).

Fixes #18212
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.
Projects
None yet
4 participants