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 EcoVec generic over choice of reference count type #32

Closed
wants to merge 4 commits into from

Conversation

Kmeakin
Copy link
Contributor

@Kmeakin Kmeakin commented Sep 9, 2023

This patch adds a reference count type parameter to EcoVec<T>, allowing it to be used with non-atomic reference counts if the vec is only being used on a single thread.

I left EcoString non-generic over refence counts, but that could also be made generic in a future patch.

This patch adds a reference count type parameter to `EcoVec<T>`, allowing it to
be used with non-atomic reference counts if the vec is only being used on a
single thread.

I left `EcoString` non-generic over refence counts, but that could also be made
generic in a future patch.
@codecov-commenter
Copy link

codecov-commenter commented Sep 9, 2023

Codecov Report

Patch coverage is 56.60% of modified lines.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Files Changed Coverage
src/refcount.rs 46.87%
src/vec.rs 62.50%
src/dynamic.rs 100.00%

📢 Thoughts on this report? Let us know!.

@laurmaedje
Copy link
Member

This idea has come up before and my thoughts are quite well expressed by this comment on a similar PR to smol_str: rust-analyzer/smol_str#37 (comment)

@laurmaedje
Copy link
Member

See also discussion on the reddit post: https://www.reddit.com/r/rust/comments/117ksvr/comment/j9ek5t3/

@Kmeakin
Copy link
Contributor Author

Kmeakin commented Sep 10, 2023

This idea has come up before and my thoughts are quite well expressed by this comment on a similar PR to smol_str: rust-analyzer/smol_str#37 (comment)

That's a shame but I understand the design decision

@Kmeakin Kmeakin closed this Sep 10, 2023
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.

3 participants