-
Notifications
You must be signed in to change notification settings - Fork 55
Consider writing return types from the perspective of the user instead of the implementor #15
Comments
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 |
Things I like about this approach:-
Things I don't like:-
EDIT: I actually quite like the function signature. What I don't like is how the body is being implemented. I would prefer using // 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)
} |
I've just pushed a branch implementing the first part of this for comparison purposes; including supporting returning both It does lose support for returning |
@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 ( My current implementation would basically end up looking like what you wrote without the |
Awesome! I will try to play with that tomorrow.
Instead of trying to detect it, you could keep the
You are probably right. Personally, I try to avoid
In my example I was using the #[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 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"))
} |
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 #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 |
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 @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
@alexcrichton What do you think about being forced to |
Ah ok, makes sense!
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 I'm not entirely sure how to balance this :( |
I've re-added boxed futures/streams by assuming any non- |
I totally understand your position and I actually feel the same way. As you can see from my comments in #15 (comment) and #15 (comment) I have been arguing for returning only #[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 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 // 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 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 😄 |
@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 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)
]))
}
This wouldn't really work since an
That would work, and I'm tempted to do it. But it has the same issue I mentioned about rewriting There are a couple of issues with just returning any
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 |
@Nemo157 I see. Thanks for explaining this. If we are going to end up returning |
Another thought I just had, this should also allow easy integration of nominal types once the new type RustLang = impl Future<Item=String, Error=io::Error>;
#[async]
fn fetch_rust_lang(client: hyper::Client) -> RustLang {
...
} compared to requiring use of 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) |
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. |
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 |
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:
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: |
@withoutboats I actually like that part of this proposal because it makes #[async]
fn foo(&self, arg: &str) -> Result<i32, io::Error> I would assume that 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> + '_ |
@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
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 |
Yep, I was misrembering how lifetime elision works. Still, I think that improving how
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 |
Specifically, there is an example in #11 (comment) that I've talked about with @withoutboats in #rust before. |
Referring back to my old comment about nominal 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 {
...
}
} |
From this comment you say:
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:
this would mean having
(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 matchingResult
, or potentially any type implementing a matchingTry
).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 futureimpl Trait
improvements could make this work)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 signaturePotentially if the signature specifies the final type then it may be possible to have a single macro that returns either a
Future
orStream
based on the return typeFinally, specifying the final signature also has more consistency with other languages:
C#:
TypeScript:
Dart:
Hack:
Although there is one language that I think counts more towards the current side:
Scala:
The text was updated successfully, but these errors were encountered: