async and sync in the same program

[sunfishcode]

I maintain some libraries that have sync and async users. The async users wrap calls in spawn_blocking, which works, but isn't ergonomic and may be suboptimal. maybe_async looks like it could potentially be (part of) an alternative approach.

If I read the docs right, maybe_async can support both sync and async users, but by using a cargo feature, it can't support both in the same program. If the library gets used in multiple places in the same program, and one of them enables is_sync, it would break the other places that need async.

It seems like it would be useful to have a macro that expands to both sync and async, using different names, so that they can coexist. For example,

#[maybe_async::both]
async fn foo() -> bool {
    true
}

might expand to:

#[cfg(feature = "async")]
async fn async_foo() -> bool {
    true
}
#[cfg(feature = "sync")]
fn sync_foo() -> bool {
    true
}

so that users can call whichever form they need by name (though I'm not attached to the particular naming convention here).

[fMeow]

Ruyt 1.51 introduce a new resolver in cargo, which will compile crate for each set of different feature gates on dependencies.

With this v2 resolver, this issue is automatically solved. Just add the following lines in your Cargo.toml:

[package]
resolver = "2"
# Or if you're using a workspace
[workspace]
resolver = "2"

Plus, it doesn't make sense to compile both async and sync in the same program. Why use a blocking code when you can use async version?

[scouten]

Building on what @sunfishcode wrote, I have a trait in one of my crates (sorry, source is not public at this time) that is used in another of my programs in both sync (local file system) and async (contacting a remote service) contexts. For the local file system part of my program, I'd really rather not take on the complexity of async.

Please reopen and reconsider.

[fMeow]

I think the new resolver can help when one of dependencies use a different feature(blocking or async). I haven't tested it yet, but I think it should work.

And regarding the case of providing async and blocking at the same time, I don't really get it. When the program is able to run async code, it is natural to stick with async implementation. And when async executor is not available, it's ok to use blocking version as a fallback.

I have actually tried writing a [new procedural macro]
(https://github.com/fMeow/amphi) to duplicate the code turned the copied ones to blocking code on another module. But my implementation make it really hard to maintain and debug. It deprives compiler from the ability to tell us which lines goes wrong.

[marioortizmanero]

Unfortunately no. The recommendation in the documentation is wrong:

RECOMMENDATION: Enable resolver ver2 in your crate, which is introduced in Rust 1.51. If not, two crates in dependency with conflict version (one async and another blocking) can fail complilation.

Because the resolver v2 only fixes the conflicts in very specific cases (https://doc.rust-lang.org/cargo/reference/resolver.html#feature-resolver-version-2), none of which match this one:

• Features for target-specific dependencies are not enabled if the target is not currently being built.
• Features enabled on build-dependencies or proc-macros will not be unified when those same dependencies are used as a normal dependency.
• Features enabled on dev-dependencies will not be unified when those same dependencies are used as a normal dependency, unless those dev-dependencies are currently being built.

I've also made a repo to try it myself: https://github.com/marioortizmanero/resolver-v2-conflict

We're trying to fix this at rspotify (ramsayleung/rspotify#222) and we've ended up at the same conclusion. maybe_async should work with both sync and async.

[marioortizmanero]

Also, I think it might be best to expand with modules:

#[maybe_async::both]
async fn foo() -> bool {
    true
}

to this:

#[cfg(feature = "async")]
mod impl_async {
    pub async fn foo() -> bool {
        true
    }
}

#[cfg(feature = "sync")]
mod impl_sync {
    pub fn foo() -> bool {
        true
    }
}

So that it's just easier to use and more flexible overall. Though the only way I can think of to make it work without duplicate module names is something like:

maybe_async! { // should only be called once per namespace
    async fn a() {}
    async fn b() {}
}

Which would generate:

mod impl_sync {
    fn a() {}
    fn b() {}
}
mod impl_async {
    async fn a() {}
    async fn b() {}
}

At the cost of slightly changing the semantics of this macro. Not sure if it's worth it.

[marioortizmanero]

It should also be the default behavior because the current one is just wrong; no need to use maybe_async::both. I will try to see if I can implement this myself in https://github.com/marioortizmanero/maybe-async-rs (but I don't promise anything, I'll be leaving in a couple days)

[marioortizmanero]

After attempting to implement this I've come to the conclusion that it's far from trivial (I kinda saw it coming).

• maybe_async should work for structs as well. Sometimes you can't just add _sync/_async to every single method in your struct for usability reasons. It might be wiser to just have ThingSync::method over Thing::method_sync at some point.

• Two traits would be generated now, like TraitSync and TraitAsync, which means errors about duplicate trait methods available, because both of them expose the same function names. Try to compile this example yourself: https://github.com/marioortizmanero/maybe-async-rs/blob/main/examples/service_client.rs

  error[E0034]: multiple applicable items in scope
    --> examples/service_client.rs:55:15
     |
  55 |         Self::post("http://correct_url4create", String::from("my_bucket")).await
     |               ^^^^ multiple `post` found
     |
  note: candidate #1 is defined in an impl of the trait `InnerClientSync` for the type `ServiceClient`
    --> examples/service_client.rs:16:11
     |
  16 |     async fn post(url: Url, data: String) -> Response {
     |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  note: candidate #2 is defined in an impl of the trait `InnerClientAsync` for the type `ServiceClient`
    --> examples/service_client.rs:16:5
     |
  16 |     async fn post(url: Url, data: String) -> Response {
     |     ^^^^^^^^^^^^^
  help: disambiguate the associated function for candidate #1
     |
  55 |         InnerClientSync::post("http://correct_url4create", String::from("my_bucket")).await
     |         ^^^^^^^^^^^^^^^^^^^^^
  help: disambiguate the associated function for candidate #2
     |
  55 |         InnerClientAsync::post("http://correct_url4create", String::from("my_bucket")).await
     |         ^^^^^^^^^^^^^^^^^^^^^^
  

  The only solution I can come up with is to have the macro replace Self, StructName and TraitName for TraitNameSync/TraitNameAsync. Same for self, but it would have to be treated differently and converted to TraitNameSync::method(self, ...), for example.

  Thing is, even this isn't easy because self/Self doesn't necessarily mean it's calling a method from the trait. So the user would be forced to explicitly use TraitName::method(self, ...) instead of self.method(...). That way the macro can easily replace it with its sync/async variant. Problem: that looks terrible.

TLDR: processing functions in the macro works well because it's as easy as adding the suffix _async/_sync, but when it comes to traits it gets complicated because of conflicts.

So yeah, I'm stuck. The only way to do this may be appending _sync or _async.

[marioortizmanero]

@fMeow, can you add a warning about this library to the docs until this is worked on?

[fMeow]

Sure, I will update the doc in both readme and documentation in crate.io in the next minor release.

[marioortizmanero]

Update: we can actually do the duplicate traits thing if we also duplicate the struct that implements it. So instead of having a single struct that implements both the async and the sync traits, which is ambiguous, we have two structs, one for each trait.

I've got it working more or less in my fork, but I only modified the both macro (previously maybe_async) and it's not complete. It's enough to get these examples to compile:

• This is how maybe_async would be used: https://github.com/marioortizmanero/maybe-async-rs/blob/main/examples/service_client.rs. It compiles with any combination of is_sync and is_async.
• What it would expand to when the user enables both sync and async: https://github.com/marioortizmanero/maybe-async-rs/blob/main/examples/service_client_both.rs (You can see what's actually automatically generated by running cargo expand --example service_client --all-features > expanded.rs)

So basically, maybe_async::both will now work for:

• Functions: appending _sync/_async
• Structs and enums: appending Sync/Async
• Traits: appending Sync/Async
• Structs and enums impl blocks: appending Sync/Async
• Trait impl blocks: appending Sync/Async to the trait name, to the self type (the implementor), or to both. This could be configured with parameters.

We could later add parameters to configure the suffix (suffix = "_Async"), or to configure the entire name (async_name = "Stream", sync_name = "Iter").

What do you think? Do you like it? Is it enough for everyone's use cases? I know it would for rspotify myself.