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

confusing diagnostic when calling a method that doesn't exist on a trait but exists on another trait #111312

Closed
fee1-dead opened this issue May 7, 2023 · 9 comments · Fixed by #111588
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@fee1-dead
Copy link
Member

fee1-dead commented May 7, 2023

Code

trait Has {
    fn has() {}
}

trait HasNot {}

fn main() {
    HasNot::has();
}

Current output

error[E0782]: trait objects must include the `dyn` keyword
 --> src/main.rs:8:5
  |
8 |     HasNot::has();
  |     ^^^^^^
  |
help: add `dyn` keyword before this trait
  |
8 |     <dyn HasNot>::has();
  |     ++++       +

error[E0599]: no function or associated item named `has` found for trait object `dyn HasNot` in the current scope
 --> src/main.rs:8:13
  |
8 |     HasNot::has();
  |             ^^^ function or associated item not found in `dyn HasNot`
  |
  = help: items from traits can only be used if the trait is implemented and in scope
note: `Has` defines an item `has`, perhaps you need to implement it
 --> src/main.rs:1:1
  |
1 | trait Has {
  | ^^^^^^^^^

Some errors have detailed explanations: E0599, E0782.
For more information about an error, try `rustc --explain E0599`.
error: could not compile `playground` due to 2 previous errors

code snippet by @WaffleLapkin. (this is from December 2022)

@fee1-dead fee1-dead 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. D-confusing Diagnostics: Confusing error or lint that should be reworked. labels May 7, 2023
@mu001999
Copy link
Contributor

mu001999 commented May 7, 2023

@rustbot claim

@mu001999
Copy link
Contributor

mu001999 commented May 7, 2023

What is your desired output? I browsed the tests and the current output is as expected to suggest implementing Has for dyn HasNot.

@mu001999
Copy link
Contributor

mu001999 commented May 7, 2023

The following snippet works fine (play):

trait Has {
    fn has() {}
}

trait HasNot {}

impl Has for dyn HasNot {}

fn main() {
    <dyn HasNot>::has();
}

@fee1-dead
Copy link
Member Author

This is confusing, because the intention is almost never to call the trait method on a trait object. The diagnostic remains the same even if Has is not object safe. This happened to me when I just wanted to call a method on Has instead.

@mu001999
Copy link
Contributor

mu001999 commented May 7, 2023

This is confusing, because the intention is almost never to call the trait method on a trait object. The diagnostic remains the same even if Has is not object safe. This happened to me when I just wanted to call a method on Has instead.

Using trait name without dyn as the trait object was allowed before the edition 2021 (E0782), while the associative function has cannot be called via HasNot::has(), so the former is considered to be the intention when meeting HasNot::has().

Then the first suggestion adds <dyn HasNot> and leads to the next suggestion adding impl Has for dyn HasNot. This process does not require Has to be object-safe.

@fee1-dead
Copy link
Member Author

fee1-dead commented May 7, 2023

Sorry, I mean when Has requires Sized or when HasNot is not object safe. Also, I know that trait name without dyn was allowed. Anyways I think the errors for this code should be improved. I still think that people who would write this kind of code almost certainly did not mean to call <dyn HasNot as Has>::has()

@fee1-dead
Copy link
Member Author

fee1-dead commented May 7, 2023

For additional context, here's the original error that I got:

image

I meant to use the bare function max_by, but rustc is not being helpful at indicating that Ord does not have the max_by. For other people who might encounter this issue it might be because they used the wrong trait and meant to call on the other trait instead. It is very rare that people actually meant to do <dyn Foo as Bar>::bar() so it's unhelpful to assume that that was the intention.

@mu001999
Copy link
Contributor

mu001999 commented May 8, 2023

I think this may need a bit more works, and I open a related topic on internals.

@mu001999
Copy link
Contributor

@rustbot claim

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 D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
2 participants