Reland "[dart2js] Add runtime type check for await."

This is a reland of commit c81711b

Original change's description:
> [dart2js] Add runtime type check for `await`.
>
> See #49396 for details.
>
> Fixes: #50601
> Change-Id: Ie89130cffe642b3e4935d7d02fe2e34f7fc8b12e
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/316320
> Commit-Queue: Mayank Patke <[email protected]>
> Reviewed-by: Stephen Adams <[email protected]>

Change-Id: Ida3258ee3768e8bff0161019511647db8b161473
Bug: #50601
Bug: b/295131730
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319462
Commit-Queue: Mayank Patke <[email protected]>
Reviewed-by: Stephen Adams <[email protected]>

Author: fishythefish

pkg/compiler/lib/src/common/elements.dart

@@ -88,6 +88,10 @@ abstract class CommonElements {
   /// The `Future` class defined in 'async';.
   late final ClassEntity futureClass = _findClass(asyncLibrary, 'Future');
 
+  /// The `Future.value` constructor.
+  late final ConstructorEntity? futureValueConstructor =
+      _env.lookupConstructor(futureClass, 'value');
+
   /// The `Stream` class defined in 'async';
   late final ClassEntity streamClass = _findClass(asyncLibrary, 'Stream');
 

pkg/compiler/lib/src/ir/closure.dart

@@ -215,6 +215,9 @@ enum VariableUseKind {
 
   /// A type argument of an generic instantiation.
   instantiationTypeArgument,
+
+  /// A type variable used in the runtime check type of an `await` expression.
+  awaitCheck,
 }
 
 class VariableUse {
@@ -304,6 +307,9 @@ class VariableUse {
   static const VariableUse fieldType =
       VariableUse._simple(VariableUseKind.fieldType);
 
+  static const VariableUse awaitCheck =
+      VariableUse._simple(VariableUseKind.awaitCheck);
+
   @override
   int get hashCode =>
       kind.hashCode * 11 +

pkg/compiler/lib/src/ir/impact.dart

@@ -58,6 +58,8 @@ abstract class ImpactRegistry {
 
   void registerSyncStar(ir.DartType elementType);
 
+  void registerAwaitCheck();
+
   void registerAsync(ir.DartType elementType);
 
   void registerAsyncStar(ir.DartType elementType);

pkg/compiler/lib/src/ir/impact_data.dart

@@ -4,6 +4,7 @@
 
 import 'package:kernel/ast.dart' as ir;
 import 'package:kernel/class_hierarchy.dart' as ir;
+import 'package:kernel/core_types.dart' as ir;
 import 'package:kernel/type_environment.dart' as ir;
 
 import '../common.dart';
@@ -50,6 +51,8 @@ class ImpactBuilder extends StaticTypeVisitor implements ImpactRegistry {
       : super(
             staticTypeContext.typeEnvironment, classHierarchy, staticTypeCache);
 
+  ir.CoreTypes get _coreTypes => typeEnvironment.coreTypes;
+
   @override
   VariableScopeModel get variableScopeModel => _variableScopeModel!;
 
@@ -607,6 +610,24 @@ class ImpactBuilder extends StaticTypeVisitor implements ImpactRegistry {
         .visitConstant(node.constant);
   }
 
+  @override
+  void handleAwaitExpression(ir.AwaitExpression node) {
+    final runtimeCheckType = node.runtimeCheckType;
+    if (runtimeCheckType != null) {
+      // Register the impacts which would have been registered if this were
+      // implemented directly as Dart code. Some of these may be redundant with
+      // the impacts we already register for `async` code, but we include them
+      // for completeness.
+      registerAwaitCheck();
+      registerIsCheck(runtimeCheckType);
+      final typeArgument =
+          (runtimeCheckType as ir.InterfaceType).typeArguments.single;
+      registerNew(_coreTypes.futureValueFactory, runtimeCheckType, 1, const [],
+          [typeArgument], getDeferredImport(node),
+          isConst: false);
+    }
+  }
+
   void _registerFeature(_Feature feature) {
     (_data._features ??= EnumSet<_Feature>()).add(feature);
   }
@@ -881,6 +902,11 @@ class ImpactBuilder extends StaticTypeVisitor implements ImpactRegistry {
     _registerTypeUse(elementType, _TypeUseKind.asyncMarker);
   }
 
+  @override
+  void registerAwaitCheck() {
+    _registerFeature(_Feature.awaitCheck);
+  }
+
   @override
   void registerSyncStar(ir.DartType elementType) {
     _registerTypeUse(elementType, _TypeUseKind.syncStarMarker);
@@ -1395,6 +1421,9 @@ class ImpactData {
           case _Feature.intLiteral:
             registry.registerIntLiteral();
             break;
+          case _Feature.awaitCheck:
+            registry.registerAwaitCheck();
+            break;
         }
       }
     }
@@ -1903,6 +1932,7 @@ enum _Feature {
   intLiteral,
   symbolLiteral,
   doubleLiteral,
+  awaitCheck,
 }
 
 class _TypeUse {

pkg/compiler/lib/src/ir/scope_visitor.dart

@@ -839,6 +839,16 @@ class ScopeModelBuilder extends ir.Visitor<EvaluationComplexity>
   @override
   EvaluationComplexity visitAwaitExpression(ir.AwaitExpression node) {
     node.operand = _handleExpression(node.operand);
+    final runtimeCheckType = node.runtimeCheckType;
+    if (runtimeCheckType != null) {
+      visitInContext(runtimeCheckType, VariableUse.awaitCheck);
+      final typeArgument =
+          (runtimeCheckType as ir.InterfaceType).typeArguments.single;
+      visitInContext(
+          typeArgument,
+          VariableUse.constructorTypeArgument(
+              _typeEnvironment.coreTypes.futureValueFactory));
+    }
     return const EvaluationComplexity.lazy();
   }
 

pkg/compiler/lib/src/ir/static_type.dart

@@ -1838,6 +1838,14 @@ abstract class StaticTypeVisitor extends StaticTypeBase {
     handleConstantExpression(node);
     return super.visitConstantExpression(node);
   }
+
+  void handleAwaitExpression(ir.AwaitExpression node) {}
+
+  @override
+  ir.DartType visitAwaitExpression(ir.AwaitExpression node) {
+    handleAwaitExpression(node);
+    return super.visitAwaitExpression(node);
+  }
 }
 
 class ArgumentTypes {

pkg/compiler/lib/src/js_backend/backend_impact.dart

@@ -110,6 +110,12 @@ class BackendImpacts {
     ],
   );
 
+  late final BackendImpact awaitExpression = BackendImpact(
+    staticUses: [
+      _commonElements.futureValueConstructor!,
+    ],
+  );
+
   late final BackendImpact asyncBody = BackendImpact(
     staticUses: [
       _commonElements.asyncHelperAwait,

pkg/compiler/lib/src/js_backend/backend_usage.dart

@@ -171,6 +171,7 @@ class BackendUsageBuilderImpl implements BackendUsageBuilder {
   bool _isValidEntity(Entity element) {
     if (element is ConstructorEntity &&
         (element == _commonElements.streamIteratorConstructor ||
+            element == _commonElements.futureValueConstructor ||
             _commonElements.isSymbolConstructor(element))) {
       // TODO(johnniwinther): These are valid but we could be more precise.
       return true;

pkg/compiler/lib/src/js_model/closure.dart

@@ -226,6 +226,7 @@ class ClosureDataBuilder {
       for (VariableUse usage in useSet) {
         switch (usage.kind) {
           case VariableUseKind.explicit:
+          case VariableUseKind.awaitCheck:
             return true;
           case VariableUseKind.implicitCast:
             if (_annotationsData

pkg/compiler/lib/src/kernel/kernel_impact.dart

@@ -201,6 +201,11 @@ class KernelImpactConverter implements ImpactRegistry {
         <DartType>[elementMap.getDartType(elementType)]));
   }
 
+  @override
+  void registerAwaitCheck() {
+    registerBackendImpact(_impacts.awaitExpression);
+  }
+
   @override
   void registerAsync(ir.DartType elementType) {
     registerBackendImpact(_impacts.asyncBody);

pkg/compiler/lib/src/ssa/builder.dart

@@ -6060,11 +6060,64 @@ class KernelSsaGraphBuilder extends ir.Visitor<void> with ir.VisitorVoidMixin {
 
   @override
   void visitAwaitExpression(ir.AwaitExpression node) {
-    node.operand.accept(this);
+    // `await e` first checks if the runtime type of `e` is a subtype of
+    // `Future<T>` where `T = flatten(S)` and `S` is the static type of `e`.
+    // (The type `Future<T>` is helpfully precomputed by the CFE and stored in
+    // [AwaitExpression.runtimeCheckType].)
+    //
+    // If the check succeeds (or if no `runtimeCheckType` is provided), we await
+    // `e` directly.
+    //
+    // If the check fails, we must await `Future.value(e)` instead.
+    //
+    // See https://github.com/dart-lang/sdk/issues/49396 for details.
+
+    final operand = node.operand;
+    operand.accept(this);
     HInstruction awaited = pop();
-    // TODO(herhut): Improve this type.
-    push(HAwait(awaited, _abstractValueDomain.dynamicType)
-      ..sourceInformation = _sourceInformationBuilder.buildAwait(node));
+    final runtimeCheckType = node.runtimeCheckType;
+    final sourceInformation = _sourceInformationBuilder.buildAwait(node);
+
+    void checkType() {
+      // TODO(fishythefish): Can we get rid of the redundancy with the type
+      // checks in async_patch?
+      _pushIsTest(runtimeCheckType!, awaited, sourceInformation);
+    }
+
+    void pushAwait(HInstruction expression) {
+      // TODO(herhut): Improve this type.
+      push(HAwait(expression, _abstractValueDomain.dynamicType)
+        ..sourceInformation = sourceInformation);
+    }
+
+    void awaitUnwrapped() {
+      pushAwait(awaited);
+    }
+
+    void awaitWrapped() {
+      final constructor = _commonElements.futureValueConstructor!;
+      final arguments = [awaited];
+      // If [runtimeCheckType] exists, it is guaranteed to be `Future<T>`, and
+      // we want to call `Future<T>.value`.
+      final typeArgument = _elementMap.getDartType(
+          (runtimeCheckType as ir.InterfaceType).typeArguments.single);
+      final typeArguments = [typeArgument];
+      _addTypeArguments(arguments, typeArguments, sourceInformation);
+      final instanceType = _commonElements.futureType(typeArgument);
+      _addImplicitInstantiation(instanceType);
+      _pushStaticInvocation(constructor, arguments,
+          _typeInferenceMap.getReturnTypeOf(constructor), typeArguments,
+          sourceInformation: sourceInformation, instanceType: instanceType);
+      _removeImplicitInstantiation(instanceType);
+      pushAwait(pop());
+    }
+
+    if (runtimeCheckType == null) {
+      awaitUnwrapped();
+    } else {
+      SsaBranchBuilder(this)
+          .handleConditional(checkType, awaitUnwrapped, awaitWrapped);
+    }
   }
 
   @override

tests/web/await_non_future_test.dart

@@ -0,0 +1,13 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+class A<T> {
+  factory A() = B<T>;
+}
+
+class B<T> implements A<T> {}
+
+main() async {
+  await A<void>();
+}

tests/web/regress/issue/295131730_test.dart

@@ -0,0 +1,18 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:async';
+import 'package:expect/expect.dart';
+
+Function _foo<T>(FutureOr<T> f()) {
+  return (() async {
+    await f();
+  });
+}
+
+main() async {
+  var x = 0;
+  _foo<int>(() => x = 1)();
+  Expect.equals(1, x);
+}