Skip to content

Incorrect Suggestion when Returning a Bare Trait from a Function #127691

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
veera-sivarajan opened this issue Jul 13, 2024 · 4 comments
Closed

Incorrect Suggestion when Returning a Bare Trait from a Function #127691

veera-sivarajan opened this issue Jul 13, 2024 · 4 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-trait-objects Area: trait objects, vtable layout D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@veera-sivarajan
Copy link
Contributor

Code

// edition 2021
trait Trait {}

fn fun() -> Trait {
    todo!()
}

fn main() {}

Current output

error[E0746]: return type cannot have an unboxed trait object
 --> <source>:5:13
  |
5 | fn fun() -> Trait {
  |             ^^^^^ doesn't have a size known at compile-time
  |
help: box the return type, and wrap all of the returned values in `Box::new`
  |
5 ~ fn fun() -> Box<Trait> {
6 ~     Box::new(todo!())
  |

error[E0782]: trait objects must include the `dyn` keyword
 --> <source>:5:13
  |
5 | fn fun() -> Trait {
  |             ^^^^^
  |
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
  |
5 | fn fun() -> impl Trait {
  |             ++++
help: alternatively, you can return an owned trait object
  |
5 | fn fun() -> Box<dyn Trait> {
  |             +++++++      +

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0746, E0782.
For more information about an error, try `rustc --explain E0746`.

Desired output

error[E0782]: trait objects must include the `dyn` keyword
 --> <source>:5:13
  |
5 | fn fun() -> Trait {
  |             ^^^^^
  |
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
  |
5 | fn fun() -> impl Trait {
  |             ++++
help: alternatively, you can return an owned trait object
  |
5 | fn fun() -> Box<dyn Trait> {
  |             +++++++      +

Rationale and extra context

This error report has two issues:

  1. E0746 incorrectly suggests to return a Box<Trait>.
  2. Both the errors say the same thing and E0746 should be suppressed in favor of E0782.

Other cases

No response

Rust Version

rustc 1.79.0

Anything else?

No response

@veera-sivarajan veera-sivarajan 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 Jul 13, 2024
@veera-sivarajan
Copy link
Contributor Author

@rustbot label +D-incorrect +D-verbose +A-trait-objects

@rustbot rustbot added A-trait-objects Area: trait objects, vtable layout D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. labels Jul 13, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 2, 2024
…stebank

Suggest `impl Trait` for References to Bare Trait in Function Header

Fixes rust-lang#125139

This PR suggests `impl Trait` when `&Trait` is found as a function parameter type or return type. This makes use of existing diagnostics by adding `peel_refs()` when checking for type equality.

Additionaly, it makes a few other improvements:
1. Checks if functions inside impl blocks have bare trait in their headers.
2. Introduces a trait `NextLifetimeParamName` similar to the existing `NextTypeParamName` for suggesting a lifetime name. Also, abstracts out the common logic between the two trait impls.

### Related Issues
I ran into a bunch of related diagnostic issues but couldn't fix them within the scope of this PR. So, I have created the following issues:
1. [Misleading Suggestion when Returning a Reference to a Bare Trait from a Function](rust-lang#127689)
2. [Verbose Error When a Function Takes a Bare Trait as Parameter](rust-lang#127690)
3. [Incorrect Suggestion when Returning a Bare Trait from a Function](rust-lang#127691)

r​? `@estebank` since you implemented  rust-lang#119148
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 3, 2024
…stebank

Suggest `impl Trait` for References to Bare Trait in Function Header

Fixes rust-lang#125139

This PR suggests `impl Trait` when `&Trait` is found as a function parameter type or return type. This makes use of existing diagnostics by adding `peel_refs()` when checking for type equality.

Additionaly, it makes a few other improvements:
1. Checks if functions inside impl blocks have bare trait in their headers.
2. Introduces a trait `NextLifetimeParamName` similar to the existing `NextTypeParamName` for suggesting a lifetime name. Also, abstracts out the common logic between the two trait impls.

### Related Issues
I ran into a bunch of related diagnostic issues but couldn't fix them within the scope of this PR. So, I have created the following issues:
1. [Misleading Suggestion when Returning a Reference to a Bare Trait from a Function](rust-lang#127689)
2. [Verbose Error When a Function Takes a Bare Trait as Parameter](rust-lang#127690)
3. [Incorrect Suggestion when Returning a Bare Trait from a Function](rust-lang#127691)

r​? ``@estebank`` since you implemented  rust-lang#119148
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 3, 2024
…stebank

Suggest `impl Trait` for References to Bare Trait in Function Header

Fixes rust-lang#125139

This PR suggests `impl Trait` when `&Trait` is found as a function parameter type or return type. This makes use of existing diagnostics by adding `peel_refs()` when checking for type equality.

Additionaly, it makes a few other improvements:
1. Checks if functions inside impl blocks have bare trait in their headers.
2. Introduces a trait `NextLifetimeParamName` similar to the existing `NextTypeParamName` for suggesting a lifetime name. Also, abstracts out the common logic between the two trait impls.

### Related Issues
I ran into a bunch of related diagnostic issues but couldn't fix them within the scope of this PR. So, I have created the following issues:
1. [Misleading Suggestion when Returning a Reference to a Bare Trait from a Function](rust-lang#127689)
2. [Verbose Error When a Function Takes a Bare Trait as Parameter](rust-lang#127690)
3. [Incorrect Suggestion when Returning a Bare Trait from a Function](rust-lang#127691)

r​? ```@estebank``` since you implemented  rust-lang#119148
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 5, 2024
Rollup merge of rust-lang#127692 - veera-sivarajan:bugfix-125139, r=estebank

Suggest `impl Trait` for References to Bare Trait in Function Header

Fixes rust-lang#125139

This PR suggests `impl Trait` when `&Trait` is found as a function parameter type or return type. This makes use of existing diagnostics by adding `peel_refs()` when checking for type equality.

Additionaly, it makes a few other improvements:
1. Checks if functions inside impl blocks have bare trait in their headers.
2. Introduces a trait `NextLifetimeParamName` similar to the existing `NextTypeParamName` for suggesting a lifetime name. Also, abstracts out the common logic between the two trait impls.

### Related Issues
I ran into a bunch of related diagnostic issues but couldn't fix them within the scope of this PR. So, I have created the following issues:
1. [Misleading Suggestion when Returning a Reference to a Bare Trait from a Function](rust-lang#127689)
2. [Verbose Error When a Function Takes a Bare Trait as Parameter](rust-lang#127690)
3. [Incorrect Suggestion when Returning a Bare Trait from a Function](rust-lang#127691)

r​? ```@estebank``` since you implemented  rust-lang#119148
@progval
Copy link
Contributor

progval commented Oct 8, 2024

Additionally, the E0746 suggestion is doubly incorrect when the trait is returned from an async function: the suggestions are not syntactically valid:

trait Trait {}

async fn fun() -> Trait {
    todo!()
}

fn main() {}

causes:

error[E0746]: return type cannot have an unboxed trait object
 --> src/main.rs:3:25
  |
3 |   async fn fun() -> Trait {
  |  _________________________^
4 | |     todo!()
5 | | }
  | |_^ doesn't have a size known at compile-time
  |
help: consider returning an `impl Trait` instead of a `dyn Trait`
  |
3 | async fn fun() -> Trait impl {
  |                         ++++
help: alternatively, box the return type, and wrap all of the returned values in `Box::new`
  |
3 ~ async fn fun() -> Trait Box<dyn {
4 |     todo!()
5 ~ }>
  |

error[E0782]: trait objects must include the `dyn` keyword
 --> src/main.rs:3:19
  |
3 | async fn fun() -> Trait {
  |                   ^^^^^
  |
help: add `dyn` keyword before this trait
  |
3 | async fn fun() -> dyn Trait {
  |                   +++
help: you might have meant to write a bound here
  |
1 | : Trait {
  | ~

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

@GrigorenkoPV
Copy link
Contributor

As of the latest master (ecf2d1f), the error message is now:

error[E0782]: expected a type, found a trait
 --> src/main.rs:4:13
  |
4 | fn fun() -> Trait {
  |             ^^^^^
  |
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
  |
4 | fn fun() -> impl Trait {
  |             ++++
help: alternatively, you can return an owned trait object
  |
4 | fn fun() -> Box<dyn Trait> {
  |             +++++++      +

which looks ideal, IMO. So this issue can probably be closed now?

The async case is still a bit wrong, but less broken than before:

error[E0782]: expected a type, found a trait
 --> src/main.rs:3:19
  |
3 | async fn fun() -> Trait {
  |                   ^^^^^
  |
help: you can add the `dyn` keyword if you want a trait object
  |
3 | async fn fun() -> dyn Trait {
  |                   +++
help: you might have meant to write a bound here
  |
1 | : Trait {
  | ~

but maybe this should be moved into a separate issue?

@veera-sivarajan
Copy link
Contributor Author

Yup, thank you!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-trait-objects Area: trait objects, vtable layout D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. 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

4 participants