Skip to content

More accurate suggestion for missing binop trait bounds #96442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
estebank opened this issue Apr 26, 2022 · 1 comment
Open

More accurate suggestion for missing binop trait bounds #96442

estebank opened this issue Apr 26, 2022 · 1 comment
Assignees
@willcrichton
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-papercut Diagnostics: An error or lint that needs small tweaks. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

estebank commented Apr 26, 2022 •
edited by rustbot
Loading

#94034 (comment)

We currently emit

LL | impl<B: std::ops::Add> Add for D<B> {
   |       +++++++++++++++

but it should be

LL | impl<B: std::ops::Add<Output = B>> Add for D<B> {
   |       +++++++++++++++++++++++++++
@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-papercut Diagnostics: An error or lint that needs small tweaks. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. labels Apr 26, 2022
@willcrichton
Copy link
Contributor

I will take a stab at this soon. I am fairly certain I know how to do it -- there's an Expectation type that describes the expected return type, and if that expectation exists, we can use it to bound Output=<Expectation>.

I tried implementing that today. This is the core change: willcrichton@e652939#diff-ea6f9d55e90333a1f5fc5e202d46862a6a031410831f2df42317ac024f821162R519-R539

This change seems to introduce some noise in other tests involving binops that will require manual fixing. But maybe next weekend I will try to finish this.

@rustbot claim

@willcrichton willcrichton mentioned this issue Apr 27, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 16, 2022
@bors
Auto merge of rust-lang#96482 - willcrichton:fix-trait-suggestion-for…
d695a49 
…-binops, r=estebank

Add Output = expected type trait obligation for known binary operators

This PR is a follow-on to rust-lang#94034 that addresses rust-lang#96442. That is, after replacing the trait-suggestion logic in `op.rs` with a more generic path that analyzes a general set of `Obligation`s, then we lost some specificity in the suggestions where the bounds on the associated type `Output=` would not get suggested.

This PR fixes this issue by changing `FnCtxt::construct_obligation_for_trait` to include a new `ProjectionPredicate` obligation for binary operators that obliges that `Output` is the same as the expected type of the expression. Additionally, to get the expected type of the expression, this PR threads the `Expectation<'tcx>` structure throughout several functions.

See src/test/ui/generic-associated-types/missing-bounds.stderr for an example of how this works.

One side effect of this change is it causes type-check failures with binops to include additional information. Specifically, many now say

```
error: type mismatch resolving `<Lhs as TheBinop>::Output == ExpectedTy`
```

It's up for discussion whether this added context is worth it to the user.

r? `@estebank`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@willcrichton willcrichton

Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-papercut Diagnostics: An error or lint that needs small tweaks. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@willcrichton @estebank