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

warn: where_clauses_object_safety when adding default implementation for async_trait #228

Closed
NishantJoshi00 opened this issue Jan 8, 2023 · 3 comments · Fixed by rust-lang/rust#107082

Comments

@NishantJoshi00
Copy link

Description

Getting warning #[warn(where_clauses_object_safety)] when adding default implementation for async_trait

Sample Case

use tokio::time;

#[async_trait::async_trait]
trait Waiter {
    async fn wait_inner_sec(&self, name: &str);
}

struct Walker {
    inner: u64,
}

impl Walker {
    fn new(inner: u64) -> Self { Self { inner } }
}

#[async_trait::async_trait]
impl Waiter for Walker {
    async fn wait_inner_sec(&self, name: &str) {
        println!("waiting for: {}", name);
        time::sleep(time::Duration::new(self.inner, 0)).await
    }
}

#[tokio::main]
async fn main() {
    let walker = Walker::new(1);
    let dyn_walker: Box<&(dyn Waiter)> = Box::new(&walker);
    dyn_walker.wait_inner_sec("plane").await;

}

This example works properly and there is no issue with object-safety here!

Now, if I add a default implementation for the trait

#[async_trait::async_trait]
trait Waiter {
  async fn wait_inner_sec(&self, name: &str) {
    println!("waiting for: {}", name);
    time::sleep(time::Duration::new(5, 0)).await
  }
}

After this change, I suddenly get 1 warning and 1 error.

The Error

error[E0277]: `dyn Waiter` cannot be shared between threads safely
  --> src/main.rs:31:5
   |
31 |     dyn_walker.wait_inner_sec("plane").await;
   |     ^^^^^^^^^^ -------------- required by a bound introduced by this call
   |     |
   |     `dyn Waiter` cannot be shared between threads safely
   |
   = help: the trait `Sync` is not implemented for `dyn Waiter`
note: required by a bound in `Waiter::wait_inner_sec`
  --> src/main.rs:5:5
   |
