Skip to content

Improve diagnostics for ambiguous binops with as _#156591

Open
qaijuang wants to merge 3 commits into
rust-lang:mainfrom
qaijuang:diagnostics-as-infer-binop
Open

Improve diagnostics for ambiguous binops with as _#156591
qaijuang wants to merge 3 commits into
rust-lang:mainfrom
qaijuang:diagnostics-as-infer-binop

Conversation

@qaijuang

@qaijuang qaijuang commented May 15, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

View all comments

Fixes #156004

as _ inside the RHS of an overloaded binary operation can leave the cast target unresolved while there is still a pending operator obligation that knows the more useful ambiguity.

This PR keeps the normal resolved-cast path unchanged, such that when the cast target is still a type variable, cast checking now looks for a pending BinOp obligation whose RHS contains the cast span and replays that obligation in a fresh diagnostic fulfillment context. If the replayed obligation is actually ambiguous, rustc reports the existing fulfillment ambiguity, including the competing impl candidates.

Best reviewed commit-by-commit.

@rustbot rustbot added 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. labels May 15, 2026
@qaijuang qaijuang marked this pull request as ready for review May 15, 2026 10:10
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 15, 2026
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 15, 2026
@rustbot

rustbot commented May 15, 2026

Copy link
Copy Markdown
Collaborator

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
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 73 candidates
  • Random selection from 17 candidates

@qaijuang

qaijuang commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

cc @mqudsi

hkBst
hkBst suggested changes May 27, 2026
View reviewed changes

@hkBst hkBst left a comment •
edited by rustbot
Loading

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.

In addition to my code comments, I wonder if we could not reuse the existing diagnostic code for "multiple impls satisfying type: Trait" instead, since that is ultimately the reason for the ambiguity of "as _" here.

View changes since this review

Comment thread compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs Outdated
Comment thread compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs Outdated
Comment thread tests/ui/inference/multiple-impl-apply.stderr
@qaijuang

qaijuang commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

In addition to my code comments, I wonder if we could not reuse the existing diagnostic code for "multiple impls satisfying type: Trait" instead, since that is ultimately the reason for the ambiguity of "as _" here.

View changes since this review

I not sure what you mean here, we are reusing existing

@qaijuang

qaijuang commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

github acting slow as usual -- the updates is yet to be processed

@hkBst

hkBst commented May 27, 2026

Copy link
Copy Markdown
Member

github acting slow as usual -- the updates is yet to be processed

Been checking the past 2 hours, but still seems to show nothing new... :(

@qaijuang qaijuang force-pushed the diagnostics-as-infer-binop branch from 30e4f90 to daf9b2e Compare May 27, 2026 15:15
@qaijuang

qaijuang commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

github acting slow as usual -- the updates is yet to be processed

Been checking the past 2 hours, but still seems to show nothing new... :(

It was a git cache issue 😪

hkBst
hkBst suggested changes May 28, 2026
View reviewed changes
Comment thread compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs Outdated
Comment thread compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs Outdated
@qaijuang qaijuang force-pushed the diagnostics-as-infer-binop branch from daf9b2e to c401e16 Compare May 28, 2026 10:05
hkBst
hkBst suggested changes May 28, 2026
View reviewed changes
Comment thread compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
@qaijuang qaijuang force-pushed the diagnostics-as-infer-binop branch from c401e16 to af30177 Compare May 28, 2026 17:07
@hkBst

hkBst commented May 29, 2026

Copy link
Copy Markdown
Member

@qaijuang Please reread my review comments (which you marked resolved) and try to implement my suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@hkBst hkBst hkBst requested changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@TaKO8Ki TaKO8Ki

Labels

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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Failed type inference for primitive ops doesn't provide reason

4 participants

@qaijuang @rustbot @hkBst @TaKO8Ki