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

E0412 is correct but E0782 is emitted (in trait definitions) #116434

Closed
Colonial-Dev opened this issue Oct 4, 2023 · 9 comments · Fixed by #121111
Closed

E0412 is correct but E0782 is emitted (in trait definitions) #116434

Colonial-Dev opened this issue Oct 4, 2023 · 9 comments · Fixed by #121111
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Colonial-Dev
Copy link
Contributor

Colonial-Dev commented Oct 4, 2023

Code

trait Foo {
    type Clone;
    fn foo() -> Clone
}

Current output

error[E0782]: trait objects must include the `dyn` keyword
 --> src/main.rs:4:17
  |
4 |     fn foo() -> Clone;
  |                 ^^^^^
  |
help: add `dyn` keyword before this trait
  |
4 |     fn foo() -> dyn Clone;
  |                 +++

For more information about this error, try `rustc --explain E0782`.

Desired output

Obviously this isn't helpful, as Clone isn't object safe. The correct error is E0412:

error[E0412]: cannot find type `Clone` in this scope
 --> src/main.rs:4:17
  |
4 |     fn foo() -> Clone;
  |                 ^^^
  |
help: you might have meant to use the associated type
  |
4 |     fn foo() -> Self::Clone;
  |                 ++++++

For more information about this error, try `rustc --explain E0412`.

Rationale and extra context

E0412 is correctly emitted if the associated type name is changed to not collide with any traits currently in scope. So:

trait Foo {
    type Bar;
    fn foo() -> Bar
}

Errors as expected.

I've also seen this confuse new users "in the wild" on Rustcord - that's what originally inspired me to file this issue.

Other cases

No response

Anything else?

No response

@Colonial-Dev Colonial-Dev 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 Oct 4, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 4, 2023
@saethlin saethlin added A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 8, 2023
@obeis obeis removed their assignment Jan 14, 2024
@trevyn
Copy link
Contributor

trevyn commented Jan 20, 2024

@rustbot claim

@trevyn
Copy link
Contributor

trevyn commented Jan 20, 2024

It looks like #119752 (in the queue) changes this to E0038 without a suggestion (for edition 2021):

error[E0038]: the trait `Clone` cannot be made into an object
  --> /Users/e/Documents/GitHub/rust/tests/ui/suggestions/issue-116434.rs:5:17
   |
LL |     fn foo() -> Clone;
   |                 ^^^^^ `Clone` cannot be made into an object
   |
   = note: the trait cannot be made into an object because it requires `Self: Sized`
   = note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>

A suggestion to -> impl Clone is also valid (and with the above pull appears for edition < 2021), but perhaps your -> Self::Clone makes more sense in context, if not exactly... great.

Do you have any links or can share how this is tripping up other users?

@trevyn
Copy link
Contributor

trevyn commented Jan 20, 2024

Ah, apparently it's possible to produce multiple suggestions. I'll poke at it and see what comes out.

@Colonial-Dev
Copy link
Contributor Author

Do you have any links or can share how this is tripping up other users?

It's been a while, but what I saw on the Discord went something like this:

trait DbHandle {
    /* irrelevant, but not object safe */ 
} 

// Some lines away... 

trait DbInterface {
    // (presumably assuming the namespaces don't overlap) 
    type DbHandle;

    fn handle() -> DbHandle;
} 

The user then gets the "please add dyn" message, which they follow.

The compiler then tells them "sorry you can't dyn that," and they wind up in the Discord very confused about the mixed signals.

@trevyn
Copy link
Contributor

trevyn commented Jan 20, 2024

The problematic dyn suggestion is also gone from nightly: playground

The type (trait) DbHandle is in scope, so I don't think E0412 (cannot find type in this scope) is correct.

If it provides the correct suggestion though, it would definitely be an improvement to include that suggestion in the output. I don't see a clear way to get there from the E0038 code path though. :(

@trevyn
Copy link
Contributor

trevyn commented Jan 21, 2024

Making some progress on this, currently blocked on #120164

@trevyn
Copy link
Contributor

trevyn commented Jan 21, 2024

@rustbot label -D-invalid-suggestion

@rustbot rustbot removed the D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. label Jan 21, 2024
@trevyn
Copy link
Contributor

trevyn commented Jan 25, 2024

blocked on #120275

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 31, 2024
…rors

Be less confident when `dyn` suggestion is not checked for object safety

rust-lang#120275 no longer checks bare traits for object safety when making a `dyn` suggestion on Rust < 2021. In this case, qualify the suggestion with a note that the trait must be object safe, to prevent user confusion as seen in rust-lang#116434

r? `@fmease`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 31, 2024
…rors

Be less confident when `dyn` suggestion is not checked for object safety

rust-lang#120275 no longer checks bare traits for object safety when making a `dyn` suggestion on Rust < 2021. In this case, qualify the suggestion with a note that the trait must be object safe, to prevent user confusion as seen in rust-lang#116434

r? ``@fmease``
@trevyn
Copy link
Contributor

trevyn commented Feb 2, 2024

I have a PR ready to go for this that adds the Self:: suggestion on E0038, waiting for bors to merge #120530

oli-obk added a commit to oli-obk/rust that referenced this issue Feb 13, 2024
…rors

Be less confident when `dyn` suggestion is not checked for object safety

rust-lang#120275 no longer checks bare traits for object safety when making a `dyn` suggestion on Rust < 2021. In this case, qualify the suggestion with a note that the trait must be object safe, to prevent user confusion as seen in rust-lang#116434

r? `@fmease`
oli-obk added a commit to oli-obk/rust that referenced this issue Feb 13, 2024
…rors

Be less confident when `dyn` suggestion is not checked for object safety

rust-lang#120275 no longer checks bare traits for object safety when making a `dyn` suggestion on Rust < 2021. In this case, qualify the suggestion with a note that the trait must be object safe, to prevent user confusion as seen in rust-lang#116434

r? ``@fmease``
oli-obk added a commit to oli-obk/rust that referenced this issue Feb 14, 2024
…rors

Be less confident when `dyn` suggestion is not checked for object safety

rust-lang#120275 no longer checks bare traits for object safety when making a `dyn` suggestion on Rust < 2021. In this case, qualify the suggestion with a note that the trait must be object safe, to prevent user confusion as seen in rust-lang#116434

r? ```@fmease```
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 14, 2024
Rollup merge of rust-lang#120530 - trevyn:issue-116434, r=compiler-errors

Be less confident when `dyn` suggestion is not checked for object safety

rust-lang#120275 no longer checks bare traits for object safety when making a `dyn` suggestion on Rust < 2021. In this case, qualify the suggestion with a note that the trait must be object safe, to prevent user confusion as seen in rust-lang#116434

r? ```@fmease```
@bors bors closed this as completed in 670bdbf Feb 16, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 16, 2024
Rollup merge of rust-lang#121111 - trevyn:associated-type-suggestion, r=davidtwco

For E0038, suggest associated type if available

Closes rust-lang#116434
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 A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. 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.

5 participants