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

Make allocator not Clone #468

Merged
merged 2 commits into from Sep 5, 2023
Merged

Conversation

JustForFun88
Copy link
Contributor

@JustForFun88 JustForFun88 commented Sep 1, 2023

@Amanieu Implements the idea proposed in #465 (comment). The A: Allocator + Clone requirement remains only for Clone implementations.

The pull request is basically complete, just a final review (and maybe some tweaks if needed). Also, I haven't documented the functions yet. This can be done after the initial approval of this pull request.

Main changes made:

  1. Allacator moved from RawTableInner to RawTable;
  2. IS_ZERO_SIZED_TYPE and DATA_NEEDS_DROP constants moved from separate structures to SizedTypeProperties trait;
  3. For ease of implementation and avoidance of duplication, some functions are downgraded from RawTable to RawTableInner. These are: with_capacity, iter, drop_elements. Additionally, the drop_inner_table function has been created through which impl Drop for RawTable<T, A> is now implemented.

src/raw/mod.rs Outdated Show resolved Hide resolved
src/raw/mod.rs Outdated Show resolved Hide resolved
@JustForFun88
Copy link
Contributor Author

JustForFun88 commented Sep 1, 2023

If you're happy with the implementation, I can start documenting the changes I've made.

Upd 1: I'm working on documenting functions. I'll probably finish tomorrow.

@Amanieu I have made all you requests and document my all changes. I also made additional changes compared to the previous review: using const RawTableInner::NEW instead of RawTableInner::new();

In addition, I made the reserve_rehash function unsafe, since it was incorrectly labeled as safe from the start.

Could you please review the PR again?

@Amanieu
Copy link
Member

Amanieu commented Sep 5, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 5, 2023

📌 Commit 91d3675 has been approved by Amanieu

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Sep 5, 2023

⌛ Testing commit 91d3675 with merge fe81cad...

@bors
Copy link
Collaborator

bors commented Sep 5, 2023

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing fe81cad to master...

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