Skip to content

Conversation

@cyrgani
Copy link
Contributor

@cyrgani cyrgani commented Dec 14, 2025

The Eq::assert_receiver_is_total_eq method 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 implement Eq.
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_unstable lint (#64266).

See also rust-lang/libs-team#704.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 14, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 14, 2025

r? @madsmtm

rustbot has assigned @madsmtm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Warn,
"manual implementation of the internal `Eq::assert_receiver_is_total_eq` method",
@future_incompatible = FutureIncompatibleInfo {
reason: fcw!(FutureReleaseError #150000),
Copy link
Contributor Author

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

@rust-log-analyzer

This comment has been minimized.

@rust-cloud-vms rust-cloud-vms bot force-pushed the no-eq-assert-receiver-method branch from faf29de to 81dc07d Compare December 14, 2025 11:30
@madsmtm madsmtm changed the title deprecate Eq::assert_receiver_is_total_eq and emit a FCW on manual … deprecate Eq::assert_receiver_is_total_eq and emit FCW on manual impls Dec 14, 2025
Copy link
Contributor

@madsmtm madsmtm left a 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?

View changes since this review

@madsmtm madsmtm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants