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

More accurate Set and Map size warnings #1010

Open
meooow25 opened this issue Jun 8, 2024 · 3 comments
Open

More accurate Set and Map size warnings #1010

meooow25 opened this issue Jun 8, 2024 · 3 comments

Comments

@meooow25
Copy link
Contributor

meooow25 commented Jun 8, 2024

Set and Map documentation carry warnings that say

The size of the set must not exceed maxBound::Int. Violation of this condition is not detected and if the size limit is exceeded, its behaviour is undefined.

However, we multiple sizes by delta = 3 without caring about overflow when balancing, which means we get violated invariants earlier than that.

I doubt anyone will be running into this in practice, but the limit should be document as maxBound `div` 3 perhaps. Or less, in case there are other potentially troublesome operations I missed.

@treeowl
Copy link
Contributor

treeowl commented Jun 8, 2024

Good point. Is it possible to avoid the problem by being careful about how we do the comparisons? It's not an issue for 64 bit, but we still (nominally) support 32 bit, and (theoretically) 29 bit.

@meooow25
Copy link
Contributor Author

meooow25 commented Jun 9, 2024

Is it possible to avoid the problem by being careful about how we do the comparisons?

Well we could replace a * 3 > b with a > b `quot` 3. This would likely be a slowdown, benchmarks can tell.

@treeowl
Copy link
Contributor

treeowl commented Jun 9, 2024

Hmph. That does sound like a slowdown. Checking for overflow would probably be a slowdown too (more work for branch prediction), but what do I know? Changing the documentation is at least easy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants