feat!: Implement support for generic params

[DanikVitek]

Also:

• Improve error messages
• Extendable type-safe rewrite of the macro expansion

Closes #16.

[DanikVitek]

Hello, @scouten. Ready for review

[DanikVitek]

Should be marked as "Closes #16"

[DanikVitek]

I need to add support for #[cfg_attr(..., async_generic)], so don not merge just yet

[DanikVitek]

Or rather, attributes on async_signature

[DanikVitek]

Battle-testing the implementation on borsh-rs

[scouten]

@DanikVitek this looks very interesting. I'll want to make sure the CI runs pass before merging and also for you to add the #[cfg(...)] upgrades that you are planning.

Let me know how I can help.

[scouten]

Great and welcome proposal! I look forward to the final draft as discussed in my initial comments.

[DanikVitek]

I might also add the sync_signature meta tag for functions to allow annotating sync functions with attributes, unique to them. For example, doc comments should be somewhat different for sync and async functions

[DanikVitek]

@DanikVitek this looks very interesting. I'll want to make sure the CI runs pass before merging and also for you to add the #[cfg(...)] upgrades that you are planning.

Let me know how I can help.

I need to add new features behind feature flags?

[scouten-adobe]

Now that I think about it, what are the reasons anybody would want to change the return type of their async function relative to the sync function? The only real reason I can come up with is if they're using a custom Future implementation. But in that case, why would they use this crate, as the custom future requires to move the implementation elsewhere, or surround the whole function body with an async move {...} block?

We haven't encountered that situation yet, but I wouldn't completely rule it out. Possible the return type could be something that implements a different type in sync vs async worlds. We do have that situation routinely for function parameters.

[scouten-adobe]

This is a really impressive change and looks very well thought out.

This takes me beyond my knowledge level for Rust's proc-macros. If we merge this, would you be willing to join me as a co-maintainer of this crate?

[scouten-adobe]

What was the issue with newer versions of trybuild?

[DanikVitek]

it required a higher MSRV. I wanted to lower the MSRV to 1.67

[DanikVitek]

This is a really impressive change and looks very well thought out.

This takes me beyond my knowledge level for Rust's proc-macros. If we merge this, would you be willing to join me as a co-maintainer of this crate?

Sure, I'd be happy to

[scouten]

@DanikVitek I've sent you an invite to collaborator status. Looks like there are a few Clippy and rustfmt details to sort out before we merge this, but once the tests run, feel free to proceed.

[dj8yfo]

i'll post 2 suggestions which appear reasonable to me, though they might not be reasonable at all and totally fine to be ignored:

1. using the following instead of impl_fut (cargo fmt appears not to react to the newline and not to attempt to fold everything into 1 line)

    #[async_generic(
        async_signature = 
            fn <'a, W: AsyncWrite>(&'a self, writer: &'a mut W) -> impl Future<Output = Result<()>> + Send + 'a
    )]
    ...
    #[async_generic(
        async_signature =
            async fn <'a, W: AsyncWrite>(&'a self, writer: &'a mut W) -> Result<()>
    )]

2. using something more wild for if _sync { ... } else { ... } expressions, like:

@if_sync_async_expr [
    _sync { 
        /// ....
    } _async {
        /// ...
    } 
]

or smth else of that kind, so that it's clearly visible that it's special proc-macro magic and not regular rust syntax.
This way imo it would be more justified to use this arbitrarily as long as it can be parsed, due to the barrier with regular syntax having been established.

[DanikVitek]

1. using the following instead of impl_fut (cargo fmt appears not to react to the newline and not to attempt to fold everything into 1 line)

    #[async_generic(
        async_signature = 
            fn <'a, W: AsyncWrite>(&'a self, writer: &'a mut W) -> impl Future<Output = Result<()>> + Send + 'a
    )]
    ...
    #[async_generic(
        async_signature =
            async fn <'a, W: AsyncWrite>(&'a self, writer: &'a mut W) -> Result<()>
    )]

@dj8yfo @scouten regarding this, I've came up with 5 "modes" alongside the impl_fut one: async_fn (default), impl_fut, pin_box_fut, ready, pin_box_ready. These keywords indicate to the macro, which tupe of function body to generate for the async function:

• async_fn — simply inline the body;
• impl_fut — wrap the result into async move {} if there's at least one .await call;
• pin_box_fut — same as impl_fut, but additionally wrapped into Box::pin() call;
• ready — wrap the function body into the core::future::ready() call. If the function body is synchronous, this is better than async {};
• pin_box_ready — same as ready, but additionally wrapped into Box::pin() call.

I'm not sure how else to better indicate this niche behavior, but open for suggestions

[dj8yfo]

@DanikVitek well, then maybe just split it into
async_signature = fn ... , async_mode = impl_fut,
where async_mode should only be used if it's smth exotic and not the most frequent default. This way it'll also be clear that these 5 options are part of async_mode toggle documentation.
It's really quite hard to read when both are piled up in a single attribute async_signature[impl_fut]<'boom_some_lifetime...,

[dj8yfo]

Also _signature implies it includes method name too, maybe the name could be included too to explicitly show it's the same in the case of traits. In some other cases it may theoretically differ

[DanikVitek]

1. using the following instead of impl_fut (cargo fmt appears not to react to the newline and not to attempt to fold everything into 1 line)

    #[async_generic(
        async_signature = 
            fn <'a, W: AsyncWrite>(&'a self, writer: &'a mut W) -> impl Future<Output = Result<()>> + Send + 'a
    )]
    ...
    #[async_generic(
        async_signature =
            async fn <'a, W: AsyncWrite>(&'a self, writer: &'a mut W) -> Result<()>
    )]

@dj8yfo Also, the next question is how to assign attributes on the signatures? Like this:

#[async_generic(
    async_signature =
        /// docs
        #[cfg(feature = "async")]
        async fn <'a, W: AsyncWrite>(&'a self, writer: &'a mut W) -> Result<()>
)]

or the same way it was before:

#[async_generic(
    /// docs
    #[cfg(feature = "async")]
    async_signature =
        async fn <'a, W: AsyncWrite>(&'a self, writer: &'a mut W) -> Result<()>
)]

?
If the first variant is used, it becomes much more verbose to add attributes to an async function with the same parameters as the sync function

[DanikVitek]

And the function signature is tied to its mode, so async fn cannot be used with anything other than async_fn mode and vice versa

[dj8yfo]

And the function signature is tied to its mode, so async fn cannot be used with anything other than async_fn mode and vice versa

this can be enforced by a diagnostic, and the user may experiment with wrong combinations and learn about it from diagnostic

[dj8yfo]

or the same way it was before:

#[async_generic(
    /// docs
    #[cfg(feature = "async")]
    async_signature =
        async fn <'a, W: AsyncWrite>(&'a self, writer: &'a mut W) -> Result<()>
)]

?

this looks better, but maybe you could consider splitting this into async_variant_attributes, smth like:

#[async_generic(
     async_variant_attributes = (
         #[doc = "....."]
         #[cfg(feature = "async")]
    ), 
    async_signature =
        async fn <'a, W: AsyncWrite>(&'a self, writer: &'a mut W) -> Result<()>
)]

This way it could be also mentioned on the async_variant_attributes toggle doc that it replaces all attributes on the method/function of the sync variant, which is similar to current behaviour, i believe.

[scouten]

My sincere apologies for letting this linger for a long time. I have some split loyalties here – I'd like to take this change, but I also realize that I need to keep this simple enough that I can rely on and maintain this crate for use by my team (see https://crates.io/c2pa).

For that reason, I've decided not to merge this PR. I'd like to invite you to take the work done in this PR into a new more advanced generic crate. Please use any existing code from async_generic with my blessing.