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

Remove trait bounds on structs #1098

Merged
merged 3 commits into from
Sep 27, 2021
Merged

Remove trait bounds on structs #1098

merged 3 commits into from
Sep 27, 2021

Conversation

ethanfrey
Copy link
Member

These are not needed in the structs, but rather in the impl blocks.
The stemmed from when I was newer to Rust and didn't realise that #[derive(Clone)] was valid if some generics didn't implement Clone. I discovered this is like impl<C: Clone> Clone for Msg<C> { ... }, so we get that check for free.

When working with multi-test and more powerful use of generics and traits, these restrictions were quite troublesome. If you have a type that doesn't implement one and it is used somewhere that needs it, that will cause a compiler error. So no need to enforce that on the type.

@uint
Copy link
Contributor

uint commented Sep 15, 2021

For the record, the fact std library derives like #[derive(Clone)] require all type parameters to implement the trait is a bug.

rust-lang/rust#26925
https://www.reddit.com/r/rust/comments/517rsk/deriving_copy_for_a_struct_with_references_to/

Enforcing a trait bound on the types themselves is a valid thing to do and often useful, it just requires a manual impl block currently or to use a better derive macro from an external crate.

Edit: I'm not suggesting a different course of action, just adding info.

Copy link
Contributor

@hashedone hashedone left a comment

Choose a reason for hiding this comment

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

Love the change.

A hint for implementing traits for generics - just use derivative - it saved me hundreds lines of code while implementing Default when not all fields has their default, and also Debug when not all fields are implementing it, but it is also helpful for reducing bounds (just by #[derivative(Clone(bound = ""))]). One of my fav utility crates.

Copy link
Contributor

@uint uint left a comment

Choose a reason for hiding this comment

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

lgtm

@uint uint merged commit 160d0b3 into main Sep 27, 2021
@uint uint deleted the remove-trait-bounds-on-structs branch September 27, 2021 08:30
@ethanfrey
Copy link
Member Author

Yay! 🎉

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