-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
deprecate Eq::assert_receiver_is_total_eq and emit FCW on manual impls
#149978
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
base: main
Are you sure you want to change the base?
Conversation
| Warn, | ||
| "manual implementation of the internal `Eq::assert_receiver_is_total_eq` method", | ||
| @future_incompatible = FutureIncompatibleInfo { | ||
| reason: fcw!(FutureReleaseError #150000), |
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.
TODO: create a real tracking issue once there is consensus on this change
This comment has been minimized.
This comment has been minimized.
faf29de to
81dc07d
Compare
Eq::assert_receiver_is_total_eq and emit a FCW on manual …Eq::assert_receiver_is_total_eq and emit FCW on manual impls
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.
Implementation looks good, feel free to r=me on that once you have t-libs signoff (FCP?).
Procedurally, this might also need t-lang approval for the lint name IIUC? Or can t-libs decide that?
The
Eq::assert_receiver_is_total_eqmethod is purely meant as an implementation detail by#[derive(Eq)]to add checks that all fields of the type the derive is applied to also implementEq.The method is already
#[doc(hidden)]and has a comment saying// This should never be implemented by hand..Unfortunately, it has been stable since 1.0 and there are some cases on GitHub (https://github.com/search?q=assert_receiver_is_total_eq&type=code) where people have implemented this method manually, sometimes even with actual code in the method body (example: https://github.com/Shresht7/codecrafters-redis-rust/blob/31f0ec453c504b4ab053a7b1c3ff548ff36a9db5/src/parser/resp/types.rs#L255).
To prevent further confusion from this, this PR is deprecating the method and adds a FCW when it is manually implemented (this is necessary as the deprecation warning is not emitted when the method is implemented, only when it is called).
This is similar to what was previously done with the
soft_unstablelint (#64266).See also rust-lang/libs-team#704.