Skip to content

An async function declared to return FutureOr<T> can't directly return a T #606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jamesderlin opened this issue Oct 3, 2019 · 11 comments

Comments

@jamesderlin
Copy link

jamesderlin commented Oct 3, 2019

Currently declaring an async function as returning FutureOr<T> is allowed but seems misleading and useless (it's no different than returning Future<T>) except maybe in the case of overriding a method already declared to return FutureOr<T>. That is, if I have:

FutureOr<int> foo() async {
  return 42;
}

then foo() is Future<int> is true and and foo() is int is not, and the only way to extract the value is via await/then().

Motivation:

I want to have a function:

FutureOr<int> foo(bool doOptionalWork) async {
  // ... Do some preliminary work here...

  if (doOptionalWork) {
    await someAsyncOperation();
  }

  // ... Do some additional work here...
  return result;
}

with the expectation that calling foo(false) will return an int. (The intent is avoid forcing await/then() onto the caller.)

A workaround is to transform the code to avoid using async:

FutureOr<int> foo(bool doOptionalWork) {
  // ... Do some preliminary work here...

  int doAdditionalWork() {
    // ... Do some additional work here...
    return result;
  }

  if (doOptionalWork) {
    return someAsyncOperation().then((_) {
      return doAdditionalWork();
    });
  }

  return doAdditionalWork();
}

Could the compiler do such a transformation automatically?

@natebosch
Copy link
Member

natebosch commented Oct 3, 2019

seems misleading and useless (it's no different than returning Future<T>) except maybe in the case of overriding a method already declared to return FutureOr<T>

The important bit isn't if the async method is overriding an interface - it's whether it may be overridden elsewhere. If we force an async method to not return FutureOr then it can't be used any place where that signature is important, which is too limiting IMO.

Could the compiler do such a transformation automatically?

It would be breaking for a method marked async to sometimes start returning a synchronous value.

@jamesderlin
Copy link
Author

It would be breaking for a method marked async to sometimes start returning a synchronous value.

Oh, right, good point. In principle, I feel like it's fair game for a function declared to return FutureOr<T> to start returning a synchronous value.

In practice, since the current behavior is to always return a Future, I'd expect that most existing code would look like:

FutureOr<int> foo() async => 42;

var x = await foo();

and that code should still work even if foo() returned a synchronous value.

I think cases where it would break:

  • Code that explicitly checks the return type. (But since FutureOr<T> foo() async currently always returns a Future<T>, such checks in existing code either aren't useful or aren't doing what people expected.)
  • Code that explicitly casts the return type to Future<T>.

Are there others?

Anyway, even though it'd be breaking, I think it's still maybe worth considering for, say, Dart 3 or something. (dart-sync-async was breaking too.) Alternatively maybe we could add new syntax to opt-in to such behavior (e.g. FutureOr<T> foo() async?).

@natebosch
Copy link
Member

Are there others?

There could be places where the value flows through a references that is statically dynamic and then any method on Future (like .then) gets called on it.

As long as we only did this if the return type was declared a FutureOr the breakage would likely be minimal.

@lrhn
Copy link
Member

lrhn commented Oct 8, 2019

Side note: I very highly recommend never using a FutureOr type as a return type of your public API.

If you do, your API clients must always either check whether it is a value or a future, and have two code paths, or use await and always treat it as a future.
Your API ergonomics is better if you always give a Future out. Then users only need one code path, one which they will need anyway.
If the performance overhead of waiting for a future every time is too crippling, you should probably look for a completely different approach. If not, returning a value directly, only in a way where the client can't actually depend on that happening, is probably just premature optmization.

(It's fine to accept a FutureOr because then the extra work falls on you, not the clients of your API. You are in a position to decide whether you want that or not, your clients will have to deal with what you give them. So no covariantly occurring FutureOr in your API surface.).

@lrhn
Copy link
Member

lrhn commented Oct 8, 2019

As for the change to async functions: That will most likely not happen. It is a breaking change, even if it only applies to functions with a declared return type of FutureOr<something>.

The return type is not what determines the body's behavior, it's the async which means that the function will return a Future.

It would be highly confusing if return 2; returned a future in some cases and an int in other cases, which it has to do:

FutureOr<int> tricky() async {
  if (test) await something();
  return 2;
}

Here you want the function to return the integer 2 if the test is false, right?
If the test is true, the function has to return a Future because there is no ready result when the function's invocation returns. It means that the same return statement will either return a value or complete a future, depending on when it is invoked.
(It's not impossible, we already treat throws happening before the first await differently because they can't complete the future complete synchronously, so we could definitely recognize a return before the first await as not needing a completer, and if the function is FutureOr, just return the value. I just don't think it's predictable enough to be a good language feature).

@TimWhiting
Copy link

TimWhiting commented Oct 6, 2021

Hi @lrhn

What about something like an async block:

FutureOr<int> tricky() {
  if (test) async {
    await something();
  }
  return 2;
}

That way async semantics don't change, await still wraps the result of something() in a Future even if it has a non-future type, but this function doesn't have to be async, just the if block. If a function ever enters an async block it then returns a completer.future to the result of the function. If it never enters an async block, the function returns normally. I don't know how doable it is from a compiler standpoint, but that would be the semantics. Alternatively you could require an async block to return from the function on all code paths, if the proposed semantics are too hard to implement. Could also work with for loops / while loops, in which case if it ever enters the loop the function returns a future.

@lrhn
Copy link
Member

lrhn commented Oct 6, 2021

I don't see how that async block differs from just making the function async.
I guess it would be a compile-time error to have an async block inside an async function, or inside a function not returning FutureOr<T>, but it's not as visible as the actual async marker on async functions.

The return 2; can still happen asynchronously, so you have to be ready to expect a future. That means that the method body is an async body, the only difference is that if it manages to complete synchronously, without hitting any async or await, it can, perhaps, return the value directly without wrapping it in a future.

From a compiler perspecitive, it's fine. We already have to handle synchronous completions specially to avoid completing the returned future immediately. We could allow returning a value instead.

I just don't want to. Returning FutureOr is what I strongly discouraged doing above, so it's a hard sell to make me want a feature which does nothing else. Supporting returning FutureOr is a non-goal to me, because I think it's a bad API.

@rrousselGit
Copy link

Supporting returning FutureOr is a non-goal to me, because I think it's a bad API.

Why?

The alternative is relying on a custom implementation of Future to make Future.then synchronous, like SynchronousFuture from Flutter.
I think that's worse than FutureOr, which it explicit in that the function can execute synchronously.

@TimWhiting
Copy link

TimWhiting commented Oct 6, 2021

Side note: I very highly recommend never using a FutureOr type as a return type of your public API.

If you do, your API clients must always either check whether it is a value or a future, and have two code paths, or use await and always treat it as a future.

Part of the proposal is to make it easier for the API clients to make this check without having to use .then.

FutureOr<int> transformResult() {
  final someFutureOr = getSomethingFromAPIOrCache();
  if (someFutureOr is Future) async {
    return transform(await someFutureOr());
  }
  return transform(someFutureOr);
}

(It's fine to accept a FutureOr because then the extra work falls on you, not the clients of your API. You are in a position to decide whether you want that or not, your clients will have to deal with what you give them. So no covariantly occurring FutureOr in your API surface.).

Handling a FutureOr as extra work on the library is fine, and we can work around checking the types and using .then the problem is that accepting a FutureOr Function() from a client means you expect them to know how to create a FutureOr, which many users will always pass in an async function not realizing that in order to return anything synchronously they have to opt for a non-async function with .then.

For example users expect:

callFunctionThatTakesFutureOrFunction(() async {
  if (condition) {
    return 2
  }
  return await something();
});

To return synchronously when condition is true, since it obviously does not need to be a Future.

@rrousselGit
Copy link

This could also link with #1874 which could allow macros to implement a custom await keyword

Like:

FutureOr<T> futureOr;

T result =  @maybeAwait(futureOr);

which decomposes into:

FutureOr<T> futureOr;

T result =  futureOr is T ? futureOr : await futureOr;

@lrhn
Copy link
Member

lrhn commented Oct 6, 2021

Part of the proposal is to make it easier for the API clients to make this check without having to use .then.

I'd recommend just always doing await if you get a FutureOr<X>. Don't bother trying to get a synchronous path through a potentially asynchronous function, just accept that potentially asynchronous just means asynchronous. The only reason to try for a synchronous path is performance. If that performance is your problem, you shouldn't be using asynchrony at all, or you should have two separate paths.

the problem is that accepting a FutureOr Function() from a client means you expect them to know how to create a FutureOr

No. I expect them to pass either a function returning Future<T> or, as a convenience, a function returning T (which I'll probably treat exactly as a Future<T>), just so they won't need to wrap the value in a future themselves. The goal was never to have a synchronous behavior, it was to help the client by doing the future-wrapping for them, and allowing them to use an existing non-asynchronous function instead of having to write a new function which returns a future with the same value.

Mixing synchronous and asynchronous computations in a way that preserves synchronousness was, and is, not a goal.

(I guess I'm saying that I don't agree with the goal this request, and making it a language feature that you can create a FutureOr returning function that can use await and also return a value synchronously, is not something I want for the language because it's not functionality I'd ever recommend using. That said, don't me stop you from trying to design it anyway. Maybe I'm missing something.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants