Skip to content

deprecate Eq::assert_receiver_is_total_eq and emit FCW on manual impls#149978

Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
cyrgani:no-eq-assert-receiver-method
Feb 26, 2026
Merged

deprecate Eq::assert_receiver_is_total_eq and emit FCW on manual impls#149978
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
cyrgani:no-eq-assert-receiver-method

Conversation

@cyrgani
Copy link
Contributor

@cyrgani cyrgani commented Dec 14, 2025

View all comments

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

@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
@cyrgani cyrgani added I-lang-nominated Nominated for discussion during a lang team meeting. I-libs-nominated Nominated for discussion during a libs team meeting. labels Dec 14, 2025
@traviscross traviscross added I-libs-api-nominated Nominated for discussion during a libs-api team meeting. P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang and removed I-libs-nominated Nominated for discussion during a libs team meeting. labels Dec 17, 2025
@Amanieu
Copy link
Member

Amanieu commented Jan 6, 2026

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
Copy link
Collaborator

Error encounted:
Provided team libs-api lang is invalid

@Amanieu Amanieu added T-lang Relevant to the language team T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed 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 Jan 6, 2026
@Amanieu
Copy link
Member

Amanieu commented Jan 6, 2026

@rfcbot merge

@rust-rfcbot
Copy link
Collaborator

Error encounted:
Provided team `` is invalid

@Amanieu
Copy link
Member

Amanieu commented Jan 6, 2026

@rfcbot merge libs-api,lang

@rust-rfcbot
Copy link
Collaborator

rust-rfcbot commented Jan 6, 2026

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

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 rust-rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jan 6, 2026
@Amanieu Amanieu removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Jan 6, 2026
@rust-bors rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 25, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 25, 2026
…od, r=madsmtm

