-
Notifications
You must be signed in to change notification settings - Fork 745
Derive from any other trait only when deriving from Copy #2200
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
Conversation
c8bc986
to
84a2ec0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should do this only when the struct is packed, or what am I missing? Otherwise it might break existing code gratuitously.
5ba9bd2
to
dea052e
Compare
@emilio Good point, it should be fixed now. |
dea052e
to
606cf91
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good! I think we can simplify it slightly (given we always treat packed = None
as false
, but let me know if I'm missing something.
Looks good to me with that.
606cf91
to
3a7ba55
Compare
It's impossible to #[derive] from any other trait when not deriving from Copy when using the newest Rust nightly. Any attempt to do that results in the following error: error: `#[derive]` can't be used on a `#[repr(packed)]` struct that does not derive Copy (error E0133) Fixes: rust-lang#2083 Signed-off-by: Michal Rostecki <[email protected]>
3a7ba55
to
e8805de
Compare
Thank you for review! I like the idea with using just bool without Option, I applied it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks good for now. I think this might not be fully correct in the presence of a non-packed struct containing a packed struct... For that we probably need to track this properly in the CanDerive
analysis.
That shouldn't be too hard, but doing this in the meantime is fine.
@emilio Good call, I will try to address that in a separate PR |
It's impossible to #[derive] from any other trait when not deriving from
Copy when using the newest Rust nightly. Any attempt to do that results
in the following error:
Fixes: #2083
Signed-off-by: Michal Rostecki [email protected]