-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Small cleanups in HashMap based off of new rust features. #19935
Conversation
Full(bucket) => bucket | ||
}; | ||
|
||
if bucket.hash() == hash { | ||
let found_match = { |
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.
minor concern that we're losing some semantics here
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.
how so?
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.
In isolation, we've replaced the helpful name found_match
with some more opaque if statement.
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 can add 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 love comments! \o/
r=me with squash |
4bb2a5b
to
c42e2f6
Compare
Small cleanups in HashMap based off of new rust features. Reviewed-by: Gankro
Using a type alias for iterator implementations is fragile since this exposes the implementation to users of the iterator, and any changes could break existing code. This commit changes the iterators of `VecMap` to use proper new types, rather than type aliases. However, since it is fair-game to treat a type-alias as the aliased type, this is a: [breaking-change].
r? @gankro @pczarn