deprecate `Eq::assert_receiver_is_total_eq` and emit FCW on manual impls

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 (rust-lang#64266).

See also rust-lang/libs-team#704.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 25, 2026
…od, r=madsmtm

deprecate `Eq::assert_receiver_is_total_eq` and emit FCW on manual impls

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 (rust-lang#64266).

See also rust-lang/libs-team#704.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 25, 2026
…od, r=madsmtm

deprecate `Eq::assert_receiver_is_total_eq` and emit FCW on manual impls

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 (rust-lang#64266).

See also rust-lang/libs-team#704.
@traviscross traviscross added the relnotes Marks issues that should be documented in the release notes of the next release. label Feb 25, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 25, 2026
…od, r=madsmtm

deprecate `Eq::assert_receiver_is_total_eq` and emit FCW on manual impls

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 (rust-lang#64266).

See also rust-lang/libs-team#704.
rust-bors bot pushed a commit that referenced this pull request Feb 25, 2026
…uwer

Rollup of 13 pull requests

Successful merges:

 - #153079 (Revert "Move aarch64-apple dist builder to dynamic llvm linking")
 - #148146 (CI: use alternative disks if available)
 - #149937 (spliit out `linker-info` from `linker-messages`)
 - #151771 (Fix: On wasm targets, call `panic_in_cleanup` if panic occurs in cleanup)
 - #153035 (rustc_target: callconv: powerpc64: Use llvm_abiname rather than target_abi for ABI determination)
 - #153075 (mGCA: Lower negated literals directly and reject non-integer negations)
 - #153078 (Remove `QuerySystemFns`)
 - #149978 (deprecate `Eq::assert_receiver_is_total_eq` and emit FCW on manual impls)
 - #153029 (Rename `rustc::pass_by_value` lint as `rustc::disallowed_pass_by_ref`.)
 - #153063 (`is_ty_must_use`: do not require a `span` argument)
 - #153071 (Update books)
 - #153092 (Remove redundant self usages)
 - #153094 (Simplify `AppendOnlyVec` iterators)

Failed merges:

 - #153091 (Migration of `LintDiagnostic` - part 4)
rust-bors bot pushed a commit that referenced this pull request Feb 26, 2026
Rollup of 13 pull requests

Successful merges:

 - #148146 (CI: use alternative disks if available)
 - #151771 (Fix: On wasm targets, call `panic_in_cleanup` if panic occurs in cleanup)
 - #153035 (rustc_target: callconv: powerpc64: Use llvm_abiname rather than target_abi for ABI determination)
 - #153075 (mGCA: Lower negated literals directly and reject non-integer negations)
 - #153078 (Remove `QuerySystemFns`)
 - #153089 (interpret: avoid dummy spans in the stacktrace)
 - #153111 (Refactor url_parts to return is_absolute instead of out param)
 - #149978 (deprecate `Eq::assert_receiver_is_total_eq` and emit FCW on manual impls)
 - #153029 (Rename `rustc::pass_by_value` lint as `rustc::disallowed_pass_by_ref`.)
 - #153063 (`is_ty_must_use`: do not require a `span` argument)
 - #153071 (Update books)
 - #153092 (Remove redundant self usages)
 - #153094 (Simplify `AppendOnlyVec` iterators)

Failed merges:

 - #153091 (Migration of `LintDiagnostic` - part 4)
@rust-bors rust-bors bot merged commit 7a88346 into rust-lang:main Feb 26, 2026
11 checks passed
@rustbot rustbot added this to the 1.95.0 milestone Feb 26, 2026
rust-timer added a commit that referenced this pull request Feb 26, 2026
Rollup merge of #149978 - cyrgani:no-eq-assert-receiver-method, r=madsmtm

deprecate `Eq::assert_receiver_is_total_eq` and emit FCW on manual impls

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.
@cyrgani cyrgani deleted the no-eq-assert-receiver-method branch February 26, 2026 08:08
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Feb 26, 2026
Rollup of 13 pull requests

Successful merges:

 - rust-lang/rust#148146 (CI: use alternative disks if available)
 - rust-lang/rust#151771 (Fix: On wasm targets, call `panic_in_cleanup` if panic occurs in cleanup)
 - rust-lang/rust#153035 (rustc_target: callconv: powerpc64: Use llvm_abiname rather than target_abi for ABI determination)
 - rust-lang/rust#153075 (mGCA: Lower negated literals directly and reject non-integer negations)
 - rust-lang/rust#153078 (Remove `QuerySystemFns`)
 - rust-lang/rust#153089 (interpret: avoid dummy spans in the stacktrace)
 - rust-lang/rust#153111 (Refactor url_parts to return is_absolute instead of out param)
 - rust-lang/rust#149978 (deprecate `Eq::assert_receiver_is_total_eq` and emit FCW on manual impls)
 - rust-lang/rust#153029 (Rename `rustc::pass_by_value` lint as `rustc::disallowed_pass_by_ref`.)
 - rust-lang/rust#153063 (`is_ty_must_use`: do not require a `span` argument)
 - rust-lang/rust#153071 (Update books)
 - rust-lang/rust#153092 (Remove redundant self usages)
 - rust-lang/rust#153094 (Simplify `AppendOnlyVec` iterators)

Failed merges:

 - rust-lang/rust#153091 (Migration of `LintDiagnostic` - part 4)
@JonathanBrouwer
Copy link
Contributor

@rust-timer build 74d03a9

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (74d03a9): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
0.5% [0.3%, 0.9%] 4
Regressions ❌
(secondary)
2.2% [0.1%, 5.9%] 6
Improvements ✅
(primary)
-0.5% [-0.5%, -0.5%] 1
Improvements ✅
(secondary)
-0.3% [-0.3%, -0.3%] 1
All ❌✅ (primary) 0.3% [-0.5%, 0.9%] 5

Max RSS (memory usage)

Results (primary 3.5%, secondary 4.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
4.9% [2.8%, 7.5%] 5
Regressions ❌
(secondary)
4.7% [0.8%, 8.0%] 6
Improvements ✅
(primary)
-3.8% [-3.8%, -3.8%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 3.5% [-3.8%, 7.5%] 6

Cycles

Results (primary -2.1%, secondary 2.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.1% [3.6%, 4.6%] 2
Improvements ✅
(primary)
-2.1% [-2.1%, -2.1%] 1
Improvements ✅
(secondary)
-2.3% [-2.3%, -2.3%] 1
All ❌✅ (primary) -2.1% [-2.1%, -2.1%] 1

Binary size

Results (primary 0.3%, secondary 3.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
0.3% [0.0%, 1.3%] 34
Regressions ❌
(secondary)
3.0% [0.1%, 12.8%] 15
Improvements ✅
(primary)
-0.5% [-0.5%, -0.5%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.3% [-0.5%, 1.3%] 35

Bootstrap: 492.161s -> 479.662s (-2.54%)
Artifact size: 395.78 MiB -> 397.79 MiB (0.51%)

@rustbot rustbot added the perf-regression Performance regression. label Feb 26, 2026
nonself_args: vec![],
ret_ty: Unit,
attributes: thin_vec![
cx.attr_word(sym::inline, span),
Copy link
Contributor

Choose a reason for hiding this comment

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

Damn, I didn't think about that, but probably this that causes the regression? Inlined functions aren't codegen'd.

@cyrgani will you file a PR to fix it, or should I?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nvmd, I filed #153157 myself

rust-bors bot pushed a commit that referenced this pull request Mar 2, 2026
…JonathanBrouwer

Re-add `#[inline]` to `Eq::assert_fields_are_eq`

Fixes a compile-time regression in #149978: non-inline methods are generally codegen'd while inline methods are deferred (and this function should never be called, so deferring is the right choice).

r? JonathanBrouwer
CC @cyrgani
github-actions bot pushed a commit to rust-lang/rust-analyzer that referenced this pull request Mar 2, 2026
Rollup of 13 pull requests

Successful merges:

 - rust-lang/rust#148146 (CI: use alternative disks if available)
 - rust-lang/rust#151771 (Fix: On wasm targets, call `panic_in_cleanup` if panic occurs in cleanup)
 - rust-lang/rust#153035 (rustc_target: callconv: powerpc64: Use llvm_abiname rather than target_abi for ABI determination)
 - rust-lang/rust#153075 (mGCA: Lower negated literals directly and reject non-integer negations)
 - rust-lang/rust#153078 (Remove `QuerySystemFns`)
 - rust-lang/rust#153089 (interpret: avoid dummy spans in the stacktrace)
 - rust-lang/rust#153111 (Refactor url_parts to return is_absolute instead of out param)
 - rust-lang/rust#149978 (deprecate `Eq::assert_receiver_is_total_eq` and emit FCW on manual impls)
 - rust-lang/rust#153029 (Rename `rustc::pass_by_value` lint as `rustc::disallowed_pass_by_ref`.)
 - rust-lang/rust#153063 (`is_ty_must_use`: do not require a `span` argument)
 - rust-lang/rust#153071 (Update books)
 - rust-lang/rust#153092 (Remove redundant self usages)
 - rust-lang/rust#153094 (Simplify `AppendOnlyVec` iterators)

Failed merges:

 - rust-lang/rust#153091 (Migration of `LintDiagnostic` - part 4)
@madsmtm madsmtm added perf-regression-triaged The performance regression has been triaged. and removed perf-regression-triaged The performance regression has been triaged. labels Mar 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. perf-regression Performance regression. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting

Projects

None yet

Development

Successfully merging this pull request may close these issues.