Better handle increasing vector size near usize::MAX #23849
Conversation
for only half of the maximum size available on the architecture. This allows vectors to keep expanding with those two methods until the amount of bytes exceeds usize.
try to get capacity for usize::MAX
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pcwalton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see CONTRIBUTING.md for more information. |
|
I believe the largest allocation we fundamentally support is |
|
Are you sure? In the current version of the source https://github.com/rust-lang/rust/blob/master/src/libcollections/vec.rs there's a lot of code that acts like usize::MAX is the max allowed, eg lines 209, 309, 337, 643, 655, 716, 1232. If isize::MAX is the max allowed allocation then there are a lot of changes needed. Though these other lines could be handled in a different PR. (Also doesn't restricting to isize::MAX seem a bit limiting on 32 bit and smaller architectures?) |
|
I guess |
|
Gotcha, sounds good. I'll fix these instances up tomorrow, would you like me to include the other parts of the file I found above in this PR as well, or submit a different one? |
|
Made these locations max out as isize::MAX. I added a fixme comment to fix the rest of the file to also assume isize::MAX is the max size, I'll fix those as well after the PR. |
|
r? @pnkfelix |
|
@bors r+ |
|
📌 Commit f8493d0 has been approved by |
|
⌛ Testing commit f8493d0 with merge f156c61... |
|
💔 Test failed - auto-linux-32-opt |
|
That compile failure is ... worrisome. I wonder if a bug has been injected very recently |
|
Or maybe I misunderstood a recent discussion of how general unary |
|
(The workaround should be easy, in any case,.. use |
|
(or wait, maybe this is just an issue where the |
|
Yeah it compiled and the tests passed... Not sure what the problem is. |
|
I just checked again on my local version, whose most recent commit is commit fa3d778 and it compiles with no problem. So it does appear to be a recent bug. |
|
Ah nvm. The problem wasn't introduced. I had compiled/tested with make check-stage1-collections NO_REBUILD=1 NO_BENCH=1 However when I just do a full make, it fails on stage0. So it looks like this is a case of the older version of the compiler which builds stage0 not supporting it. I'll push a fix shortly. |
with "let" when building on stage0. So change the error message to a static const.
Right now, if the user requests to increase the vector size via reserve() or push_back() and the request brings the attempted memory above usize::MAX, we panic. With this change there is only a panic if the minimum requested memory that could meet the requirement is above usize::MAX- otherwise it simply requests its largest capacity possible, usize::MAX.
Right now, if the user requests to increase the vector size via reserve() or push_back() and the request brings the attempted memory above usize::MAX, we panic.
With this change there is only a panic if the minimum requested memory that could meet the requirement is above usize::MAX- otherwise it simply requests its largest capacity possible, usize::MAX.