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

Misleading suggestions for trait objects in modern rust #131051

Closed
jake-87 opened this issue Sep 30, 2024 · 5 comments
Closed

Misleading suggestions for trait objects in modern rust #131051

jake-87 opened this issue Sep 30, 2024 · 5 comments
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

@jake-87
Copy link

jake-87 commented Sep 30, 2024

Code

trait Foo<T> {
    fn id(me: T) -> T;
}

/* note the "missing" for ... (in this case for i64, in order for this to compile) */
impl Foo<i64> { 
    fn id(me: i64) -> i64 {me}
}

fn main() {
    let x: i64 = <i64 as Foo<i64>>::id(10);
    println!("{}",x);
}

Current output

error[E0038]: the trait `Foo` cannot be made into an object
 --> <source>:5:6
  |
5 | impl Foo<i64> {
  |      ^^^^^^^^ `Foo` cannot be made into an object
  |
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>
 --> <source>:2:8
  |
1 | trait Foo<T> {
  |       --- this trait cannot be made into an object...
2 |     fn id(me: T) -> T;
  |        ^^ ...because associated function `id` has no `self` parameter
help: consider turning `id` into a method by giving it a `&self` argument
  |
2 |     fn id(&self, me: T) -> T;
  |           ++++++
help: alternatively, consider constraining `id` so it does not apply to trait objects
  |
2 |     fn id(me: T) -> T where Self: Sized;
  |                       +++++++++++++++++

error[E0277]: the trait bound `i64: Foo<i64>` is not satisfied
  --> <source>:10:19
   |
10 |     let x: i64 = <i64 as Foo<i64>>::id(10);
   |                   ^^^ the trait `Foo<i64>` is not implemented for `i64`
   |
help: this trait has no implementations, consider adding one
  --> <source>:1:1
   |
1  | trait Foo<T> {
   | ^^^^^^^^^^^^

error[E0782]: trait objects must include the `dyn` keyword
 --> <source>:5:6
  |
5 | impl Foo<i64> {
  |      ^^^^^^^^
  |
help: add `dyn` keyword before this trait
  |
5 | impl dyn Foo<i64> {
  |      +++

error: aborting due to 3 previous errors

Some errors have detailed explanations: E0038, E0277, E0782.
For more information about an error, try `rustc --explain E0038`.
Compiler returned: 1

Desired output

  1. At least some suggestion that a for may be missing (ie to signal an implementation of a trait), instead of only a suggestion around anonymous trait objects, which is somewhat misleading in this case.
  2. The object-safety error would ideally not appear.

Rationale and extra context

The lack of a for ... here, to implement Foo for some type, causes two large unrelated errors, both of which are only relevant for code not migrated from Rust 2015. While these are obviously good to keep around, I would think that a suggestion that a for ... might be missing could also be useful, as it's a very plausible error to make.
Additionally, having the object-safety error come up in a modern rust edition, when it is known that that impl cannot be for a trait object, seems like it may cause more confusion than it solves. That error is obviously still needed, but for it to be "blocked" behind impl ... being changed to impl dyn ... if the Rust edition is >=2018, ensuring the "intentional" presence of a trait object, seems to me like a good approach.

This would be a very easy mistake to make if you were attempting to use Rust traits like Haskell typeclasses because the below compiles and runs fine, due to the lack of implicit self:

class Foo t where
  id' :: t -> t
instance Foo Int where 
  id' x = x
main = print (id' (10 :: Int))

Rust Version

rustc 1.81.0 (eeb90cd 2024-09-04)
binary: rustc
commit-hash: eeb90cd
commit-date: 2024-09-04
host: x86_64-unknown-linux-gnu
release: 1.81.0
LLVM version: 18.1.7
Compiler returned: 0

@jake-87 jake-87 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 Sep 30, 2024
@kpreid
Copy link
Contributor

kpreid commented Oct 1, 2024

Similar:

@VulnBandit
Copy link
Contributor

@rustbot claim.
I will try to tackle this vulnerability

@jake-87
Copy link
Author

jake-87 commented Oct 3, 2024

@VulnBandit I'm sorry, I'm quite confused. This is not a vulnerability.

@VulnBandit
Copy link
Contributor

Yes I meant to say I will write code to fix this

workingjubilee added a commit to workingjubilee/rustc that referenced this issue Oct 5, 2024
…errors

Account for `impl Trait {` when `impl Trait for Type {` was intended

On editions where bare traits are never allowed, detect if the user has written `impl Trait` with no type, silence any dyn-compatibility errors, and provide a structured suggestion for the potentially missing type:

```
error[E0782]: trait objects must include the `dyn` keyword
  --> $DIR/missing-for-type-in-impl.rs:8:6
   |
LL | impl Foo<i64> {
   |      ^^^^^^^^
   |
help: add `dyn` keyword before this trait
   |
LL | impl dyn Foo<i64> {
   |      +++
help: you might have intended to implement this trait for a given type
   |
LL | impl Foo<i64> for /* Type */ {
   |               ++++++++++++++
```

CC rust-lang#131051.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 5, 2024
Rollup merge of rust-lang#131273 - estebank:issue-131051, r=compiler-errors

Account for `impl Trait {` when `impl Trait for Type {` was intended

On editions where bare traits are never allowed, detect if the user has written `impl Trait` with no type, silence any dyn-compatibility errors, and provide a structured suggestion for the potentially missing type:

```
error[E0782]: trait objects must include the `dyn` keyword
  --> $DIR/missing-for-type-in-impl.rs:8:6
   |
LL | impl Foo<i64> {
   |      ^^^^^^^^
   |
help: add `dyn` keyword before this trait
   |
LL | impl dyn Foo<i64> {
   |      +++
help: you might have intended to implement this trait for a given type
   |
LL | impl Foo<i64> for /* Type */ {
   |               ++++++++++++++
```

CC rust-lang#131051.
@jake-87 jake-87 closed this as completed Oct 5, 2024
@jake-87
Copy link
Author

jake-87 commented Oct 5, 2024

@estebank Thanks!

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

No branches or pull requests

3 participants