Stabilize try_reserve by kornelski · Pull Request #87993 · rust-lang/rust (original) (raw)
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
TimDiekmann, quininer, Friz64, kellerkindt, paolobarbolini, Wodann, GrayJack, and Kerollmops reacted with thumbs up emoji avl, GrayJack, jdahlstrom, Kerollmops, Razican, and kellerkindt reacted with hooray emoji c410-f3r and GrayJack reacted with eyes emoji
(rust-highfive has picked a reviewer for you, use r? to override)
Marks issues that should be documented in the release notes of the next release.
Relevant to the library team, which will review and decide on the PR/issue.
labels
This comment has been minimized.
cc @rust-lang/wg-allocators
Relevant to the library API team, which will review and decide on the PR/issue.
and removed T-libs
Relevant to the library team, which will review and decide on the PR/issue.
labels
r? @joshtriplett perhaps, not sure what current T-libs FCP practice is. I don't think I see a completed one though.
FCP for stabilization:
@rfcbot merge
@dtolnay Thanks for catching that. I don't think we need that From instance.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request
🔔 This is now entering its final comment period, as per the review above. 🔔
Manishearth added a commit to Manishearth/rust that referenced this pull request
📌 Commit 00152d8 has been approved by joshtriplett
🌲 The tree is currently closed for pull requests below priority 4. This pull request will be tested once the tree is reopened.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
Manishearth added a commit to Manishearth/rust that referenced this pull request
Manishearth added a commit to Manishearth/rust that referenced this pull request
Manishearth added a commit to Manishearth/rust that referenced this pull request
bors added a commit to rust-lang-ci/rust that referenced this pull request
95 tasks
Why wasn't the actual error kind stabilized? There's no point to this feature without the ability to introspect.
@LeonineKing1199 You can see in the original RFC discussion that it wasn't certain if the error should be stabilized exactly as it is implemented currently, with separate cases that just mirror an inconsistency in std (where some allocation failures panic, and some abort). Waiting for resolution of that question was delaying stabilization of the entire feature.
I'm interested to hear what is your use-case and what data you'd like to get out of the error type, to help guide how the error type should be designed.
Ha ha, looks like my case was already addressed:
One I can think of is someone creating a custom data structure, and wanting to mimic std API. But that's not a very strong case.
It is for implementing the stdlib APIs without polluting it with polyfills that are incompatible with what's in the stdlib.
Great. So you're not looking to get any data out of it yourself, you just want to return a TryReserveError instance? If there was TryReserveError::default() or just a very basic TryReserveError::new(allocation_size), would that work for you?
For now a hacky way to get some instance is Vec::<u8>::new().try_reserve(usize::MAX).unwrap_err(). That's guaranteed to always fail fast.
Why wasn't the actual error kind stabilized? There's no point to this feature without the ability to introspect.
In my opinion, the error is more for the end user, or very special case for something like kernel linux. For a server I will log the error and continue anyway so don't really care, even if I would consider a CapacityOverflow a code bug in a server or at least get a look at it.
@kornelski Do you think we could have const value TryReserveError::CAPACITY_OVERFLOW ? Unless if we want to be able to precise the maximum value at runtine and possibly the value used I don't expect this to change a lot.
Great. So you're not looking to get any data out of it yourself, you just want to return a
TryReserveErrorinstance? If there wasTryReserveError::default()or just a very basicTryReserveError::new(allocation_size), would that work for you?
Actually, this would work perfectly!
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…, r=yaahc
add BinaryHeap::try_reserve and BinaryHeap::try_reserve_exact
try_reserve of many collections were stablized in rust-lang#87993 in 1.57.0. Add try_reserve for the rest collections such as BinaryHeap should be not controversial.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…, r=yaahc
add BinaryHeap::try_reserve and BinaryHeap::try_reserve_exact
try_reserve of many collections were stablized in rust-lang#87993 in 1.57.0. Add try_reserve for the rest collections such as BinaryHeap should be not controversial.
Labels
This issue / PR is in PFCP or FCP with a disposition to merge it.
The final comment period is finished for this PR / Issue.
Marks issues that should be documented in the release notes of the next release.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the library API team, which will review and decide on the PR/issue.