Skip to content

std.enums: fix EnumSet.init and EnumMap.init for non-exhaustive enums #19309

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 4 commits into from
Mar 24, 2024

Conversation

sjb3d
Copy link
Contributor

@sjb3d sjb3d commented Mar 14, 2024

Proposed fix and EnumSet test case, fixes #19418.

@sjb3d sjb3d changed the title Fix EnumSet.init and EnumMap.init for non-exhaustive enums std.enums: fix EnumSet.init and EnumMap.init for non-exhaustive enums Mar 14, 2024
@Vexu Vexu enabled auto-merge (squash) March 24, 2024 11:23
@castholm
Copy link
Contributor

The failing test is the std_enums_big_enums standalone test which verifies that enums with thousands of fields can participate in std.enums data structures without requiring overriding the comptime branch eval quota. Now that there is an additional call to Indexer.indexOf at comptime there will be more backward branches, so you will need to comb through the @setEvalBranchQuota expressions in std.enums and increment them to the new required bare minimum as appropriate.

As a starting point I would suggest iteratively running zig build-obj test/standalone/std_enums_big_enums.zig -fno-emit-bin and modifying the @setevalBranchQuota expressions until you find the new minimum quotas.

@sjb3d
Copy link
Contributor Author

sjb3d commented Mar 24, 2024

Ah thanks for the info. I hadn't realised that indexOf is a search when the enum is not dense. It looks like this will be n^2 cost for this case, so I'll rework the PR to only use indexOf for this non-exhaustive case, which should stay under the current limits. (I think!)

auto-merge was automatically disabled March 24, 2024 16:11

Head branch was pushed to by a user without write access

@sjb3d
Copy link
Contributor Author

sjb3d commented Mar 24, 2024

Latest in branch seems to pass locally, will need approval to trigger the CI workflow though. Thanks!

@Vexu Vexu enabled auto-merge (squash) March 24, 2024 17:33
@Vexu Vexu merged commit 5c62831 into ziglang:master Mar 24, 2024
@sjb3d sjb3d deleted the enum_set_fix branch April 25, 2024 08:48
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.

Compile error when using EnumSet with non-exhaustive enum
3 participants