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 SecondaryMap for HashSet #62

Merged
merged 2 commits into from
Jun 6, 2023
Merged

Conversation

aborgna-q
Copy link
Collaborator

@aborgna-q aborgna-q commented Jun 6, 2023

Extracted from #60. Partially implements #55.
The HashMap is a lot more tricky due to having to return references to a generic default value.

@aborgna-q aborgna-q requested a review from ss2165 June 6, 2023 15:24
Comment on lines +284 to +288
if HashSet::contains(self, &key) {
&true
} else {
&false
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if HashSet::contains(self, &key) {
&true
} else {
&false
}
&HashSet::contains(self, &key)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Annoyingly we cannot do that, because the return value of HashSet::contains(self, &key) is a local variable, where true and false can be statically defined :/

Copy link
Member

Choose a reason for hiding this comment

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

that is quite funny

src/secondary.rs Outdated
}

#[inline]
fn resize(&mut self, _new_len: usize) {}
Copy link
Member

Choose a reason for hiding this comment

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

not sure I follow why this is empty

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The method was meant to truncate the linear vecs in Unmanaged map and BitVec, but that doesn't make sense on sparse unordered containers.

It should probably be removed from the trait (it's never used).

@aborgna-q aborgna-q merged commit 53a3285 into main Jun 6, 2023
6 checks passed
@aborgna-q aborgna-q deleted the feature/hashset-secondarymap branch June 6, 2023 15:50
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.

None yet

2 participants