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

Implement Arena::get2_mut #22

Merged
merged 6 commits into from
Feb 11, 2021
Merged

Implement Arena::get2_mut #22

merged 6 commits into from
Feb 11, 2021

Conversation

lynzrand
Copy link
Contributor

@lynzrand lynzrand commented Feb 8, 2021

This PR adds Arena::get2_mut that returns mutable reference to two items of different indices at once.

This PR closes #21.


Well... there's definitely some code gymnastics inside to implementing this thing in safe Rust. Is there a better way to implement this, or maybe an unsafe implementation could be preferred?

@LPGhatguy LPGhatguy self-requested a review February 9, 2021 16:24
src/arena.rs Outdated Show resolved Hide resolved
@lynzrand
Copy link
Contributor Author

lynzrand commented Feb 10, 2021

I just implemented the unsafe version. It passed miri tests on my machine. Implementation notes are next to the code.


A quick note here:

According to lifetime elision rules, get2_mut(&mut self, Index, Index) -> (Option<&mut T>, Option<&mut T>) has the same lifetime as get2_mut(&'a mut self, Index, Index) -> (Option<&'a mut T>, Option<&'a mut T>). This means the function exports the expected lifetimes for references.


some random meme

src/arena.rs Outdated Show resolved Hide resolved
@LPGhatguy
Copy link
Owner

Implementation is looking good, my comment is mostly a style nit.

Can you also add an entry to CHANGELOG.md that notes that Arena::get2_mut is an unreleased change? Thanks!

lynzrand and others added 2 commits February 11, 2021 09:16
Co-authored-by: Lucien Greathouse <me@lpghatguy.com>
@lynzrand
Copy link
Contributor Author

Added!

Ignore the force push please... That was just me trying to fix a style issue without wasting another commit and failed. I promise I won't do that the next time!

@LPGhatguy LPGhatguy merged commit 3e38675 into LPGhatguy:main Feb 11, 2021
@LPGhatguy
Copy link
Owner

Thanks! :D

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.

Add Arena::get2_mut(Index, Index) or similar methods
2 participants