dart-lang / language

Design of the Dart language
Other
2.67k stars 205 forks source link

Destructuring in await for #2764

Open munificent opened 1 year ago

munificent commented 1 year ago

The patterns proposal allows patterns in for loop variable declarations for:

The missing one is await for statements. (Now that I think about it, there's no await for element either...)

Given that await for is so rarely used, I don't think it's important to support patterns there in the initial release. But I'm filing this to track whether we want to eventually.

lrhn commented 1 year ago

I was assuming that await for (p in e) and for (p in e) used the same grammar and binding behavior, with an

await? for ( ... )

production, and only semantic differences.

There is an await for-element, and it works:

void main() async {
  var x = [await for (var v in stream()) v];
  print(x);
}
Stream<int> stream() async* {
 yield 1;
 yield 2;
}
munificent commented 1 year ago

There is an await for-element, and it works:

Huh, today I learned. Maybe I did know that at one point and forgot?

stereotype441 commented 1 year ago

Personally I lean toward supporting patterns in await for, because the "await"-ness of a for-loop feels orthogonal to its "pattern"-ness. Yes, it's an implementation burden to us to support it, but if we don't support it, it's a cognitive burden on the users because they have to remember that a certain obscure combination of features isn't supported, even though by all rights there's no good reason why it shouldn't work.

As a side note, destructuring isn't the only possible use of patterns in for-loops. Personally, I think cast patterns in for-loops are really exciting, because they will let us restore a piece of functionality we lost when we introduced when we added null safety. Before null safety you used to be able to do:

f(List<num> nums) {
  // I know all the nums are ints so downcast
  for (int i in nums) {
    ...
  }
}

But with null safety, this became an error, and the only workaround was to do something ugly like:

f(List<num> nums) {
  for (var n in nums) {
    var i = n as int;
    ...
  }
}

With pattern support, we'll be able to do this:

f(List<num> nums) {
  for ((var i as int) in nums) {
    ...
  }
}

Which I think is great.

I would hate to be able to do this in for-loops but not in await-for; that just seems like an unnecessary source of user frustration.

bwilkerson commented 1 year ago

If we decide to not support patterns in await for, we'll need tests to ensure that we're producing good diagnostics in those cases. The analyzer (and hence parser) happily accept an await for with a pattern without producing any diagnostics (at least not about using patterns in an await for, there is currently a bad diagnostic being produced that will need to be fixed).

lrhn commented 1 year ago

An await for (declPattern in stream) { body } should be indistinguishable from await for («TypeOfDeclarationPattern(declPattern)» $tmp in stream) { declPattern = $tmp; body } (other than possibly the stack traces of patterns that can fail).

If nothing else, I think we should do such a desugaring (because otherwise the users will have to write that exact same thing themselves).

The only non-trivial point is the doing the type inference from declPattern x to and back from stream, but it should go something like:

  1. Extract context type schema C from declaration pattern.
  2. Infer stream with context type schema Stream<C> to static type R.
    • It's a compile-time error if R is not assignable to Stream<Object?>, which means implementing Stream<X> for some X or being Never or dynamic.
  3. Deduce element type E of stream type R (X from above, Never or dynamic.)
  4. Infer types in pattern from matched value type E.
  5. Determine the type of $tmp from that.

We do 1. and 4. for every pattern assignment. We do 2. and 3. for await for (variableDecl in stream) ... today. For step 5, the type of $tmp is E. So, we should have all the pieces.

We have to do type inference before desugaring, otherwise the context type schema from the pattern won't affect the stream expression.

munificent commented 1 year ago

I'm agnostic. If the implementation folks feel it's not too burdensome to get it into 3.0, I will happily specify it (and by that I mean copy/paste what Lasse wrote). If it's going to add any significant stress for the implementers I am completely happy to punt on it.

To give you a sense of how rare await for is, here is all the statements in a large corpus of pub packages:

-- Statements (2154376 total) --
 728235 ( 33.803%): expression  =================
 600301 ( 27.864%): block       ==============
 305370 ( 14.174%): variable    =======
 256912 ( 11.925%): return      ======
 180976 (  8.400%): if          =====
  24462 (  1.135%): break       =
  13394 (  0.622%): for-in      =
  12200 (  0.566%): try         =
   8259 (  0.383%): assert      =
   8018 (  0.372%): switch      =
   7573 (  0.352%): for         =
   4429 (  0.206%): while       =
   2089 (  0.097%): continue    =
   1443 (  0.067%): yield       =
    381 (  0.018%): do-while    =
    267 (  0.012%): await for   =
     67 (  0.003%): empty       =

-- For (21234 total) --
  13394 ( 63.078%): for-in  =================================
   7573 ( 35.665%): for     ===================
    267 (  1.257%): await   =