Skip to content
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

Tweak suggestions for bare trait used as a type #119148

Merged
merged 6 commits into from
Jan 5, 2024

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Dec 20, 2023

error[E0782]: trait objects must include the `dyn` keyword
  --> $DIR/not-on-bare-trait-2021.rs:11:11
   |
LL | fn bar(x: Foo) -> Foo {
   |           ^^^
   |
help: use a generic type parameter, constrained by the trait `Foo`
   |
LL | fn bar<T: Foo>(x: T) -> Foo {
   |       ++++++++    ~
help: you can also use `impl Foo`, but users won't be able to specify the type paramer when calling the `fn`, having to rely exclusively on type inference
   |
LL | fn bar(x: impl Foo) -> Foo {
   |           ++++
help: alternatively, use a trait object to accept any type that implements `Foo`, accessing its methods at runtime using dynamic dispatch
   |
LL | fn bar(x: &dyn Foo) -> Foo {
   |           ++++

error[E0782]: trait objects must include the `dyn` keyword
  --> $DIR/not-on-bare-trait-2021.rs:11:19
   |
LL | fn bar(x: Foo) -> Foo {
   |                   ^^^
   |
help: use `impl Foo` to return an opaque type, as long as you return a single underlying type
   |
LL | fn bar(x: Foo) -> impl Foo {
   |                   ++++
help: alternatively, you can return an owned trait object
   |
LL | fn bar(x: Foo) -> Box<dyn Foo> {
   |                   +++++++    +

Fix #119525:


error[E0038]: the trait `Ord` cannot be made into an object
  --> $DIR/bare-trait-dont-suggest-dyn.rs:3:33
   |
LL | fn ord_prefer_dot(s: String) -> Ord {
   |                                 ^^^ `Ord` 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>
  --> $SRC_DIR/core/src/cmp.rs:LL:COL
   |
   = note: the trait cannot be made into an object because it uses `Self` as a type parameter
  ::: $SRC_DIR/core/src/cmp.rs:LL:COL
   |
   = note: the trait cannot be made into an object because it uses `Self` as a type parameter
help: consider using an opaque type instead
   |
LL | fn ord_prefer_dot(s: String) -> impl Ord {
   |                                 ++++

@estebank estebank marked this pull request as ready for review December 20, 2023 01:05
@rustbot
Copy link
Collaborator

rustbot commented Dec 20, 2023

r? @davidtwco

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 20, 2023
@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Dec 20, 2023

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@bors

This comment was marked as resolved.

Add the following suggestions:

```
error[E0782]: trait objects must include the `dyn` keyword
  --> $DIR/not-on-bare-trait-2021.rs:11:11
   |
LL | fn bar(x: Foo) -> Foo {
   |           ^^^
   |
help: use a generic type parameter, constrained by the trait `Foo`
   |
LL | fn bar<T: Foo>(x: T) -> Foo {
   |       ++++++++    ~
help: you can also use `impl Foo`, but users won't be able to specify the type paramer when calling the `fn`, having to rely exclusively on type inference
   |
LL | fn bar(x: impl Foo) -> Foo {
   |           ++++
help: alternatively, use a trait object to accept any type that implements `Foo`, accessing its methods at runtime using dynamic dispatch
   |
LL | fn bar(x: &dyn Foo) -> Foo {
   |           ++++

error[E0782]: trait objects must include the `dyn` keyword
  --> $DIR/not-on-bare-trait-2021.rs:11:19
   |
LL | fn bar(x: Foo) -> Foo {
   |                   ^^^
   |
help: use `impl Foo` to return an opaque type, as long as you return a single underlying type
   |
LL | fn bar(x: Foo) -> impl Foo {
   |                   ++++
help: alternatively, you can return an owned trait object
   |
LL | fn bar(x: Foo) -> Box<dyn Foo> {
   |                   +++++++    +
```
Note the parentheses in the last suggestion:

```
error[E0277]: the size for values of type `(dyn Foo + Send + 'static)` cannot be known at compilation time
  --> $DIR/not-on-bare-trait.rs:7:8
   |
LL | fn foo(_x: Foo + Send) {
   |        ^^ doesn't have a size known at compile-time
   |
   = help: the trait `Sized` is not implemented for `(dyn Foo + Send + 'static)`
   = help: unsized fn params are gated as an unstable feature
help: you can use `impl Trait` as the argument type
   |
LL | fn foo(_x: impl Foo + Send) {
   |            ++++
help: function arguments must have a statically known size, borrowed types always have a known size
   |
LL | fn foo(_x: &(Foo + Send)) {
   |            ++          +
```
…tType`

This gets us more accurate suggestions.
@rust-log-analyzer

This comment has been minimized.

@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jan 5, 2024

📌 Commit 698dfc3 has been approved by davidtwco

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 5, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 5, 2024
…mpiler-errors

Rollup of 10 pull requests

Successful merges:

 - rust-lang#119034 (Allow coverage tests to ignore test modes, and to enable color in coverage reports)
 - rust-lang#119148 (Tweak suggestions for bare trait used as a type)
 - rust-lang#119538 (Cleanup error handlers: round 5)
 - rust-lang#119566 (Remove `-Zdump-mir-spanview`)
 - rust-lang#119567 (Remove `-Zreport-delayed-bugs`.)
 - rust-lang#119577 (Migrate memory overlap check from validator to lint)
 - rust-lang#119583 (Make `intrinsics::assume` const stable)
 - rust-lang#119586 ([rustdoc] Fix invalid handling for static method calls in jump to definition feature)
 - rust-lang#119588 (Move `i586-unknown-netbsd` from tier 2 to tier 3 platform support table)
 - rust-lang#119601 (`Emitter` cleanups)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f41ad1b into rust-lang:master Jan 5, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 5, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 5, 2024
Rollup merge of rust-lang#119148 - estebank:bare-traits, r=davidtwco

Tweak suggestions for bare trait used as a type

```
error[E0782]: trait objects must include the `dyn` keyword
  --> $DIR/not-on-bare-trait-2021.rs:11:11
   |
LL | fn bar(x: Foo) -> Foo {
   |           ^^^
   |
help: use a generic type parameter, constrained by the trait `Foo`
   |
LL | fn bar<T: Foo>(x: T) -> Foo {
   |       ++++++++    ~
help: you can also use `impl Foo`, but users won't be able to specify the type paramer when calling the `fn`, having to rely exclusively on type inference
   |
LL | fn bar(x: impl Foo) -> Foo {
   |           ++++
help: alternatively, use a trait object to accept any type that implements `Foo`, accessing its methods at runtime using dynamic dispatch
   |
LL | fn bar(x: &dyn Foo) -> Foo {
   |           ++++

error[E0782]: trait objects must include the `dyn` keyword
  --> $DIR/not-on-bare-trait-2021.rs:11:19
   |
LL | fn bar(x: Foo) -> Foo {
   |                   ^^^
   |
help: use `impl Foo` to return an opaque type, as long as you return a single underlying type
   |
LL | fn bar(x: Foo) -> impl Foo {
   |                   ++++
help: alternatively, you can return an owned trait object
   |
LL | fn bar(x: Foo) -> Box<dyn Foo> {
   |                   +++++++    +
```

Fix rust-lang#119525:

```

error[E0038]: the trait `Ord` cannot be made into an object
  --> $DIR/bare-trait-dont-suggest-dyn.rs:3:33
   |
LL | fn ord_prefer_dot(s: String) -> Ord {
   |                                 ^^^ `Ord` 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>
  --> $SRC_DIR/core/src/cmp.rs:LL:COL
   |
   = note: the trait cannot be made into an object because it uses `Self` as a type parameter
  ::: $SRC_DIR/core/src/cmp.rs:LL:COL
   |
   = note: the trait cannot be made into an object because it uses `Self` as a type parameter
help: consider using an opaque type instead
   |
LL | fn ord_prefer_dot(s: String) -> impl Ord {
   |                                 ++++
```
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jan 11, 2024
Tweak suggestions for bare trait used as a type

```
error[E0782]: trait objects must include the `dyn` keyword
  --> $DIR/not-on-bare-trait-2021.rs:11:11
   |
LL | fn bar(x: Foo) -> Foo {
   |           ^^^
   |
help: use a generic type parameter, constrained by the trait `Foo`
   |
LL | fn bar<T: Foo>(x: T) -> Foo {
   |       ++++++++    ~
help: you can also use `impl Foo`, but users won't be able to specify the type paramer when calling the `fn`, having to rely exclusively on type inference
   |
LL | fn bar(x: impl Foo) -> Foo {
   |           ++++
help: alternatively, use a trait object to accept any type that implements `Foo`, accessing its methods at runtime using dynamic dispatch
   |
LL | fn bar(x: &dyn Foo) -> Foo {
   |           ++++

error[E0782]: trait objects must include the `dyn` keyword
  --> $DIR/not-on-bare-trait-2021.rs:11:19
   |
LL | fn bar(x: Foo) -> Foo {
   |                   ^^^
   |
help: use `impl Foo` to return an opaque type, as long as you return a single underlying type
   |
LL | fn bar(x: Foo) -> impl Foo {
   |                   ++++
help: alternatively, you can return an owned trait object
   |
LL | fn bar(x: Foo) -> Box<dyn Foo> {
   |                   +++++++    +
```

Fix rust-lang#119525:

```

error[E0038]: the trait `Ord` cannot be made into an object
  --> $DIR/bare-trait-dont-suggest-dyn.rs:3:33
   |
LL | fn ord_prefer_dot(s: String) -> Ord {
   |                                 ^^^ `Ord` 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>
  --> $SRC_DIR/core/src/cmp.rs:LL:COL
   |
   = note: the trait cannot be made into an object because it uses `Self` as a type parameter
  ::: $SRC_DIR/core/src/cmp.rs:LL:COL
   |
   = note: the trait cannot be made into an object because it uses `Self` as a type parameter
help: consider using an opaque type instead
   |
LL | fn ord_prefer_dot(s: String) -> impl Ord {
   |                                 ++++
```
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 22, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request 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 pull request 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 pull request 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 pull request 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustc suggests dyn SomeTrait for traits which aren't object safe
6 participants