Skip to content

Suppress unreachable_code lint in derive(PartialEq, Clone)#154910

Merged
rust-bors[bot] merged 3 commits intorust-lang:mainfrom
WaffleLapkin:unreachable-derive
Apr 8, 2026
Merged

Suppress unreachable_code lint in derive(PartialEq, Clone)#154910
rust-bors[bot] merged 3 commits intorust-lang:mainfrom
WaffleLapkin:unreachable-derive

Conversation

@WaffleLapkin
Copy link
Copy Markdown
Member

Resolves #154900

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 6, 2026

Changes to the code generated for builtin derived traits.

cc @nnethercote

@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. labels Apr 6, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 6, 2026

r? @chenyukang

rustbot has assigned @chenyukang.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 69 candidates
  • Random selection from 10 candidates

@Urgau
Copy link
Copy Markdown
Member

Urgau commented Apr 6, 2026

I was going to say that some people may forbid the lint, but fortunately that doesn't seems to be the case.

Copy link
Copy Markdown
Contributor

@nnethercote nnethercote left a comment

Choose a reason for hiding this comment

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

I'm surprised by the diff on deriving-all-codegen.stdout. The code looks like it goes to some effort to only produce #[allow(unreachable_code)] on types where it's needed (those involving !) and yet the output has that attribute on the derived clone for every type.

What I was expecting: all the existing examples in deriving-all-codegen.stdout would be unchanged, and there would be some new types added that contain ! and have unreachable_code in their output.

View changes since this review

@theemathas
Copy link
Copy Markdown
Contributor

@Urgau There are people who #![forbid(unused)]. https://github.com/search?q=lang%3Arust+%2F%5C%5Bforbid%5C%28unused%5C%29%5C%5D%2F&type=code

The unreachable_code lint is in the unused lint group. As a result, trying to #[allow(unreachable_code)] while also having #![forbid(unused)] currently emits a FCW: #81670

@WaffleLapkin
Copy link
Copy Markdown
Member Author

WaffleLapkin commented Apr 7, 2026

@nnethercote the macros don't have access to type information, so they can't know if a type contains ! or not. The code checks if there are any fields (because they then can potentially be !). Note that doing a syntax check for ! would not be enough, as the real life example which prompted me to do this used a type alias to !.

@nnethercote
Copy link
Copy Markdown
Contributor

I see. Is it worth going to the effort to check if there are any fields? You could just slap #[allow(unreachable)] on everything and most of the time it'll be the same result.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 7, 2026

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.

@WaffleLapkin
Copy link
Copy Markdown
Member Author

@nnethercote I think I did the check originally thinking it could be more precise (and thus useful), but in the state I ended up in it was indeed somewhat useless. I simplified code by just indeed always adding the #[allow].

@rust-log-analyzer

This comment has been minimized.

@nnethercote
Copy link
Copy Markdown
Contributor

Makes sense.

Is the forbid(unused) issue a showstopper for this?

@nnethercote
Copy link
Copy Markdown
Contributor

These derived impls all have #[automatically_derived] added to them. Perhaps we could disable lints when that attribute is present?

@WaffleLapkin
Copy link
Copy Markdown
Member Author

Is the #154910 (comment) issue a showstopper for this?

Yes... with this change #[forbid(unreachable_code)] makes derive(Clone) error with allow(unreachable_code) incompatible with previous forbid. Ugh.

This is relevant for `derive(Clone, PartialEq)` on adts with fields of
type never.
@WaffleLapkin
Copy link
Copy Markdown
Member Author

These derived impls all have #[automatically_derived] added to them. Perhaps we could disable lints when that attribute is present?

I'm not sure disabling all lints is a good idea, and it's definitely not something I want to work on right now.

However disabling unreachable code specifically seems simple enough. I updated the code to do that instead of adding allows.

Copy link
Copy Markdown
Contributor

@nnethercote nnethercote left a comment

Choose a reason for hiding this comment

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

Nice! That turned out simpler and better across the board.

View changes since this review

@nnethercote
Copy link
Copy Markdown
Contributor

@bors r+ rollup

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Apr 8, 2026

📌 Commit ddf9d4c has been approved by nnethercote

It is now in the queue for this repository.

@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 Apr 8, 2026
rust-bors Bot pushed a commit that referenced this pull request Apr 8, 2026
…uwer

Rollup of 15 pull requests

Successful merges:

 - #153995 (Use convergent attribute to funcs for GPU targets)
 - #154184 (stabilize s390x vector registers)
 - #151898 (constify DoubleEndedIterator)
 - #154235 (remove unnecessary variables and delimiter check)
 - #154473 (move borrow checker tests)
 - #154745 (Replace span_look_ahead with span_followed_by)
 - #154778 (make field representing types invariant over the base type)
 - #154867 (Fix private fields diagnostics and improve error messages)
 - #154879 (Don't store `pattern_ty` in `TestableCase`)
 - #154910 (Suppress `unreachable_code` lint in `derive(PartialEq, Clone)`)
 - #154923 (Fix ICE in next-solver dyn-compatibility check)
 - #154934 (Add getters for `rustc_pattern_analysis::constructor::Slice` fields)
 - #154938 (match exhaustiveness: Show the guard exhaustivity note only when it's the guards alone that cause non-exhaustiveness)
 - #154961 (Use derived impl for `GappedRange` subdiagnostic)
 - #154980 (rustc-dev-guide subtree update)
@WaffleLapkin
Copy link
Copy Markdown
Member Author

@nnethercote thanks a lot for your review! it turned a lot better indeed :)

@rust-bors rust-bors Bot merged commit 839aaee into rust-lang:main Apr 8, 2026
11 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Apr 8, 2026
rust-timer added a commit that referenced this pull request Apr 8, 2026
Rollup merge of #154910 - WaffleLapkin:unreachable-derive, r=nnethercote

Suppress `unreachable_code` lint in `derive(PartialEq, Clone)`

Resolves #154900
github-actions Bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Apr 13, 2026
…uwer

Rollup of 15 pull requests

Successful merges:

 - rust-lang/rust#153995 (Use convergent attribute to funcs for GPU targets)
 - rust-lang/rust#154184 (stabilize s390x vector registers)
 - rust-lang/rust#151898 (constify DoubleEndedIterator)
 - rust-lang/rust#154235 (remove unnecessary variables and delimiter check)
 - rust-lang/rust#154473 (move borrow checker tests)
 - rust-lang/rust#154745 (Replace span_look_ahead with span_followed_by)
 - rust-lang/rust#154778 (make field representing types invariant over the base type)
 - rust-lang/rust#154867 (Fix private fields diagnostics and improve error messages)
 - rust-lang/rust#154879 (Don't store `pattern_ty` in `TestableCase`)
 - rust-lang/rust#154910 (Suppress `unreachable_code` lint in `derive(PartialEq, Clone)`)
 - rust-lang/rust#154923 (Fix ICE in next-solver dyn-compatibility check)
 - rust-lang/rust#154934 (Add getters for `rustc_pattern_analysis::constructor::Slice` fields)
 - rust-lang/rust#154938 (match exhaustiveness: Show the guard exhaustivity note only when it's the guards alone that cause non-exhaustiveness)
 - rust-lang/rust#154961 (Use derived impl for `GappedRange` subdiagnostic)
 - rust-lang/rust#154980 (rustc-dev-guide subtree update)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deriving Clone and PartialEq on a type containing ! creates warnings

7 participants