Consider writing return types from the perspective of the user instead of the implementor

[Nemo157]

From this comment you say:

[...] one thing I've tried to strive for as much as possible is that if a function has -> T then return t: T works anywhere in the function [...]

while I do appreciate that as a decent goal, I think I would prefer if the function signature was written to match the intended signature. If we take the example from the readme, instead of the current:

#[async]
fn fetch_rust_lang(client: hyper::Client) -> io::Result<String> {
    ...
}

this would mean having

#[async]
fn fetch_rust_lang(client: hyper::Client) -> impl Future<Item=String, Error=io::Error> {
    ...
}

(the ergonomics here could probably be improved in the future via Trait aliases).

The main downside of this is, as you say, the type allowed to be passed to return is no longer actually declared anywhere (I assume it would either have to be a matching Result, or potentially any type implementing a matching Try).

The upside for me is that I'm always designing an API first from the side of the consumer, so I might first write something like (ignoring the fact this fails currently, I hope some combination of never_type and future impl Trait improvements could make this work)

fn fetch_rust_lang(client: hyper::Client) -> impl Future<Item=String, Error=io::Error> {
    unimplemented!("TODO")
}

If I can then implement it as a small iterator adapter then everything's fine, I just replace the body and it's done. If it takes a little more implementation work then maybe I want to use #[async], and suddenly I've got to change the signature

+#[async]
+fn fetch_rust_lang(client: hyper::Client) -> io::Result<String> {
+    ...
-fn fetch_rust_lang(client: hyper::Client) -> impl Future<Item=String, Error=io::Error> {
-    unimplemented!("TODO")
 }

---

Potentially if the signature specifies the final type then it may be possible to have a single macro that returns either a Future or Stream based on the return type

---

Finally, specifying the final signature also has more consistency with other languages:

C#:

async Task<int> AccessTheWebAsync()
{
    HttpClient client = new HttpClient();
    string urlContents = await client.GetStringAsync("http://msdn.microsoft.com");
    return urlContents.Length;
}

TypeScript%3B%0D%0A%7D%0D%0Aasync%20function%20foo()%3A%20Promise%3Cnumber%3E%20%7B%0D%0A%20%20%20%20await%20delay(100)%3B%0D%0A%20%20%20%20return%205%3B%0D%0A%7D):

async function foo(): Promise<number> {
    await delay(100);
    return 5;
}

Dart:

If the expression being returned has type T, the function should have return type Future<T> (or a supertype thereof). Otherwise, a static warning is issued.

Hack:

async function curl_A(): Awaitable<string> {
  $x = await \HH\Asio\curl_exec("http://example.com/");
  return $x;
}

---

Although there is one language that I think counts more towards the current side:

Scala:

On the level of types, async and await are methods with simple, intuitive types:

def async[T](body: => T): Future[T]
def await[T](future: Future[T]): T

[Nemo157]

One additional step that could be taken with this syntax is to have the body behaving something like the proposed "throwing functions". That would give a full rewrite of the README example something similar to:

#[async]
fn fetch_rust_lang(client: hyper::Client) -> impl Future<Item=String, Error=io::Error> {
    let response = await!(client.get("https://www.rust-lang.org"))?;
    if !response.status().is_success() {
        throw io::Error::new(io::ErrorKind::Other, "request failed");
    }
    let body = await!(response.body().concat())?;
    String::from_utf8(body)?
}

using throw for returning an error and returning the unwrapped String directly at the end.

[rushmorem]

Things I like about this approach:-

• This should fix https://github.com/alexcrichton/futures-await/issues/11 just as async_block works fine right now. I think this is HUGE.
• I see you are using the ? operator in your body. Unless we do some serious macro voodoo in the async macro (which I doubt @alexcrichton would approve :smile:), unless I'm missing another way of achieving this, we would have to implement std::ops::Try for futures and streams, which means https://github.com/alexcrichton/futures-await/issues/3#issuecomment-322419631 will become a thing as well (edit: after playing around with this idea, I realised that this is actually a technical argument for keeping await!(...)? because that way we won't have to implement Try for every type that implements Future or Stream otherwise we would have to move these traits into the compiler just to impl<F: Future> Try for F and impl<S: Stream> Try for S).

Things I don't like:-

• I'm not a big fan of that "throwing functions" proposal. I like the way errors are currently handled in Rust. The only improvement I hope to see is [derive(Error)] in the std library.
• Directly returning a type (String in this case) that is not actually an implementation of a Future like what the return type says will probably take getting used to and that will probably confuse new users quite a bit. I know we are simply returning Future::Item here but that's not how we write and read other Rust code.

Having said that, if we could fix those issues I highlighted in the things I like using Result as a return type then I will prefer the current approach. While it does have a slight drawback of having Result as a return type where as the final function would actually be a Future or a Stream, it's easy to map these return types mentally. For me the huge benefit is that it optimizes for writing code.

EDIT: I actually quite like the function signature. What I don't like is how the body is being implemented. I would prefer using futures::future::{ok, err} the way we use the std library's Ok and Err instead. That is

// Just putting this here so you can see where the Ok and Err in the body are coming
// from but this will be imported by the `async` macro so these won't interfere with
// your other code 
use futures::future::{ok as Ok, err as Err};

#[async]
fn fetch_rust_lang(client: hyper::Client) -> impl Future<Item=String, Error=io::Error> {
    let response = await!(client.get("https://www.rust-lang.org"))?;
    if !response.status().is_success() {
        return Err(io::Error::new(io::ErrorKind::Other, "request failed"))
    }
    let body = await!(response.body().concat())?;
    let string = String::from_utf8(body)?;
    Ok(string)
}

[Nemo157]

I've just pushed a branch implementing the first part of this for comparison purposes; including supporting returning both impl Future and impl Stream using a single macro: https://github.com/Nemo157/futures-await/tree/alternate-signatures

