Skip to content

std.static_string_map: Cast length of input for runtime initialization #20350

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

Merged
merged 3 commits into from
Jun 20, 2024

Conversation

burgerindividual
Copy link
Contributor

In the init function, there seems to be a missing @intCast for the length of the input to a u32, which breaks usage of the function with a slice input. This should be possible, and the change reflects the cast in initComptime.

@travisstaloch
Copy link
Contributor

travisstaloch commented Jun 19, 2024

Thanks for catching that. Would you mind adding a small test which would otherwise fail without this patch?

EDIT:

Maybe adjust this test, casting slice to an actual slice:
https://github.com/ziglang/zig/pull/20350/files#diff-c98b1463cb9424e705583637493540a1eca3dfb294ccddb812fcb676153cc6eeR286

@nektro
Copy link
Contributor

nektro commented Jun 20, 2024

why is the len field u32 instead of usize?

@travisstaloch
Copy link
Contributor

why is the len field u32 instead of usize?

This was done as an optimization in #19719

i noticed a speedup reducing the size of the struct from 48 to 32
bytes and thus use u32s instead of usize for all length fields

@burgerindividual
Copy link
Contributor Author

I updated the test case and verified that it does not compile without the additional @intCast, and with the cast it runs successfully.

@travisstaloch
Copy link
Contributor

I updated the test case and verified that it does not compile without the additional @intCast, and with the cast it runs successfully.

Thanks! Looks good.

@Vexu Vexu enabled auto-merge (squash) June 20, 2024 20:58
@Vexu Vexu merged commit b6fd34a into ziglang:master Jun 20, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants