Skip to content
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.

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

Suggested bounds restriction suggests duplicate trait #107335

Closed
edward-shen opened this issue Jan 26, 2023 · 3 comments · Fixed by #107555
Closed

Suggested bounds restriction suggests duplicate trait #107335

edward-shen opened this issue Jan 26, 2023 · 3 comments · Fixed by #107555
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@edward-shen
Copy link
Contributor

edward-shen commented Jan 26, 2023

Code

pub trait TryAdd<Rhs = Self> {
    type Error;
    type Output;

    fn try_add(self, rhs: Rhs) -> Result<Self::Output, Self::Error>;
}

impl<T: TryAdd> TryAdd for Option<T> {
    type Error = <T as TryAdd>::Error;
    type Output = Option<<T as TryAdd>::Output>;

    fn try_add(self, rhs: Self) -> Result<Self::Output, Self::Error> {
        Ok(self.or(rhs))
    }
}

Current output

error[E0308]: mismatched types
  --> src/lib.rs:14:12
   |
9  | impl<T: TryAdd> TryAdd for Option<T> {
   |      - this type parameter
...
14 |         Ok(self.or(rhs))
   |         -- ^^^^^^^^^^^^ expected associated type, found type parameter `T`
   |         |
   |         arguments to this enum variant are incorrect
   |
   = note: expected enum `Option<<T as TryAdd>::Output>`
              found enum `Option<T>`
help: the type constructed contains `Option<T>` due to the type of the argument passed
  --> src/lib.rs:14:9
   |
14 |         Ok(self.or(rhs))
   |         ^^^------------^
   |            |
   |            this argument influences the type of `Ok`
note: tuple variant defined here
  --> /rustc/c18a5e8a5b1afb0d7a582fe9ebad4c1996c90da3/library/core/src/result.rs:507:5
help: consider further restricting this bound
   |
9  | impl<T: TryAdd + TryAdd<Output = T>> TryAdd for Option<T> {
   |                ++++++++++++++++++++

For more information about this error, try `rustc --explain E0308`.
error: could not compile `playground` due to previous error

Desired output

error[E0308]: mismatched types
  --> src/lib.rs:14:12
   |
9  | impl<T: TryAdd> TryAdd for Option<T> {
   |      - this type parameter
...
14 |         Ok(self.or(rhs))
   |         -- ^^^^^^^^^^^^ expected associated type, found type parameter `T`
   |         |
   |         arguments to this enum variant are incorrect
   |
   = note: expected enum `Option<<T as TryAdd>::Output>`
              found enum `Option<T>`
help: the type constructed contains `Option<T>` due to the type of the argument passed
  --> src/lib.rs:14:9
   |
14 |         Ok(self.or(rhs))
   |         ^^^------------^
   |            |
   |            this argument influences the type of `Ok`
note: tuple variant defined here
  --> /rustc/c18a5e8a5b1afb0d7a582fe9ebad4c1996c90da3/library/core/src/result.rs:507:5
help: consider further restricting this bound
   |
9  | impl<T: TryAdd<Output = T>> TryAdd for Option<T> {
   |         ++++++++++++++++++

For more information about this error, try `rustc --explain E0308`.
error: could not compile `playground` due to previous error

Rationale and extra context

While the diagnostic produces valid code, I think this diagnostic could be improved. I don't think there's ever a valid scenario where a human would write MyTrait + MyTrait<Output = T> over MyTrait<Output = T>.

Other cases

No response

Anything else?

Reproduces on playground.

Tested on stable 1.66.1 and nightly 1.69.0-nightly (2023-01-25 c18a5e8)

@edward-shen edward-shen added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 26, 2023
@edward-shen
Copy link
Contributor Author

cc @compiler-errors: If this looks like a valid improvement and isn't too hard, I could also give this a shot.

@compiler-errors
Copy link
Member

compiler-errors commented Jan 26, 2023

Feel free to look into it @edward-shen, not sure at a glance if it's hard or not. Ping me on zulip or schedule something if you want to chat.

@edward-shen
Copy link
Contributor Author

@rustbot claim

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 6, 2023
…suggestion, r=compiler-errors

Modify existing bounds if they exist

Fixes rust-lang#107335.

This implementation is kinda gross but I don't really see a better way to do it.

This primarily does two things: Modifies `suggest_constraining_type_param` to accept a new parameter that indicates a span to be replaced instead of added, if presented, and limit the additive suggestions to either suggest a new bound on an existing bound (see newly added unit test) or add the generics argument if a generics argument wasn't found.

The former change is required to retain the capability to add an entirely new bounds if it was entirely omitted.

r? `@compiler-errors`
@bors bors closed this as completed in 917662a Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants