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

Move RawTableInner and TableLayout to separate module #472

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JustForFun88
Copy link
Contributor

All methods of the RawTableInner structure assume one important detail that is not described in the documentation: the ctrl and bucket_mask fields must remain unchanged throughout the life of the current RawTableInner instance. And this is without taking into account the specific requirements for the ctrl and bucket_mask fields.

Describing these requirements in each individual method will only lead to needlessly bloated documentation, since rustс has no way to block changing the individual fields of structure, and there is no way to check that they have not been changed.

To provide the minimum possible protection against changing the fields, we can only declare structures in a separate module, making necessary fields private.

Since the functions for getting the bucket_mask are marked as #[inline(always)] this should not affect compilation speed or performance.

But in general, this pull request just improves safety a little, fell free to close it if you don't like this idea.

@bors
Copy link
Collaborator

bors commented Oct 2, 2023

☔ The latest upstream changes (presumably #470) made this pull request unmergeable. Please resolve the merge conflicts.

@Amanieu
Copy link
Member

Amanieu commented Oct 2, 2023

I'm not really a fan of splitting up code just for the safe of safety, but in this case the raw/mod.rs file was already too large to be properly manageable, so it's good to split it up.

I am somewhat concerned about the compile-time impact of so many additional inline functions. Could you do a rustc perf run (using a PR on rust-lang/rust) to check that no compile-time regressions will occur once the standard library is updated to use the latest hashbrown?

@bors
Copy link
Collaborator

bors commented Feb 12, 2024

☔ The latest upstream changes (presumably #501) made this pull request unmergeable. Please resolve the merge conflicts.

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

3 participants