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

Add support for try_reserve under unstable #17

Merged
merged 1 commit into from
Nov 24, 2018
Merged

Add support for try_reserve under unstable #17

merged 1 commit into from
Nov 24, 2018

Conversation

WildCryptoFox
Copy link

@WildCryptoFox WildCryptoFox commented Nov 23, 2018

This will need to be tweaked from std::collections::CollectionAllocErr to crate::alloc::collections::CollectionAllocErr if #18 is accepted.

This is the only conflict between my 4 split PRs.

src/normal.rs Outdated
/// # use slotmap::*;
/// let mut sm = SlotMap::new();
/// sm.insert("foo");
/// sm.try_reserve(32);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unwrap()?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. Yes that should have an unwrap. Nice catch.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I see there is no try_set_capacity?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Force push.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orlp Why do you expect a try_set_capacity? try_reserve is trying to set the capacity...

Copy link
Owner

@orlp orlp Nov 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not reserve because reserve adds capacity, while we want to set the capacity. I don't see the force push though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orlp Github does.

@james-darkfox james-darkfox force-pushed the james-darkfox:try-reserve branch from 4edb5c2 to ce06cbc 36 minutes ago

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@james-darkfox Ctrl+F try_set_capacity in ce06cbc gives no results.

Copy link
Author

@WildCryptoFox WildCryptoFox Nov 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops. Wrong one. Fixed. Sorry.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orlp bump (github notifications kinda suck...)

@orlp orlp merged commit 58d6e54 into orlp:master Nov 24, 2018
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