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

Non trait object safe return diagnostic needs improvement #33375

Closed
hythloday opened this issue May 3, 2016 · 10 comments · Fixed by #68377
Closed

Non trait object safe return diagnostic needs improvement #33375

hythloday opened this issue May 3, 2016 · 10 comments · Fixed by #68377
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@hythloday
Copy link

Here's some illegal code that generates a very reasonable error message:

trait Foo {
  fn apply() -> i64;
}

struct Baz{}
static BAZ: Baz = Baz{};
impl Foo for Baz {
    fn apply(&self) -> i64 { 4 }
}

// <anon>:8:5: 8:33 error: method `apply` has a `&self` declaration in the impl, but not in the trait [E0185]

Adding a function that references this broken trait changes the error message and is imho quite misleading:

fn lookup() -> Box<&'static Foo> {
    Box::new(&BAZ)
}

<anon>:11:1: 13:2 error: the trait `Foo` cannot be made into an object [E0038]
<anon>:11 fn lookup() -> Box<&'static Foo> {
<anon>:12     Box::new(&BAZ)
<anon>:13 }
<anon>:11:1: 13:2 help: see the detailed explanation for E0038
<anon>:11:1: 13:2 note: method `apply` has no receiver

The error code has changed from something that pinpoints exactly where I went wrong to something unrelated with advice that won't help me see my problem, and highlights the wrong place (the function that's returning this trait rather than the trait itself). Ideally the error message wouldn't change at all.

rustc 1.10.0-nightly (8da2bcac5 2016-04-28)
binary: rustc
commit-hash: 8da2bcac5db1e091b90cceb19d0496f0f7501c88
commit-date: 2016-04-28
host: x86_64-apple-darwin
release: 1.10.0-nightly

(happens on stable and beta too)

@arielb1 arielb1 added the A-diagnostics Area: Messages for errors, warnings, and lints label May 3, 2016
@arielb1
Copy link
Contributor

arielb1 commented May 3, 2016

The error message is correct - the trait Foo is not object-safe, as the method apply has no receiver. The impl has nothing to do with it.

Maybe we should use "self-type" instead of "receiver"?

@arielb1
Copy link
Contributor

arielb1 commented May 3, 2016

Again: your declared a perfectly fine trait with a single static method. Then you wrote both a non-matching impl and a non-matching method. rustc reported the method, but not the impl.

@hythloday
Copy link
Author

hythloday commented May 3, 2016

No dispute that the error message is correct (or at least, not incorrect). My feeling is that it is unhelpful - the code, whether in the presence or absence of lookup, is erroneous in the same way, and the error message in the first example is the correct one to emit even in the presence of lookup. No changes to lookup can be made that will make this code compile, so it's unhelpful to highlight it as the erroneous section.

It is strange to see one error message refer to a receiver and the other refer to a '&self' declaration but that's not essential.

Hope that's clearer as to why I regard this as a bug. Thanks for the reply!

@Aatch
Copy link
Contributor

Aatch commented May 4, 2016

@hythloday they're different kinds of errors. The first one is reporting the invalid method in the impl, the second error is actually a trait-object error. Moreover, the error is pointing to where the error is occurring, as that should ideally provide more context. The compiler doesn't know that you actually meant to write fn apply(&self) in the trait or if you actually meant to use a different trait instead. If the trait isn't local, we won't even have a span to highlight anyway.

Anyway, these are two different errors, both are equally correct. They're just reporting different things and one of them isn't being shown.

@estebank
Copy link
Contributor

The current output points out at the problem:

error[E0185]: method `apply` has a `&self` declaration in the impl, but not in the trait
 --> <anon>:8:5
  |
2 |   fn apply() -> i64;
  |   ------------------ trait declared without `&self`
...
8 |     fn apply(&self) -> i64 { 4 }
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `&self` used in impl

I believe this can be closed. CC @jonathandturner

@sophiajt
Copy link
Contributor

@estebank - the original report I think is talking about the bad error given for:

trait Foo {
  fn apply() -> i64;
}

struct Baz{}
static BAZ: Baz = Baz{};
impl Foo for Baz {
    fn apply(&self) -> i64 { 4 }
}

fn lookup() -> Box<&'static Foo> {
    Box::new(&BAZ)
}

We still give the same confusing error:

error[E0038]: the trait `Foo` cannot be made into an object
  --> quickie.rs:11:1
   |
11 |   fn lookup() -> Box<&'static Foo> {
   |  _^ starting here...
12 | |     Box::new(&BAZ)
13 | | }
   | |_^ ...ending here: the trait `Foo` cannot be made into an object
   |
   = note: method `apply` has no receiver

@Mark-Simulacrum Mark-Simulacrum changed the title When trait decl lacks '&self' but trait impl defines it, defining a function returning malformed trait gives very confusing error message Non trait object safe return diagnostic needs improvement Jun 13, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 25, 2017
@estebank
Copy link
Contributor

Current output:

error[E0038]: the trait `Foo` cannot be made into an object
  --> src/lib.rs:11:1
   |
11 | fn lookup() -> Box<&'static Foo> {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Foo` cannot be made into an object
   |
   = note: method `apply` has no receiver

@estebank
Copy link
Contributor

Current output

error[E0038]: the trait `Foo` cannot be made into an object
  --> src/lib.rs:11:1
   |
2  |   fn apply() -> i64;
   |      ----- associated function `apply` has no `self` parameter
...
11 | fn lookup() -> Box<&'static Foo> {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Foo` cannot be made into an object

@estebank estebank added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. D-papercut Diagnostics: An error or lint that needs small tweaks. labels Jan 22, 2020
@estebank
Copy link
Contributor

estebank commented Feb 1, 2020

After #68347 the output would be:

error[E0038]: the trait `Foo` cannot be made into an object
  --> file.rs:11:16
   |
1  | trait Foo {
   |       --- this trait cannot be made into an object...
2  |   fn apply() -> i64;
   |      ----- ...because associated function `apply` has no `self` parameter
...
11 | fn lookup() -> Box<&'static Foo> {
   |                ^^^^^^^^^^^^^^^^^ the trait `Foo` cannot be made into an object
   |
help: consider turning `apply` into a method by giving it a `&self` argument or constraining it so it does not apply to trait objects
   |
2  |   fn apply() -> i64 where Self: Sized;
   |                     ^^^^^^^^^^^^^^^^^

@hythloday
Copy link
Author

Thanks Esteban! That is a lot clearer and exactly what I would expect.

@bors bors closed this as completed in 5b0caef Feb 4, 2020
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 C-enhancement Category: An issue proposing an enhancement or a PR with one. D-papercut Diagnostics: An error or lint that needs small tweaks. 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.

6 participants