5  |     async fn wait_inner_sec(&self, name: &str) {
   |     ^^^^^ required by this bound in `Waiter::wait_inner_sec`

This can be solved with

- let dyn_walker: Box<&(dyn Waiter)> = Box::new(&walker);
+ let dyn_walker: Box<&(dyn Waiter + Sync)> = Box::new(&walker);

The Warning

warning: the trait `Waiter` cannot be made into an object
 --> src/main.rs:5:14
  |
5 |     async fn wait_inner_sec(&self, name: &str) {
  |              ^^^^^^^^^^^^^^
  |
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #51443 <https://github.com/rust-lang/rust/issues/51443>
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/main.rs:5:14
  |
4 | trait Waiter {
  |       ------ this trait cannot be made into an object...
5 |     async fn wait_inner_sec(&self, name: &str) {
  |              ^^^^^^^^^^^^^^ ...because method `wait_inner_sec` references the `Self` type in its `where` clause
  = help: consider moving `wait_inner_sec` to another trait
  = note: `#[warn(where_clauses_object_safety)]` on by default
  • The warning doesn't provide the whole context
  • In my implementation I haven't used any where clause.
  • After expanding the macro turns out the function has a where clause, where self is referenced Self: Sync
  • This problem can be solved by trait Waiter: Sync, but should this warning even occur?
@dtolnay
Copy link
Owner

dtolnay commented Jan 8, 2023

It's a false positive rust-lang/rust#51443 (comment).
You can use #![allow(where_clauses_object_safety)] in your crate for now.

@dtolnay dtolnay closed this as completed Jan 8, 2023
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Feb 3, 2023
Autotrait bounds on dyn-safe trait methods

This PR is a successor to rust-lang#106604 implementing the approach encouraged by rust-lang#106604 (comment).

**I propose making it legal to use autotraits as trait bounds on the `Self` type of trait methods in a trait object.** rust-lang#51443 (comment) justifies why this use case is particularly important in the context of the async-trait crate.

```rust
#![feature(auto_traits)]
#![deny(where_clauses_object_safety)]

auto trait AutoTrait {}

trait MyTrait {
    fn f(&self) where Self: AutoTrait;
}

fn main() {
    let _: &dyn MyTrait;
}
```

Previously this would fail with:

```console
error: the trait `MyTrait` cannot be made into an object
 --> src/main.rs:7:8
  |
7 |     fn f(&self) where Self: AutoTrait;
  |        ^
  |
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue rust-lang#51443 <rust-lang#51443>
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/main.rs:7:8
  |
6 | trait MyTrait {
  |       ------- this trait cannot be made into an object...
7 |     fn f(&self) where Self: AutoTrait;
  |        ^ ...because method `f` references the `Self` type in its `where` clause
  = help: consider moving `f` to another trait
```

In order for this to be sound without hitting rust-lang#50781, **I further propose that we disallow handwritten autotrait impls that apply to trait objects.** Both of the following were previously allowed (_on nightly_) and no longer allowed in my proposal:

```rust
auto trait AutoTrait {}

trait MyTrait {}
impl AutoTrait for dyn MyTrait {}  // NOT ALLOWED

impl<T: ?Sized> AutoTrait for T {}  // NOT ALLOWED
```

(`impl<T> AutoTrait for T {}` remains allowed.)

After this change, traits with a default impl are implemented for a trait object **if and only if** the autotrait is one of the trait object's trait bounds (or a supertrait of a bound). In other words `dyn Trait + AutoTrait` always implements AutoTrait while `dyn Trait` never implements AutoTrait.

Fixes dtolnay/async-trait#228.

r? `@lcnr`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Feb 3, 2023
Autotrait bounds on dyn-safe trait methods

This PR is a successor to rust-lang#106604 implementing the approach encouraged by rust-lang#106604 (comment).

**I propose making it legal to use autotraits as trait bounds on the `Self` type of trait methods in a trait object.** rust-lang#51443 (comment) justifies why this use case is particularly important in the context of the async-trait crate.

```rust
#![feature(auto_traits)]
#![deny(where_clauses_object_safety)]

auto trait AutoTrait {}

trait MyTrait {
    fn f(&self) where Self: AutoTrait;
}

fn main() {
    let _: &dyn MyTrait;
}
```

Previously this would fail with:

```console
error: the trait `MyTrait` cannot be made into an object
 --> src/main.rs:7:8
  |
7 |     fn f(&self) where Self: AutoTrait;
  |        ^
  |
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue rust-lang#51443 <rust-lang#51443>
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/main.rs:7:8
  |
6 | trait MyTrait {
  |       ------- this trait cannot be made into an object...
7 |     fn f(&self) where Self: AutoTrait;
  |        ^ ...because method `f` references the `Self` type in its `where` clause
  = help: consider moving `f` to another trait
```

In order for this to be sound without hitting rust-lang#50781, **I further propose that we disallow handwritten autotrait impls that apply to trait objects.** Both of the following were previously allowed (_on nightly_) and no longer allowed in my proposal:

```rust
auto trait AutoTrait {}

trait MyTrait {}
impl AutoTrait for dyn MyTrait {}  // NOT ALLOWED

impl<T: ?Sized> AutoTrait for T {}  // NOT ALLOWED
```

(`impl<T> AutoTrait for T {}` remains allowed.)

After this change, traits with a default impl are implemented for a trait object **if and only if** the autotrait is one of the trait object's trait bounds (or a supertrait of a bound). In other words `dyn Trait + AutoTrait` always implements AutoTrait while `dyn Trait` never implements AutoTrait.

Fixes dtolnay/async-trait#228.

r? `@lcnr`
@dtolnay
Copy link
Owner

dtolnay commented Apr 25, 2023

This is fixed in Rust 1.69.0 by rust-lang/rust#107082.

@NishantJoshi00
Copy link
Author

Awesome! Thanks a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants