Skip to content

Future.wait casting is not solved by Records #2303

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

Closed
srawlins opened this issue Jun 23, 2022 · 8 comments
Closed

Future.wait casting is not solved by Records #2303

srawlins opened this issue Jun 23, 2022 · 8 comments
Assignees
Labels
patterns Issues related to pattern matching. state-duplicate This issue or pull request already exists

Comments

@srawlins
Copy link
Member

One of the five paragraphs under Motivation is about Future.wait:

https://github.com/dart-lang/language/blob/df9574472a7a84aa518b88794c5ec9ff6349f170/working/0546-patterns/records-feature-specification.md

You've probably noticed this if you've ever used Future.wait() to wait on a
couple of futures of different types. You end up having to cast the results back
out since the type system no longer knows which element in the returned list has
which type.

As far as I can tell, Records cannot solve this for the existing generic Future.wait. There is not a new signature we can use for Future.wait that would return a record, for example. (You could define a nice Future<(T0, T1)> wait2(Future<T0> f0, Future<T1> f1), wait3, wait4, etc. though.)

It looks to me like this is more directly solved with the cast binder pattern:

var [a as int, b as String] = await Future.wait([aFuture, bFuture]);

Not sure what the AI is here; perhaps the motivating example could be moved to the "cast binder" pattern section...

@srawlins srawlins added the patterns Issues related to pattern matching. label Jun 23, 2022
@lrhn
Copy link
Member

lrhn commented Jun 23, 2022

If (when!) we don't allow abstracting over pattern width, you can't get a single typed wait that handles different widths.
So yes, we'd need wait2, wait3, ..., wait22.

HOWEVER (use all the emphasis!), we can decide to change the await operator to work specially on tuple types.
Since awaiting a non-future value is pointless anyway, and we don't have any existing code containing records,
we can safely say that await record will effectively pointwisely await every component of the record, and evaluate to the result of awaiting a Future<recordType'> with the same structure as the original, and element types being the flatten of the original element types.

That's actually a generalization of await nonFutureValue and await futureValue, treating those as one-tuples.

Should it work only for an expression with a static record type, or can it also work on await expressionOfTypeObject?
It's probably safe to let it work based on the runtime type, we won't introduce a record structure which doesn't already exist in the program (which is something I care about for making implementations efficient). It does mean that await (o as Object) will need to do introspection on the object, but it already had to, checking if it's actually a future. Recognizing a record, then awaiting each elements seems doable by native code with access to the record's internal structure.

We'd have to define the behavior in case any component future completes with an error.
I'd probably go with throwing a RecordAwaitException containing the original cause (or causes?) and a record with the same structure as the expected result, but nullable types, and null values for every operation which threw. That gives you access to the values that did complete, if you need them.

@munificent Do this! We want that to be the behavior from the start, it's hard to add later, if await record is already valid and does nothing.
The alternative, to initially disallow await recordType so that we can later introduce the feature if we want to, is problematic. Especially if await ((future1, future2) as Object) will throw.

@munificent
Copy link
Member

we can safely say that await record will effectively pointwisely await every component of the record, and evaluate to the result of awaiting a Future<recordType'> with the same structure as the original, and element types being the flatten of the original element types.

Can we also do that for lists too, so that awaiting a list of futures yields a future of a list of the results? Sometimes you do need to wait for a dynamically-sized homogeneous list of asynchronous tasks. (I suppose the answer is "no" because there could be some user-defined class that implements both List and Future :( ).

Yes, I'm on board with this. Maybe we should carve it out as a separate proposal since the main patterns one already has a lot of surface area?

@munificent
Copy link
Member

Barring that, you could do something fairly nice with extensions:

extension FutureRecord2Extension<T1, T2> on (Future<T1>, Future<T2>) {
  Future<(T1, T2)> wait() async {
    var (f1, f2) = this;
    var [e1 as T1, e2 as T2] = await Future.wait([f1, f2]);
    return (e1, e2);
  }
}

extension FutureRecord3Extension<T1, T2, T3> on (Future<T1>, Future<T2>, Future<T3>) {
  Future<(T1, T2, T3)> wait() async {
    var (f1, f2, f3) = this;
    var [e1 as T1, e2 as T2, e3 as T3] = await Future.wait([f1, f2, f3]);
    return (e1, e2, e3);
  }
}

// Other arities...
Future<int> asyncThing() ...
Future<String> another() ...

main() async {
  var (n, s) = await (asyncThing(), another()).wait();
}

@lrhn
Copy link
Member

lrhn commented Jun 28, 2022

Like the extensions, hate the dynamic typing being necessary. Also doesn't scale well with named record elements.

A more direct approach, without using Future.wait, would be:

extension FR2X<T1, T2> on (Future<T1>, Future<T2>) {
  Future<(T1, T2)> wait() async {
    Completer<(T1, T2)> c = Completer.sync();
    T1? v1;
    T2? v2;
    var count = 0;
    List<AsyncError>? errors;
    void tryComplete() {
       if (++count == 2) {
         if (errors == null) {
           c.complete((v1 as T1, v2 as T2));
         } else {
           c.completeError(AwaitRecordException<(T1?, T2?)>((v1, v2), errors));
         }
       }
    }
    var (f1, f2) = this;
    unawaited(f1.then((v) { v1 = v; tryComplete()}, onError: (Object e, StackTrace s) {
      (errors ??= []).add(AsyncError(e, s); tryComplete(); 
    }));
    unawaited(f2.then((v) { v2 = v; tryComplete()}, onError: (Object e, StackTrace s) {
      (errors ??= []).add(AsyncError(e, s); tryComplete(); 
    }));
    return c.future;
  }
}

Doesn't scale to other record structures any better. This really begs to be a language or library feature, and for best typing, it really should be a language feature.

@leafpetersen
Copy link
Member

Yes, I'm on board with this. Maybe we should carve it out as a separate proposal since the main patterns one already has a lot of surface area?

I'd like to re-surface this. I think this proposal has a lot of potential value, and I think if we're going to do it, we need to do it now (since once the first record gets awaited, this becomes a breaking change).

Thoughts on this?

cc @munificent @lrhn @eernstg @jakemac53 @natebosch @stereotype441

@leafpetersen
Copy link
Member

I see that @lrhn has a proposal for this here.

@munificent
Copy link
Member

Closing this as a duplicate of #2321 since that has covers the same territory and has more details.

@munificent munificent added the state-duplicate This issue or pull request already exists label Aug 18, 2022
@srawlins
Copy link
Member Author

Thanks for all of the discussion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patterns Issues related to pattern matching. state-duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

4 participants