deprecate Eq::assert_receiver_is_total_eq and emit FCW on manual impls

[cyrgani]

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]

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

[madsmtm]

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

[Amanieu]

Since there's never a valid reason to manually implement this method, having an FCW urging people to remove their manual implementation seems like the best way to go.

@rfcbot merge libs-api lang

[rust-rfcbot]

Error encounted:
Provided team libs-api lang is invalid

[Amanieu]

@rfcbot merge

[rust-rfcbot]

Error encounted:
Provided team `` is invalid

[Amanieu]

@rfcbot merge libs-api,lang

[rust-rfcbot]

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @nikomatsakis
• @scottmcm
• @the8472
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rust-rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

[madsmtm]

Question: Does this mean that if I type #![forbid(internal_eq_trait_method_impls)], I can't derive PartialEq and Eq? Or is the span of the macro properly tracked such that lint levels like this isn't an issue? I can't recall the details there.

[cyrgani]

This is indeed a problem. The easiest fix I could think of for now was to add a second hidden, unstable method to Eq and move all the assertions into that method. (this is what this PR does now)

I tried to implement the above design with a const _, but encountered a problem with this code:

#[derive(PartialEq, Eq)]
pub struct A<'a, T>(&'a [T]);

Cleaning up the derive macro output, the difference is:

pub struct A<'a, T>(&'a [T]);

impl<'a, T> A<'a, T> {
    fn old_way() {
        let _: &'a [T];
    }
}

const _: () = {
    fn new_way<'a, T>() {
        let _: &'a [T];
    }
};

and new_way errors with error[E0309]: the parameter type T may not live long enough.

[madsmtm]

I should've been clearer in my wording here: I don't think actually think it's a blocking concern, people don't do #![forbid] that often, and this is a FCW, and the intention is for it to be unnecessary in the future when we figure out how to do the impl in a const _: ().

I just noticed it and thought it might be worthwhile to discuss the tradeoffs off.

I think I like the previous version of the PR better, adding the extra assert_fields_are_eq feels confusing (as a user, it's already weird to go look at Eq's implementation and see a private method, seeing yet another one doesn't help).

[madsmtm]

Re the error, it is indeed odd that type parameters work that way. But maybe something like this could work?

const _: () = {
    struct __AssertIsEq<'a, T: core::eq::Eq>(core::eq::AssertParamIsEq<&'a [T]>);
};

[madsmtm]

(In any case, I'd prefer that actually fixing the derive macro is done in a different PR).

[cyrgani]

For the old approach, I'm worried that someone might have #![forbid(future_incompatible)] in their code and would now get a confusing error from inside #[derive(Eq)]. (138 uses with github search)

Also I opened #152504 for the migration of the macro away from the function.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[rust-log-analyzer]

The job pr-check-2 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

REPOSITORY                                   TAG       IMAGE ID       CREATED       SIZE
ghcr.io/dependabot/dependabot-updater-core   latest    b72a662c47e3   12 days ago   790MB
=> Removing docker images...
Deleted Images:
untagged: ghcr.io/dependabot/dependabot-updater-core:latest
untagged: ghcr.io/dependabot/dependabot-updater-core@sha256:57ef9cc45f72cc4258ee1baa8243bc3cd55c0a0e570b6768c37346247be35f0d
deleted: sha256:b72a662c47e31df2e7bf59368b2b83be239f02a1baa721393717711a1a719df9
deleted: sha256:3e13ccd80f19769f39008cfc6549938e1ea4905f47b028c1df2dd6085191386c
deleted: sha256:842807995a512b2c5a9b241a3aecdbe79af6b0642d96fa5460cfcf0c9d8be295
deleted: sha256:0f9074b9f46f4570eb7cb4b65fcb3c3d909f9b1d14ca66b30508117b6deda303
deleted: sha256:2ca99cb9251d19157c56b5d91c8961bb4b35196a5ca9b4ffdccbf24abbfe2a5f