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

builtin: panic on trying to grow arrays with capacity bigger than 2^31, instead of overflowing a.cap (partial fix for #21918) #21947

Conversation

spytheman
Copy link
Member

@spytheman spytheman commented Jul 27, 2024

This PR limits dynamic array capacity growth to max_int , and makes sure that a panic happens if that needs to be exceeded, instead of silently overflowing the .cap field (thus breaking the invariant about .len <= .cap).

It is a temporary workaround for #21918 , providing a better experience (allows for growing/processing till the actual limit, and a panic with a stacktrace, when the limit is reached, for improved debugging), until V transitions to using 64 bit numbers for the .len and .cap array fields.

@spytheman
Copy link
Member Author

The performance impact for the V compiler is ~0.9%-1.6% slowdown, when tcc is used :-| .
With -prod -cc clang-18, there is no performance impact.

image

I still think, that the more extensive checks are worth it, since with them, V developers will diagnose problems earlier, with clearer diagnostic messages.

@medvednikov
Copy link
Member

I'm making int 64 bit right now, fixing tests.

@spytheman spytheman merged commit 6f20516 into vlang:master Jul 28, 2024
73 checks passed
@spytheman spytheman deleted the builtin_panic_on_array_cap_overflow_when_growing branch July 30, 2024 00:42
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.

2 participants