[dart2js] Correctness updates for async lowering.

- Wrap returns in Future.value to ensure the returned Future has the async function's return type.
- Skip any function that has try/catch/finally as these introduce more complex control flow. (Might be able to convert these using an onError callback in the future).
- Don't assume futureValueType since CFE might not populate it in Kernel when syntax is incorrect.

Change-Id: Ice3954da52a10a74f93b0adc6409a2d98e13cb3b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/241260
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Stephen Adams <sra@google.com>

pkg/compiler/lib/src/kernel/transformations/async_lowering.dart

@@ -11,7 +11,9 @@ import 'package:kernel/kernel.dart';
 class _FunctionData {
   final List<AwaitExpression> awaits = [];
   final Set<ReturnStatement> returnStatements = {};
-  bool hasAsyncLoop = false;
+  // If we find certain control flow statements in this function, we choose to
+  // not lower it.
+  bool shouldLower = true;
 
   _FunctionData();
 
@@ -35,7 +37,9 @@ class AsyncLowering {
   AsyncLowering(this._coreTypes);
 
   bool _shouldTryAsyncLowering(FunctionNode node) =>
-      node.asyncMarker == AsyncMarker.Async && !_functions.last.hasAsyncLoop;
+      node.asyncMarker == AsyncMarker.Async &&
+      node.futureValueType != null &&
+      _functions.last.shouldLower;
 
   void enterFunction(FunctionNode node) {
     _functions.add(_FunctionData());
@@ -66,6 +70,20 @@ class AsyncLowering {
             ]))));
   }
 
+  void _wrapReturns(_FunctionData functionData, FunctionNode node) {
+    final futureValueType = node.futureValueType!;
+    for (final returnStatement in functionData.returnStatements) {
+      final expression = returnStatement.expression;
+      // Ensure the returned future has a runtime type (T) matching the
+      // function's return type by wrapping with Future.value<T>.
+      if (expression == null) continue;
+      final futureValueCall = StaticInvocation(_coreTypes.futureValueFactory,
+          Arguments([expression], types: [futureValueType]));
+      returnStatement.expression = futureValueCall;
+      futureValueCall.parent = returnStatement;
+    }
+  }
+
   void _transformDirectReturnAwaits(
       FunctionNode node, _FunctionData functionData) {
     // If every await is the direct child of a return statement then we can
@@ -117,6 +135,7 @@ class AsyncLowering {
       isLowered = true;
     }
     if (isLowered) {
+      _wrapReturns(functionData, node);
       _wrapBodySync(node);
     }
   }
@@ -138,7 +157,11 @@ class AsyncLowering {
 
   void visitForInStatement(ForInStatement statement) {
     if (statement.isAsync && _functions.isNotEmpty) {
-      _functions.last.hasAsyncLoop = true;
+      _functions.last.shouldLower = false;
     }
   }
+
+  void visitTry() {
+    _functions.last.shouldLower = false;
+  }
 }

pkg/compiler/lib/src/kernel/transformations/lowering.dart

@@ -114,4 +114,18 @@ class _Lowering extends Transformer {
     statement.transformChildren(this);
     return statement;
   }
+
+  @override
+  TreeNode visitTryFinally(TryFinally statement) {
+    _asyncLowering?.visitTry();
+    statement.transformChildren(this);
+    return statement;
+  }
+
+  @override
+  TreeNode visitTryCatch(TryCatch statement) {
+    _asyncLowering?.visitTry();
+    statement.transformChildren(this);
+    return statement;
+  }
 }

pkg/front_end/testcases/dart2js/async_lowering/collapse_return_await.dart.strong.transformed.expect

