Skip to content
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

Add FutureOr<T> #28006

Closed
7 tasks done
floitschG opened this issue Dec 6, 2016 · 15 comments
Closed
7 tasks done

Add FutureOr<T> #28006

floitschG opened this issue Dec 6, 2016 · 15 comments
Assignees
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). P1 A high priority bug; for example, a single project is unusable or has many test failures
Milestone

Comments

@floitschG
Copy link
Contributor

floitschG commented Dec 6, 2016

Informal specification: https://gist.github.com/lrhn/d6815f9556acb3dc8f91d2c680ea0e54

A lot of asynchronous code in Dart allows the use of a T or a Future<T>. For example, the callback to Future.then is declared to take a T but it doesn't specify the return type, since it could be an S or a Future<S>:

Future<S> then<S>(onValue(T value), { Function onError });

We will add FutureOr<T> to support this use case. In strong mode and DDC FutureOr<T> represents the union of Future<T> and T. In Dart1, FutureOr<T> is interpreted as dynamic.

In a following release:

@floitschG floitschG added area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). labels Dec 6, 2016
@floitschG floitschG added this to the 1.50 milestone Dec 6, 2016
@floitschG
Copy link
Contributor Author

Assigning to @lrhn so he can update the description with more details.

@bwilkerson
Copy link
Member

We will add FutureOr<T> to support this use case.

Does that mean that there will be an explicit class defined in the SDK?

@floitschG
Copy link
Contributor Author

We are open for suggestions. The type/class is magical, since it's a union type.

We can add a class or a typedef (for example to simplify dartdoc generation), but the tools would need to treat it specially.

Conceptually, FutureOr<T> lives in dart:async.

@bwilkerson
Copy link
Member

If we have an explicit declaration, then we don't need to special case navigation (there's someplace with documentation we can take the user), hover text, completions (we don't have to insert a fake class name into the list) and probably a few other features I'm not thinking of immediately. So, it would be much cleaner from server's perspective if there were an actual class, no matter how different the semantics of that class are.

Given that it isn't the name of a function, class seems like a better choice than typedef to me.

@floitschG
Copy link
Contributor Author

Sounds reasonable. I created a bug for it.
(Hopefully today or tomorrow)

@a-siva
Copy link
Contributor

a-siva commented Dec 8, 2016

Could we add some tests cases for this so that the implementations can be verified.

@floitschG
Copy link
Contributor Author

I started writing some tests:
https://codereview.chromium.org/2564563003/

@mit-mit
Copy link
Member

mit-mit commented Dec 15, 2016

Informal specification is done: https://gist.github.com/lrhn/d6815f9556acb3dc8f91d2c680ea0e54

Moving to 1.22 for implementation.

@crelier
Copy link
Contributor

crelier commented Jan 3, 2017

The current language spec has a special type rule when returning a value from an async function. See section "17.12 Return":

If the body of f is marked async (9) it is a dynamic type error if o is
not null (16.2) and Future<S> is not a subtype of the actual return type
(19.8.1) of f.

Is any of this changing with the introduction of FutureOr<T>?
The VM supports this special type rule in checked mode, which is very similar to what would be needed to support the FutureOr union type prescribed here, and I am wondering whether it would not be simpler to support FutureOr in the VM, rather than mapping it to dynamic, especially since this mapping is not specified at all (e.g. how do you map FutureOr<Malformed>? or FutureOr<U, V>? etc...).

@lrhn
Copy link
Member

lrhn commented Jan 4, 2017

No change for the VM's checked mode, for two reasons.

One is that it's a requirement on the dynamic type, and there are no objects that has type FutureOr<X>. It's still ether a future or a non-future, so there is no change to the dynamic type of o.

Also, in spec mode, any occurrence of FutureOr should be treated as dynamic, so even if the return type of the async function is written as FutureOr<X>[1], then it's effectively just returning dynamic, so there is no need to check anything.

So all in all, no matter where FutureOr occurs, it doesn't make a difference to this rule, because it won't occur anyway, it's just dynamic.

For strong mode, where FutureOr doesn't mean dynamic, there shouldn't be a change as such, but Future<S> is a subtype of FutureOr<S>, so you can get away with writing FutureOr<S>foo() async { ... }. You just shouldn't.
There is the question of what Flatten(FutureOr<S>) is, and if it wasn't because I want to get rid of flatten, we would probably have to extend it so that Flatten(FutureOr<S>) := Flatten(S).

[1] It shouldn't be, it should just be Future! We should have a lint for that!

@mit-mit
Copy link
Member

mit-mit commented Jan 17, 2017

Marking informal spec and dart2js complete

@mit-mit
Copy link
Member

mit-mit commented Jan 17, 2017

Marking VM done.

@mit-mit mit-mit added P1 A high priority bug; for example, a single project is unusable or has many test failures and removed P1 A high priority bug; for example, a single project is unusable or has many test failures labels Jan 19, 2017
@mit-mit
Copy link
Member

mit-mit commented Jan 19, 2017

Updating checklist to mention that core library usage of this will follow in 1.23.

@mit-mit
Copy link
Member

mit-mit commented Jan 24, 2017

Changelog underway: https://codereview.chromium.org/2648203003/

@kevmoo
Copy link
Member

kevmoo commented Jan 30, 2017

Landed! c3f1212

@kevmoo kevmoo closed this as completed Jan 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests

7 participants