-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Use more functional programming in ArenaMap::insert #3778
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
Conversation
I find this more readable and it flattens out the body a little.
lnicola
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we need the reserve call above.
|
I was wondering that myself. I'm not sure if |
|
I think it can thanks to the size hint, but I'm not sure. |
|
Ok I think it is. |
`Take` implements `TrustedLen` so we are guaranteed that only one reserve call will be made.
|
I'm not sure what that failing macos test is about. |
|
I think it's a race, it happened in another PR or two today. |
crates/ra_arena/src/map.rs
Outdated
| } | ||
|
|
||
| let fill = (idx + 1).saturating_sub(self.v.len()); | ||
| self.v.extend(std::iter::repeat_with(|| None).take(fill)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| self.v.extend(std::iter::repeat_with(|| None).take(fill)); | |
| self.v.resize(idx + 1, None); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the imperative rain on your functional parade :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, wait, it indeed can truncate, I guess that needs to be (idx + 1).max(self.v.len())
|
bors r+ |
Build succeeded |
I find this more readable and it flattens out the body a little. Others may disagree.