rust-lang / impl-trait-utils

Utilities for working with impl traits in Rust.
Apache License 2.0
89 stars 9 forks source link

Add support for defaulted methods #20

Open mendess opened 8 months ago

mendess commented 8 months ago

Resolves #17

mendess commented 8 months ago

@compiler-errors so I gave your suggestions a try with these modifications to the example trait

#[trait_variant::make(IntFactory: Send)]
pub trait LocalIntFactory {
    const NAME: &'static str;

    type MyFut<'a>: Future
    where
        Self: 'a;

    async fn make(&self, x: u32, y: &str) -> i32;
    async fn make_mut(&mut self);
    fn stream(&self) -> impl Iterator<Item = i32>;
    fn call(&self) -> u32;
    fn another_async(&self, input: Result<(), &str>) -> Self::MyFut<'_>;
    async fn defaulted(&self) -> i32 {
        self.make(10, "10").await
    }
    async fn defaulted_mut(&mut self) -> i32 {
        self.make(10, "10").await
    }
    async fn defaulted_mut_2(&mut self) {
        self.make_mut().await
    }
    async fn defaulted_move(self) -> i32
    where
        Self: Sized,
    {
        self.make(10, "10").await
    }
}

And I generated something like this

pub trait IntFactory: Send {
    const NAME: &'static str;
    type MyFut<'a>: Future
    where
        Self: 'a;
    fn make(&self, x: u32, y: &str) -> impl ::core::future::Future<Output = i32> + Send;
    fn make_mut(&mut self) -> impl ::core::future::Future<Output = ()> + Send;
    fn stream(&self) -> impl Iterator<Item = i32> + Send;
    fn call(&self) -> u32;
    fn another_async(&self, input: Result<(), &str>) -> Self::MyFut<'_>;
    fn defaulted<'the_self_lt>(&'the_self_lt self) -> impl ::core::future::Future<Output = i32> + Send
    where
        &'the_self_lt Self: Send,
    {
        async { self.make(10, "10").await }
    }
    fn defaulted_mut<'the_self_lt>(&'the_self_lt mut self) -> impl ::core::future::Future<Output = i32> + Send
    where
        &'the_self_lt mut Self: Send, // error[E0277]: `Self` cannot be shared between threads safely
    {
        async { self.make(10, "10").await }
    }
    fn defaulted_mut_2<'the_self_lt>(&'the_self_lt mut self) -> impl ::core::future::Future<Output = ()> + Send
    where
        &'the_self_lt mut Self: Send,
    {
        async { self.make_mut().await }
    }
    fn defaulted_move(self) -> impl ::core::future::Future<Output = i32> + Send
    where
        Self: Sized,
        Self: Send, // error[E0277]: `Self` cannot be shared between threads safely
    {
        async { self.make(10, "10").await }
    }
}

The errors are because the async block of those particular default implementations don't "capture" self the same way the method does. For example, default_mut doesn't actually need &mut self for it's implementation. Hence the compiler requires Self: Sync.

Do you still think it's worth trying to generate very precise bounds like I tried to do here?

andrewtoth commented 8 months ago

@mendess I believe in your generated example you are missing the move keyword after async. Using async move {...} fixes the compile errors.

Your explanation of the error makes sense, hence why it is required to move the actual receiver type into the block instead of just borrowing &Self.

mendess commented 8 months ago

@andrewtoth I remove the move keyword of what @compiler-errors mentioned, but maybe I'm misunderstood them

compiler-errors commented 8 months ago

I didn't mention removing the move keyword--that is definitely needed.

mendess commented 8 months ago

Alright, edited the codegen to have more precise bounds for Self

This is what it looks like now

use std::future::Future;

#[allow(async_fn_in_trait)]
pub trait LocalIntFactory {
    const NAME: &'static str;

    type MyFut<'a>: Future
    where
        Self: 'a;

    async fn make(&self, x: u32, y: &str) -> i32;
    async fn make_mut(&mut self);
    fn stream(&self) -> impl Iterator<Item = i32>;
    fn call(&self) -> u32;
    fn another_async(&self, input: Result<(), &str>) -> Self::MyFut<'_>;
    async fn defaulted(&self) -> i32 {
        self.make(10, "10").await
    }
    async fn defaulted_mut(&mut self) -> i32 {
        self.make(10, "10").await
    }
    async fn defaulted_mut_2(&mut self) {
        self.make_mut().await
    }
    async fn defaulted_move(self) -> i32
    where
        Self: Sized,
    {
        self.make(10, "10").await
    }
}
pub trait IntFactory: Send {
    const NAME: &'static str;
    type MyFut<'a>: Future
    where
        Self: 'a;
    fn make(&self, x: u32, y: &str) -> impl ::core::future::Future<Output = i32> + Send;
    fn make_mut(&mut self) -> impl ::core::future::Future<Output = ()> + Send;
    fn stream(&self) -> impl Iterator<Item = i32> + Send;
    fn call(&self) -> u32;
    fn another_async(&self, input: Result<(), &str>) -> Self::MyFut<'_>;
    fn defaulted<'the_self_lt>(&'the_self_lt self) -> impl ::core::future::Future<Output = i32> + Send
    where
        &'the_self_lt Self: Send,
    {
        async move { self.make(10, "10").await }
    }
    fn defaulted_mut<'the_self_lt>(&'the_self_lt mut self) -> impl ::core::future::Future<Output = i32> + Send
    where
        &'the_self_lt mut Self: Send,
    {
        async move { self.make(10, "10").await }
    }
    fn defaulted_mut_2<'the_self_lt>(&'the_self_lt mut self) -> impl ::core::future::Future<Output = ()> + Send
    where
        &'the_self_lt mut Self: Send,
    {
        async move { self.make_mut().await }
    }
    fn defaulted_move(self) -> impl ::core::future::Future<Output = i32> + Send
    where
        Self: Sized,
        Self: Send,
    {
        async move { self.make(10, "10").await }
    }
}
impl<T> LocalIntFactory for T
where
    T: IntFactory,
    Self: Sync // this is only added if there are defaulted methods
{
    const NAME: &'static str = <Self as IntFactory>::NAME;
    type MyFut<'a> = <Self as IntFactory>::MyFut<'a> where Self: 'a;
    async fn make(&self, x: u32, y: &str) -> i32 {
        <Self as IntFactory>::make(self, x, y).await
    }
    async fn make_mut(&mut self) {
        <Self as IntFactory>::make_mut(self).await
    }
    fn stream(&self) -> impl Iterator<Item = i32> {
        <Self as IntFactory>::stream(self)
    }
    fn call(&self) -> u32 {
        <Self as IntFactory>::call(self)
    }
    fn another_async(&self, input: Result<(), &str>) -> Self::MyFut<'_> {
        <Self as IntFactory>::another_async(self, input)
    }
    async fn defaulted(&self) -> i32 {
        <Self as IntFactory>::defaulted(self).await
    }
    async fn defaulted_mut(&mut self) -> i32 {
        <Self as IntFactory>::defaulted_mut(self).await
    }
    async fn defaulted_mut_2(&mut self) {
        <Self as IntFactory>::defaulted_mut_2(self).await
    }
    async fn defaulted_move(self) -> i32
    where
        Self: Sized,
    {
        <Self as IntFactory>::defaulted_move(self).await
    }
}
mendess commented 8 months ago

Edited the codegen again and this is what it looks like now

impl<TraitVariantBlanketType> LocalIntFactory for TraitVariantBlanketType
where
    TraitVariantBlanketType: IntFactory,
    for<'s> &'s Self: Send, // no longer bounds `Self: Sync`
{
    const NAME: &'static str = <Self as IntFactory>::NAME;
    type MyFut<'a> = <Self as IntFactory<>>::MyFut<'a> where Self: 'a;
    async fn make(&self, x: u32, y: &str) -> i32 {
        <Self as IntFactory>::make(self, x, y).await
    }
    async fn make_mut(&mut self) {
        <Self as IntFactory>::make_mut(self).await
    }
    fn stream(&self) -> impl Iterator<Item = i32> {
        <Self as IntFactory>::stream(self)
    }
    fn call(&self) -> u32 {
        <Self as IntFactory>::call(self)
    }
    fn another_async(&self, input: Result<(), &str>) -> Self::MyFut<'_> {
        <Self as IntFactory>::another_async(self, input)
    }
    async fn defaulted(&self, x: u32) -> i32 {
        <Self as IntFactory>::defaulted(self, x).await
    }
    async fn defaulted_mut(&mut self) -> i32 {
        <Self as IntFactory>::defaulted_mut(self).await
    }
    async fn defaulted_mut_2(&mut self) {
        <Self as IntFactory>::defaulted_mut_2(self).await
    }
    async fn defaulted_move(self) -> i32
    where
        Self: Sized,
    {
        <Self as IntFactory>::defaulted_move(self).await
    }
}
sargarass commented 7 months ago

@mendess can you please rebase it onto new main.

mendess commented 7 months ago

@sargarass Done!

mendess commented 6 months ago

@compiler-errors could I get a re-review? Maybe get this merged? :eyes:

sargarass commented 6 months ago

@tmandry could you also take a look? We are using the Mendess branch in our closed-source project and would like to have it merged.

loichyan commented 5 months ago

I tested this PR and found that calling an async function that returns a static reference in the defaulted method leads to a compilation error:

#[trait_variant::make(MyTrait: Send)]
// 1. lifetime may not live long enough
//    returning this value requires that `'the_self_lt` must outlive `'static`
// 2. implementation of `std::marker::Send` is not general enough
//    `&'0 Self` must implement `std::marker::Send`, for any lifetime `'0`...
//    ...but `std::marker::Send` is actually implemented for the type `&'the_self_lt Self`
// 3. to declare that `impl std::future::Future<Output = i32> + std::marker::Send` captures data from argument `self`, you can add an explicit `'the_self_lt` lifetime bound: ` + 'the_self_lt`
// 4. lifetime `'the_self_lt` defined here
pub trait LocalMyTrait {
    async fn make(&self, x: u32, y: &str) -> i32;

    async fn defaulted_with_static(&self, x: u32, y: &str) -> i32 {
        let _t = get_global_var().await;
        self.make(x, y).await
    }
}

async fn get_global_var() -> &'static i32 {
    todo!()
}

The expanded code:

#[allow(async_fn_in_trait)]
pub trait LocalMyTrait {
    async fn make(&self, x: u32, y: &str) -> i32;
    async fn defaulted_with_static(&self, x: u32, y: &str) -> i32 {
        let _t = get_global_var().await;
        self.make(x, y).await
    }
}
pub trait MyTrait: Send {
    fn make(&self, x: u32, y: &str) -> impl ::core::future::Future<Output = i32> + Send;
    fn defaulted_with_static<'the_self_lt>(
        &'the_self_lt self,
        x: u32,
        y: &str,
    ) -> impl ::core::future::Future<Output = i32> + Send // Add 'the_self_lt bound here, it compiles
    where
        &'the_self_lt Self: Send,
    {
        async move {
            let __self = self;
            let x = x;
            let y = y;
            {
                let _t = get_global_var().await;
                __self.make(x, y).await
            }
        }
    }
}
impl<TraitVariantBlanketType: MyTrait> LocalMyTrait for TraitVariantBlanketType
where
    for<'s> &'s Self: Send,
{
    async fn make(&self, x: u32, y: &str) -> i32 {
        <Self as MyTrait>::make(self, x, y).await
    }
    async fn defaulted_with_static(&self, x: u32, y: &str) -> i32 {
        <Self as MyTrait>::defaulted_with_static(self, x, y).await
    }
}
loichyan commented 5 months ago

It also results in a lifetime mismatch error when overriding the default method:

#[trait_variant::make(MyTrait: Send)]
pub trait LocalMyTrait {
    async fn defaulted(&self, _x: u32, _y: &str) -> i32 {
        todo!()
    }
}

struct MyImpl;

impl MyTrait for MyImpl {
    // 1. lifetime parameters or bounds on method `defaulted` do not match the trait declaration
    // lifetimes do not match method in trait [E0195]
    async fn defaulted(&self, _x: u32, _y: &str) -> i32 {
        todo!()
    }
}
loichyan commented 5 months ago

... an async function that returns a static reference in the defaulted method leads to a compilation error It also results in a lifetime mismatch error when overriding the default method

A possible solution for the two issues is to replace the &'the_self_lt Self: Send bound with Self: Sync.

Sytten commented 4 months ago

Any movement?

tmandry commented 1 month ago

A possible solution for the two issues is to replace the &'the_self_lt Self: Send bound with Self: Sync.

That is one way, and might lead to better error messages in the Send case specifically.

In the general case we can replace &'the_self_lt Self: Send with for<'a> &'a Self: Send and it seems to work.