Skip to content

Fix trait method resolution on an adjusted never type#156047

Open
JonathanBrouwer wants to merge 2 commits intorust-lang:mainfrom
JonathanBrouwer:never-ty-method
Open

Fix trait method resolution on an adjusted never type#156047
JonathanBrouwer wants to merge 2 commits intorust-lang:mainfrom
JonathanBrouwer:never-ty-method

Conversation

@JonathanBrouwer
Copy link
Copy Markdown
Contributor

@JonathanBrouwer JonathanBrouwer commented May 1, 2026

@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 May 1, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 1, 2026

WaffleLapkin is not on the review rotation at the moment.
They may take a while to respond.

Copy link
Copy Markdown
Contributor Author

@JonathanBrouwer JonathanBrouwer May 1, 2026

Choose a reason for hiding this comment

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

Note that this PR allows more code to compile on stable, for example see this test.

View changes since the review

Copy link
Copy Markdown
Contributor Author

@JonathanBrouwer JonathanBrouwer May 3, 2026

Choose a reason for hiding this comment

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

If this is a problem we could delay the problem by putting this change behind the never_type feature gate

Comment thread compiler/rustc_hir_typeck/src/method/probe.rs Outdated
Copy link
Copy Markdown
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

I think this a reasonable change, I would expect us to emit a FCW here, I don't want to support this permanently or more importantly, this may overlap with other kinds of inference fallback in the future, so we generally don't want users to rely on this

View changes since this review

@WaffleLapkin WaffleLapkin added the F-never_type `#![feature(never_type)]` label May 1, 2026
@rust-log-analyzer

This comment has been minimized.

@WaffleLapkin WaffleLapkin added needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. T-types Relevant to the types team, which will review and decide on the PR/issue. labels May 1, 2026
@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented May 1, 2026

This PR is now based on #156048, still need to add the FCW lint, I'll do that later.
@rustbot author

Is "Trait method call on adjusted never type" the correct terminology? I'm not super familiar with the type system yet

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 2, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 2, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 2, 2026
@rust-bors

This comment has been minimized.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 3, 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.

}

declare_lint! {
/// The `trait_method_on_coerced_never_type` lint detects situations in which a never type, which
Copy link
Copy Markdown
Contributor Author

@JonathanBrouwer JonathanBrouwer May 3, 2026

Choose a reason for hiding this comment

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

I'm really unsure of what the correct terminology is, both for the lint definition and the diagnostic, so please give feedback on this

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO the proper way to say it would be "method_call_on_diverging_infer_variable" and "detects situations in which a method is called on a value resulting from a never-to-any coercion, without necessary information to infer a type for it", but I'm not sure how much we want to surface this internal language...

"detects trait method calls on an coerced never type",
@future_incompatible = FutureIncompatibleInfo {
reason: fcw!(FutureReleaseError #156047),
report_in_deps: true,
Copy link
Copy Markdown
Contributor Author

@JonathanBrouwer JonathanBrouwer May 3, 2026

Choose a reason for hiding this comment

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

I decided on a lint with level Warn with report_in_deps: true. If something else is desired, let me know

View changes since the review

@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented May 3, 2026

@rustbot ready

I rebased on #156079 and #156048 so the diff is nicely small now

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 3, 2026
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

Copy link
Copy Markdown
Member

@WaffleLapkin WaffleLapkin left a comment

Choose a reason for hiding this comment

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

Code changes look good to me. This is blocked on T-types FCP (also the wording in the lint could be improved).

View changes since this review

}

declare_lint! {
/// The `trait_method_on_coerced_never_type` lint detects situations in which a never type, which
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO the proper way to say it would be "method_call_on_diverging_infer_variable" and "detects situations in which a method is called on a value resulting from a never-to-any coercion, without necessary information to infer a type for it", but I'm not sure how much we want to surface this internal language...

Comment on lines +5731 to +5736
/// Calling trait methods on a coerced `!` was previously disallowed for the never type,
/// but it did work for empty enums such as `Infallible` since these don't coerce.
/// This means that changing the definition of `Infallible` to become a type alias to `!` (a long-term goal),
/// would break code that called a trait method on `Infallible`, in such a way that the `!` would coerce.
///
/// Therefore, to aid in the transition of changing `Infallible` to a type alias, this is temporarily allowed with a FCW.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please also add a note that calling a method on ! is never useful (it's unreachable code) and propose removing the method call / using the never to any coercion.

Also mention that if people want to support MSRVs before Infallible = !, the proper fix is to match e {} as DesiredType.

@WaffleLapkin WaffleLapkin added S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

F-never_type `#![feature(never_type)]` needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent method lookup on never type

5 participants