dtolnay / async-trait

Type erasure for async trait methods
Apache License 2.0
1.84k stars 85 forks source link

Feature: Catch unconditional_recursion #250

Closed BKDaugherty closed 1 year ago

BKDaugherty commented 1 year ago

I found myself today writing some stupid code. Once I figured out what I'd done, I was surprised that rustc didn't at least warn me about how dumb I was being. MyStruct had some other traits that it implemented that had similar methods, but I didn't pay enough attention to thinking about how Rust would figure out what method it needs to call, so I effectively wrote the following:

use async_trait::async_trait;

struct MyStruct {}

#[async_trait]
trait MyAsyncTrait {
    async fn my_method(&self);
}

#[async_trait]
impl MyAsyncTrait for MyStruct {
    async fn my_method(&self) {
    self.my_method().await
    }
}

This failed pretty poorly and I ended up with some sweet sweet stack overflows.

Because of this, I was curious if Rust would catch the synchronous versions / non-trait versions, and yay! It did!

impl MyTrait for MyStruct {
    fn my_sync_method(&self) {
    self.my_sync_method()
    }
}

trait MyTrait {
    fn my_sync_method(&self);
}

fn i_obviously_infinite_loop() {
    i_obviously_infinite_loop()
}
brendondaugherty@brendons-laptop clippy_test % rustc --version
rustc 1.71.0 (8ede3aae2 2023-07-12)
brendondaugherty@brendons-laptop clippy_test % rustup default
stable-aarch64-apple-darwin (default)
brendondaugherty@brendons-laptop clippy_test % cargo build
warning: function cannot return without recursing
  --> src/main.rs:18:5
   |
18 |     fn my_sync_method(&self) {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing
19 |     self.my_sync_method()
   |     --------------------- recursive call site
   |
   = help: a `loop` may express intention better if this is on purpose
   = note: `#[warn(unconditional_recursion)]` on by default

warning: function cannot return without recursing
  --> src/main.rs:27:1
   |
27 | fn i_obviously_infinite_loop() {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing
28 |     i_obviously_infinite_loop()
   |     --------------------------- recursive call site
   |
   = help: a `loop` may express intention better if this is on purpose

warning: function `i_obviously_infinite_loop` is never used
  --> src/main.rs:27:4
   |
27 | fn i_obviously_infinite_loop() {
   |    ^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(dead_code)]` on by default

warning: `clippy_test` (bin "clippy_test") generated 3 warnings
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s

I realize that once the macro is expanded, this code looks fairly different, and thus rustc isn't able to tell that it's unconditional_recursion. Would it be possible to maintain that we still catch unconditional recursion in the same way?

    fn my_method<'life0, 'async_trait>(
        &'life0 self,
    ) -> ::core::pin::Pin<
        Box<
            dyn ::core::future::Future<Output = ()> + ::core::marker::Send + 'async_trait,
        >,
    >
    where
        'life0: 'async_trait,
        Self: 'async_trait,
    {
        Box::pin(async move {
            let __self = self;
            let _: () = { __self.my_method().await };
        })
    }

To be honest, I don't really understand why rustc doesn't see this as unconditional_recursion.

Let me know if I should file this instead on rustc itself, (but I thought I would start here) or if you think it's just not worth it!

dtolnay commented 1 year ago

It's filed on rustc already: https://github.com/rust-lang/rust/issues/111177.

BKDaugherty commented 1 year ago

Ooops!!! Sorry @dtolnay should have done some searching there first. Thanks!!