@@ -8,19 +8,19 @@ static method bar() → asy::Future<core::int> {
 }
 static method foo1() → asy::Future<core::int> /* futureValueType= core::int */ /* originally async */ 
   return asy::Future::sync<core::int>(() → FutureOr<core::int> {
-    return self::bar();
+    return asy::Future::value<core::int>(self::bar());
   });
 static method foo2(core::bool x) → asy::Future<core::int> /* futureValueType= core::int */ /* originally async */ 
   return asy::Future::sync<core::int>(() → FutureOr<core::int> {
     if(x)
-      return asy::Future::value<core::int>(345);
-    return self::bar();
+      return asy::Future::value<core::int>(asy::Future::value<core::int>(345));
+    return asy::Future::value<core::int>(self::bar());
   });
 static method foo3(core::bool x) → asy::Future<core::int> /* futureValueType= core::int */ /* originally async */ 
   return asy::Future::sync<core::int>(() → FutureOr<core::int> {
     if(x)
-      return 234;
-    return asy::Future::value<core::int>(123);
+      return asy::Future::value<core::int>(234);
+    return asy::Future::value<core::int>(asy::Future::value<core::int>(123));
   });
 static method main() → void {
   self::foo1();

pkg/front_end/testcases/dart2js/async_lowering/collapse_return_await.dart.weak.transformed.expect

@@ -8,19 +8,19 @@ static method bar() → asy::Future<core::int> {
 }
 static method foo1() → asy::Future<core::int> /* futureValueType= core::int */ /* originally async */ 
   return asy::Future::sync<core::int>(() → FutureOr<core::int> {
-    return self::bar();
+    return asy::Future::value<core::int>(self::bar());
   });
 static method foo2(core::bool x) → asy::Future<core::int> /* futureValueType= core::int */ /* originally async */ 
   return asy::Future::sync<core::int>(() → FutureOr<core::int> {
     if(x)
-      return asy::Future::value<core::int>(345);
-    return self::bar();
+      return asy::Future::value<core::int>(asy::Future::value<core::int>(345));
+    return asy::Future::value<core::int>(self::bar());
   });
 static method foo3(core::bool x) → asy::Future<core::int> /* futureValueType= core::int */ /* originally async */ 
   return asy::Future::sync<core::int>(() → FutureOr<core::int> {
     if(x)
-      return 234;
-    return asy::Future::value<core::int>(123);
+      return asy::Future::value<core::int>(234);
+    return asy::Future::value<core::int>(asy::Future::value<core::int>(123));
   });
 static method main() → void {
   self::foo1();

pkg/front_end/testcases/dart2js/async_lowering/no_await.dart.strong.transformed.expect

@@ -6,7 +6,7 @@ import "dart:core" as core;
 static method foo1() → asy::Future<core::int> /* futureValueType= core::int */ /* originally async */ 
   return asy::Future::sync<core::int>(() → FutureOr<core::int> {
     final core::int c = 3;
-    return c;
+    return asy::Future::value<core::int>(c);
   });
 static method foo2() → asy::Future<void> /* futureValueType= void */ /* originally async */ 
   return asy::Future::sync<void>(() → FutureOr<void>? {
@@ -14,21 +14,21 @@ static method foo2() → asy::Future<void> /* futureValueType= void */ /* origin
   });
 static method foo3() → dynamic /* futureValueType= dynamic */ /* originally async */ 
   return asy::Future::sync<dynamic>(() → FutureOr<dynamic>? {
-    return 234;
+    return asy::Future::value<dynamic>(234);
   });
 static method bar(() → asy::Future<core::int> func) → void {
   func(){() → asy::Future<core::int>};
 }
 static method foo4() → asy::Future<core::bool> async /* futureValueType= core::bool */ {
   await asy::Future::value<core::int>(2);
-  self::bar(() → asy::Future<core::int> /* futureValueType= core::int */ /* originally async */ => asy::Future::sync<core::int>(() → FutureOr<core::int> => 3));
+  self::bar(() → asy::Future<core::int> /* futureValueType= core::int */ /* originally async */ => asy::Future::sync<core::int>(() → FutureOr<core::int> => asy::Future::value<core::int>(3)));
   return true;
 }
 static method foo5(core::bool x) → asy::Future<core::int> /* futureValueType= core::int */ /* originally async */ 
   return asy::Future::sync<core::int>(() → FutureOr<core::int> {
     if(x)
-      return 123;
-    return 234;
+      return asy::Future::value<core::int>(123);
+    return asy::Future::value<core::int>(234);
   });
 static method main() → void {
   self::foo1();

pkg/front_end/testcases/dart2js/async_lowering/no_await.dart.weak.transformed.expect

@@ -6,7 +6,7 @@ import "dart:core" as core;
 static method foo1() → asy::Future<core::int> /* futureValueType= core::int */ /* originally async */ 
   return asy::Future::sync<core::int>(() → FutureOr<core::int> {
     final core::int c = 3;
-    return c;
+    return asy::Future::value<core::int>(c);
   });
 static method foo2() → asy::Future<void> /* futureValueType= void */ /* originally async */ 
   return asy::Future::sync<void>(() → FutureOr<void>? {
@@ -14,21 +14,21 @@ static method foo2() → asy::Future<void> /* futureValueType= void */ /* origin
   });
 static method foo3() → dynamic /* futureValueType= dynamic */ /* originally async */ 
   return asy::Future::sync<dynamic>(() → FutureOr<dynamic>? {
-    return 234;
+    return asy::Future::value<dynamic>(234);
   });
 static method bar(() → asy::Future<core::int> func) → void {
   func(){() → asy::Future<core::int>};
 }
 static method foo4() → asy::Future<core::bool> async /* futureValueType= core::bool */ {
   await asy::Future::value<core::int>(2);
-  self::bar(() → asy::Future<core::int> /* futureValueType= core::int */ /* originally async */ => asy::Future::sync<core::int>(() → FutureOr<core::int> => 3));
+  self::bar(() → asy::Future<core::int> /* futureValueType= core::int */ /* originally async */ => asy::Future::sync<core::int>(() → FutureOr<core::int> => asy::Future::value<core::int>(3)));
   return true;
 }
 static method foo5(core::bool x) → asy::Future<core::int> /* futureValueType= core::int */ /* originally async */ 
   return asy::Future::sync<core::int>(() → FutureOr<core::int> {
     if(x)
-      return 123;
-    return 234;
+      return asy::Future::value<core::int>(123);
+    return asy::Future::value<core::int>(234);
   });
 static method main() → void {
   self::foo1();

pkg/front_end/testcases/dart2js/async_lowering/no_transform.dart

@@ -8,14 +8,35 @@ Future<int> foo2() async {
   return (await 6) + 3;
 }
 
+// Function contains an async for-in loop.
 Future<void> foo3() async {
   await for (final x in Stream.empty()) {
     break;
   }
 }
 
+// Function contains a try-finally statement.
+Future<int> foo4() async {
+  try {
+    return 3;
+  } finally {
+    return 2;
+  }
+}
+
+// Function contains a try-catch statement.
+Future<int> foo5() async {
+  try {
+    return 3;
+  } catch (e) {
+    return 2;
+  }
+}
+
 void main() {
   foo1();
   foo2();
   foo3();
+  foo4();
+  foo5();
 }

pkg/front_end/testcases/dart2js/async_lowering/no_transform.dart.strong.expect

@@ -15,8 +15,26 @@ static method foo3() → asy::Future<void> async /* futureValueType= void */ {
     break #L1;
   }
 }
+static method foo4() → asy::Future<core::int> async /* futureValueType= core::int */ {
+  try {
+    return 3;
+  }
+  finally {
+    return 2;
+  }
+}
+static method foo5() → asy::Future<core::int> async /* futureValueType= core::int */ {
+  try {
+    return 3;
+  }
+  on core::Object catch(final core::Object e) {
+    return 2;
+  }
+}
 static method main() → void {
   self::foo1();
   self::foo2();
   self::foo3();
+  self::foo4();
+  self::foo5();
 }

pkg/front_end/testcases/dart2js/async_lowering/no_transform.dart.strong.transformed.expect

@@ -15,8 +15,26 @@ static method foo3() → asy::Future<void> async /* futureValueType= void */ {
     break #L1;
   }
 }
+static method foo4() → asy::Future<core::int> async /* futureValueType= core::int */ {
+  try {
+    return 3;
+  }
+  finally {
+    return 2;
+  }
+}
+static method foo5() → asy::Future<core::int> async /* futureValueType= core::int */ {
+  try {
+    return 3;
+  }
+  on core::Object catch(final core::Object e) {
+    return 2;
+  }
+}
 static method main() → void {
   self::foo1();
   self::foo2();
   self::foo3();
+  self::foo4();
+  self::foo5();
 }

pkg/front_end/testcases/dart2js/async_lowering/no_transform.dart.textual_outline.expect

@@ -1,4 +1,6 @@
 Future<void> foo1() async {}
 Future<int> foo2() async {}
 Future<void> foo3() async {}
+Future<int> foo4() async {}
+Future<int> foo5() async {}
 void main() {}

pkg/front_end/testcases/dart2js/async_lowering/no_transform.dart.textual_outline_modelled.expect

@@ -1,4 +1,6 @@
 Future<int> foo2() async {}
+Future<int> foo4() async {}
+Future<int> foo5() async {}
 Future<void> foo1() async {}
 Future<void> foo3() async {}
 void main() {}

pkg/front_end/testcases/dart2js/async_lowering/no_transform.dart.weak.expect

@@ -15,8 +15,26 @@ static method foo3() → asy::Future<void> async /* futureValueType= void */ {
     break #L1;
   }
 }
+static method foo4() → asy::Future<core::int> async /* futureValueType= core::int */ {
+  try {
+    return 3;
+  }
+  finally {
+    return 2;
+  }
+}
+static method foo5() → asy::Future<core::int> async /* futureValueType= core::int */ {
+  try {
+    return 3;
+  }
+  on core::Object catch(final core::Object e) {
+    return 2;
+  }
+}
 static method main() → void {
   self::foo1();
   self::foo2();
   self::foo3();
+  self::foo4();
+  self::foo5();
 }

pkg/front_end/testcases/dart2js/async_lowering/no_transform.dart.weak.modular.expect

@@ -15,8 +15,26 @@ static method foo3() → asy::Future<void> async /* futureValueType= void */ {
     break #L1;
   }
 }
+static method foo4() → asy::Future<core::int> async /* futureValueType= core::int */ {
+  try {
+    return 3;
+  }
+  finally {
+    return 2;
+  }
+}
+static method foo5() → asy::Future<core::int> async /* futureValueType= core::int */ {
+  try {
+    return 3;
+  }
+  on core::Object catch(final core::Object e) {
+    return 2;
+  }
+}
 static method main() → void {
   self::foo1();
   self::foo2();
   self::foo3();
+  self::foo4();
+  self::foo5();
 }

pkg/front_end/testcases/dart2js/async_lowering/no_transform.dart.weak.outline.expect

@@ -9,5 +9,9 @@ static method foo2() → asy::Future<core::int> async
   ;
 static method foo3() → asy::Future<void> async 
   ;
+static method foo4() → asy::Future<core::int> async 
+  ;
+static method foo5() → asy::Future<core::int> async 
+  ;
 static method main() → void
   ;

pkg/front_end/testcases/dart2js/async_lowering/no_transform.dart.weak.transformed.expect

@@ -15,8 +15,26 @@ static method foo3() → asy::Future<void> async /* futureValueType= void */ {
     break #L1;
   }
 }
+static method foo4() → asy::Future<core::int> async /* futureValueType= core::int */ {
+  try {
+    return 3;
+  }
+  finally {
+    return 2;
+  }
+}
+static method foo5() → asy::Future<core::int> async /* futureValueType= core::int */ {
+  try {
+    return 3;
+  }
+  on core::Object catch(final core::Object e) {
+    return 2;
+  }
+}
 static method main() → void {
   self::foo1();
   self::foo2();
   self::foo3();
+  self::foo4();
+  self::foo5();
 }

pkg/kernel/lib/core_types.dart

@@ -181,6 +181,9 @@ class CoreTypes {
   late final Procedure futureSyncFactory =
       index.getMember('dart:async', 'Future', 'sync') as Procedure;
 
+  late final Procedure futureValueFactory =
+      index.getMember('dart:async', 'Future', 'value') as Procedure;
+
   // TODO(cstefantsova): Remove it when FutureOrType is fully supported.
   late final Class deprecatedFutureOrClass =
       index.getClass('dart:async', 'FutureOr');