It does lose support for returning Boxed variants (might be possible to support, but I'm not very good with whatever sort of trait based trickery might be used to detect whether or not the return type is Box). It also almost certainly has terrible error messages if it fails to compile.

[Nemo157]

@rushmorem my second post there is really a potential extension that I'm not all that confident of either. I don't think it would actually be implementable via macro today (do catch + rewriting all returns might get 99% of the way there, but as far as I know you can't expand macros in the body yet so you'd miss any returns hidden behind them). I didn't really spend much time thinking about it, just realised since there was going to be some sort of breakage between the signature and body that it's the sort of space that the "throwing functions" proposal could slide into.

My current implementation would basically end up looking like what you wrote without the use, it's still going via Result internally instead of using future adapters.

[rushmorem]

I've just pushed a branch implementing the first part of this for comparison purposes; including supporting returning both impl Future and impl Stream using a single macro: https://github.com/Nemo157/futures-await/tree/alternate-signatures

Awesome! I will try to play with that tomorrow.

It does lose support for returning Boxed variants (might be possible to support, but I'm not very good with whatever sort of trait based trickery might be used to detect whether or not the return type is Box).

Instead of trying to detect it, you could keep the boxed argument and use that instead.

It also almost certainly has terrible error messages if it fails to compile.

You are probably right. Personally, I try to avoid Boxing as much as possible (because of the allocation) but it does have valid use cases so I think it will be good to keep.

My current implementation would basically end up looking like what you wrote without the use, it's still going via Result internally instead of using future adapters.

In my example I was using the futures adapters to show that you could still write code that returns Futures using the current style. However, I was hoping that the code would actually be able to return any implementation of a Future when returning impl Future<...> and any implementation of a Stream when returning impl Stream<...>. So code like

#[async]
fn fetch_rust_lang(client: hyper::Client) -> impl Future<Item=String, Error=io::Error> {
    // While this example is so trivial that you don't really need to use the `async`
    // macro, we are only interested in what is being returned. Pretend that there
    // is lots of other code above this line that `async` helps you write more easily.
    client.get("https://www.rust-lang.org")
}

would still be valid. If we manage to achieve this, then we would be able satisfy @alexcrichton's goal of being able to return anything that's in the declared return type at any point in the body. Even more importantly, I think, returning Futures and Streams directly would remove any need to await at the end of the function (so we can get a Result). This is a performance win when you want to use the output of that function inside other async code.

Here, is an example. Correct me if I'm wrong, but you can't implement the example above currently. You will be forced to write the following instead

#[async]
fn fetch_rust_lang(client: hyper::Client) -> io::Result<String> {
    // We are forced to await here even though we don't have to.
    // This will block the coroutine in any `async` code that use this
    // function, even though we don't actually await the invocation
    // of this function.
    await!(client.get("https://www.rust-lang.org"))
}

[rushmorem]

As you can probably tell from my RFC above, I really like the direction you are headed with this. One thing that's been bugging me even from your other experiment in https://github.com/alexcrichton/futures-await/issues/7 is using a macro to yield elements. Having read your code, I totally understand why you are doing this. However

yield item;

is easier to read than

stream_yield!(item);

If we can't easily implement this in a way that would get rid of the macro, we can resort to rewriting yield item; to yield Async::Ready(item); in the async macro itself. You don't have to implement this yourself if you don't have enough time to do so. You can simply add a TODO linking to this comment as a comment on the stream_yield! macro as this can be done in a separate pull request.

[alexcrichton]

Thanks for the issue here @Nemo157! I actually thought what's currently implemented was how most other languages do it as it seemed the most intuitive to me, so I'm surprised to hear the opposite!

I'm still somewhat wary myself though of writing down a return of a future vs a return of a Result. I'm afraid that returning a future will encourage you to write down what future you're returning as opposed to impl Future, and rationalizing that could get... interesting. Also the loss of boxed futures is sort of sad as they're required for traits and object safety today!

@rushmorem I'm not sure I understand what you say though about how this would fix https://github.com/alexcrichton/futures-await/issues/11? Is there something we can't do today to do that?

[rushmorem]

I'm not sure I understand what you say though about how this would fix #11? Is there something we can't do today to do that?

I didn't mean to imply that it can't be fixed otherwise. It's just that I thought that issue will automatically be fixed if we use the return type like this directly since a return type like impl Future<Item=T, Error=E> + 'a would be used directly as is without trying to detect the lifetime parameters separately.

I'm still somewhat wary myself though of writing down a return of a future vs a return of a Result. I'm afraid that returning a future will encourage you to write down what future you're returning as opposed to impl Future, and rationalizing that could get... interesting.

@alexcrichton What do you think about being forced to await!() at the end of the function if you have a future rather than a result at the end of your function as explained in https://github.com/alexcrichton/futures-await/issues/15#issuecomment-327037927?

[alexcrichton]

@rushmorem

would be used directly as is without trying to detect the lifetime parameters separately

Ah ok, makes sense!

@alexcrichton What do you think about being forced to await!() at the end of the function if you have a future rather than a result at the end of your function as explained in #15 (comment)?

I agree it's unfortunate!

Despite this, though, I feel like working with futures has proven to be "difficult enough" that I'd want to make the concepts here as streamlined as possible. In the sense that if we've got little gotchas like you can return Ok(..) here but you can also return t: impl Future here it may add to the confusion early on and only end up helping power users?

I'm not entirely sure how to balance this :(

[Nemo157]

I've re-added boxed futures/streams by assuming any non-impl Trait return type must be a Box. It seems like there must be a better way to detect it but I wasn't able to come up with one.

[rushmorem]

@alexcrichton

Despite this, though, I feel like working with futures has proven to be "difficult enough" that I'd want to make the concepts here as streamlined as possible. In the sense that if we've got little gotchas like you can return Ok(..) here but you can also return t: impl Future here it may add to the confusion early on and only end up helping power users?

I'm not entirely sure how to balance this :(

I totally understand your position and I actually feel the same way. As you can see from my comments in https://github.com/alexcrichton/futures-await/issues/15#issuecomment-326772627 and https://github.com/alexcrichton/futures-await/issues/15#issuecomment-327037927 I have been arguing for returning only return t: impl Future from the word go. I'm actually strongly against also returning Result because if we don't return it then code like

#[async]
fn fetch_rust_lang(client: hyper::Client) -> impl Future<...> {
    // We are forced to await here even though we don't have to.
    // This will block the coroutine in any `async` code that use this
    // function, even though we don't actually await the invocation
    // of this function.
    await!(client.get("https://www.rust-lang.org"))
}

won't even compile because Result doesn't impl Future. This is one of the biggest reasons why I'm so excited about going in this direction.

Having said that, I would also like to reiterate that this approach is more flexible as it allows us to also write code that uses the result style as well using FutureResult rather than std::result::Result. You will notice that whenever I talk about returning Ok(...) or Err(...) when mentioning this approach I also make sure to point out that Ok and Err are actually futures combinators automatically imported by the async macro through use futures::future::{ok as Ok, err as Err};. You can see this in https://github.com/alexcrichton/futures-await/issues/15#issuecomment-326772627 and https://github.com/alexcrichton/futures-await/issues/20#issue-255144150. The example I have been using is

// Just putting this here so you can see where the Ok and Err in the body are coming
// from but this will be imported by the `async` macro so these won't interfere with
// your other code 
use futures::future::{ok as Ok, err as Err};

#[async]
fn fetch_rust_lang(client: hyper::Client) -> impl Future<Item=String, Error=io::Error> {
    let response = await!(client.get("https://www.rust-lang.org"))?;
    if !response.status().is_success() {
        return Err(io::Error::new(io::ErrorKind::Other, "request failed"))
    }
    let body = await!(response.body().concat())?;
    let string = String::from_utf8(body)?;
    Ok(string)
}

As you can see from the code above, only return t: impl Future is used because FutureResult is a Future. We can actually return any Future, not just FutureResult.

EDIT: IMHO, this is a zero-cost abstraction while also catering to both power users and normal users with compile time guarantees. It doesn't get any Rustier than this :smile:

[Nemo157]

@rushmorem I'm a bit confused about your example

#[async]
fn fetch_rust_lang(client: hyper::Client) -> impl Future<...> {
    // We are forced to await here even though we don't have to.
    // This will block the coroutine in any `async` code that use this
    // function, even though we don't actually await the invocation
    // of this function.
    await!(client.get("https://www.rust-lang.org"))
}

This doesn't involve blocking any more than any other future does, and modulo optimizer inlining of the Future shim + Generator this is identical to:

fn fetch_rust_lang(client: hyper::Client) -> impl Future<...> {
    client.get("https://www.rust-lang.org")
}

A caller can delay starting and run these both in parallel in the exact same way:

#[async]
fn fetch_rust_lang_twice(client: hyper::Client) -> impl Future<...> {
    await!(join_all([
        fetch_rust_lang(client.clone()),
        fetch_rust_lang(client)
    ]))
}

---

I was hoping that the code would actually be able to return any implementation of a Stream when returning impl Stream<...>

This wouldn't really work since an #[async] -> impl Stream function never returns a value, it only yields items and returns to signal stream completion.

---

we can resort to rewriting yield item; to yield Async::Ready(item); in the async macro itself.

That would work, and I'm tempted to do it. But it has the same issue I mentioned about rewriting return above: it will miss any yields in macros. That's actually important so that it doesn't rewrite the yield Async::NotReady; in the await! macro, but could introduce issues if any user wants to have a yield inside another macro for any reason.

---

There are a couple of issues with just returning any Future from an #[async] -> impl Future function:

• Unifying different Future implementations
  • Your example would work because your two returns are both returning FutureResult values. I feel like the #[async] syntax would exacerbate the already existing issue of trying to return multiple different Future implementations with the same Item and Error types. I would especially expect to see FutureResult used for error values a lot even when a different Future is used for the success value.
• Allowing usage of ? in the body
  • Again, your example could likely be made to work since #[async] can make the ? -> Result -> FutureResult flow work. If the user was returning any other kind of Future it would again hit an issue with trying to return multiple Future implementations (a more solvable one since we knows there's at most 2 types and can just wrap them in an Either, but that would require rewriting return nodes to wrap the user's return values...).

There have been proposals around "anonymous enums" that I feel could provide a relatively nice way to work around both those issues, but they're not even in RFC yet (/have been postponed) and would probably require variadic generic support as well, so...

Luckily there is a super easy way to just avoid the whole issue and come up with a single concrete Result type to return from any Future you have: await!.

[rushmorem]

@Nemo157 I see. Thanks for explaining this. If we are going to end up returning Result anyway then I prefer having Result as the return type.

[Nemo157]

Another thought I just had, this should also allow easy integration of nominal types once the new impl Trait type aliases are available. Something like

type RustLang = impl Future<Item=String, Error=io::Error>;
#[async]
fn fetch_rust_lang(client: hyper::Client) -> RustLang {
    ...
}

compared to requiring use of async_block! with the current implementation:

type RustLang = impl Future<Item=String, Error=io::Error>;
fn fetch_rust_lang(client: hyper::Client) -> RustLang {
    async_block! {
        ...
    }
}

(whether this is very useful or not, I'm undecided yet)

[Nemo157]

I finally got round to pushing an updated branch implementing this: Nemo157#alternate-signatures-2. Including tests showing that it fixes #11/#53 via the normal lifetime syntax.

It currently has some pretty terrible type error messages, but I have some ideas on how to fix those up.

[withoutboats]

Because of how impl Trait and trait objects handle lifetimes, this interacts very poorly with lifetime elisions. Specifically, the return type only captures explicitly called out lifetimes, but the returned generator necessarily captures all lifetimes.

So say you have a method like this:

#[async]
fn foo(&self, arg: &str) -> Result<i32, io::Error>

You'd have to write it like this:

#[async]
fn foo<'a, 'b>(&'a self, arg: &'b str) -> impl Future<Item = i32, Error = io::Error> + 'a + 'b

The alternative would be to make lifetime elisions not work the same way the work with regular impl Trait return types, but that seems clearly out to me, since the whole idea of this change is to make the return type accurately reflect with the function returns.

[esper256]

I think this is an extremely exciting development for Rust, but I would like to add that I find it (from my perspective as a heavy async user but Rust newbie) very distracting to have the async macro change the return type of the method.

I appreciate that the goal of async await is to make async code more ergonomic. While requiring a long form impl trait return type may be at odds with this goal in some respects I believe long term it will benefit users to have the actual return type visible in the code.

My reasoning is as follows: 1) The person who writes the #[async] method is the most likely to be familiar with how async works. Details such as whether or not it alters return types or not. The person consuming the method is more likely to be unaware of how #[async] works. For them, if the method signature is accurate from their point of view, they can use the method successfully without understanding generators, state machines or #[async] 2) It's less magic and follows the principle of least surprise 3) If the return type is long and unergonomic, and returning the impl Future is not chosen because of this reason, then Rust as a whole won't feel the appropriate pressure to make language choices such as trait aliases that might alleviate these issues.

The methods will already be sprinkled with await!()

There would be a certain symmetry to some kind of final macro that turns a concrete result into the correct future return type: finally!(result)

[Nemo157]

@withoutboats I actually like that part of this proposal because it makes #[async] behave more like other Rust code, if I were to see

#[async]
fn foo(&self, arg: &str) -> Result<i32, io::Error>

I would assume that foo only uses the references while it's running, and doesn't keep them bound after returning.

I would also hope that combined with some way to refer to the "elided lifetime" (e.g. eddyb's old Anonymous/placeholder lifetime RFC) the signature would not look anywhere near as bad as you show:

#[async]
fn foo(&self, arg: &str) -> impl Future<Item = i32, Error = io::Error> + '_

[withoutboats]

@Nemo157 your code example is equivalent to this (which would be a lifetime error):

#[async]
fn foo<'a, 'b>(&'a self, arg: &'b str) -> impl Future<..> + 'a

Note that the future is not bound 'b. This is because return type elision only considers the self argument.

I would assume that foo only uses the references while it's running, and doesn't keep them bound after returning.

It does use them while its running - while the future is running. This will be the natural way to think about things when the natural thing to do is to await the future.

[Nemo157]

Yep, I was misrembering how lifetime elision works. Still, I think that improving how impl Trait handles lifetimes would be a better path than hiding the lifetimes away in the macro/compiler expansion for this one specific case.

It does use them while its running - while the future is running. This will be the natural way to think about things when the natural thing to do is to await the future.

I definitely have places where retaining a distinction between the future's preparation phase and executing phase is useful to avoid borrowing data too long. I have been meaning to start a thread on irlo about "hot generators" to somehow allow this with the #[async] macro (currently it's possible with the async_block! macro, but that has some downsides). I have a feeling that attempting something like that would not interact well with self-borrowing generators though.

[Arnavion]

It does use them while its running - while the future is running. This will be the natural way to think about things when the natural thing to do is to await the future.

I definitely have places where retaining a distinction between the future's preparation phase and executing phase is useful to avoid borrowing data too long.

Specifically, there is an example in https://github.com/alexcrichton/futures-await/issues/11#issuecomment-326399783 that I've talked about with @withoutboats in #rust before.

[Nemo157]

Referring back to my old comment about nominal impl Future types, I just accidently attempted to use this today, so it's definitely something that would be useful (once abstract/existential types and generic associated types are both implemented):

pub trait CountDown {
    type Future<'a>: Future<Item = (), Error = !> + 'a;

    fn start(&mut self, count: Duration) -> Self::Future;
}

impl CountDown for Timer {
    abstract type Future<'a> = impl Future<Item = (), Error = !> + 'a;

    #[async]
    fn start(&mut self, count: Duration) -> Self::Future {
        ...
    }
}