[dart2wasm] Avoid type checks on parameters when we have guarantee th…

…ey will succeed

This is porting a VM optimization to dart2wasm that omits implicit
incoming argument type checks if either of this holds:

  * TFA inferred a type check can be omitted
  * all callers are invocations on `this` and the method isn't torn off

This also fixes the type checking of type parameters & type arguments
for CFE-inserted forwarding stubs: For those we have to check against
types from the forwarding stub, not the method itself.

=> This bug got revealed once omitting type checks in the forwarding
=> stub (which TFA annotates as `(skip check)`)

TEST=language/covariant/subtyping_with_mixin_test

Change-Id: I6ea58f688611355a686c12a4f86fb2fa76fc76d9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/380840
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ömer Ağacan <omersa@google.com>

pkg/dart2wasm/lib/code_generator.dart

@@ -228,17 +228,39 @@ abstract class AstCodeGenerator
 
   void _setupLocalParameters(Member member, ParameterInfo paramInfo,
       int parameterOffset, int implicitParams,
-      {bool isForwarder = false}) {
-    List<TypeParameter> typeParameters = member is Constructor
+      {bool isForwarder = false, bool canSafelyOmitImplicitChecks = false}) {
+    final memberFunction = member.function!;
+    final List<TypeParameter> typeParameters = member is Constructor
         ? member.enclosingClass.typeParameters
         : member.function!.typeParameters;
+    final List<VariableDeclaration> positional =
+        memberFunction.positionalParameters;
+    final List<VariableDeclaration> named = memberFunction.namedParameters;
+
+    // If this is a CFE-inserted `forwarding-stub` then the types we have to
+    // check against are those from the forwarding target.
+    //
+    // This mirrors what the VM does in
+    //    - FlowGraphBuilder::BuildTypeArgumentTypeChecks
+    //    - FlowGraphBuilder::BuildArgumentTypeChecks
+    Procedure? forwardingTarget;
+    if (member is Procedure && member.isForwardingStub) {
+      forwardingTarget = member.concreteForwardingStubTarget as Procedure?;
+    }
+    final List<TypeParameter> typeParametersToTypeCheck =
+        forwardingTarget?.typeParameters ?? typeParameters;
+    final List<VariableDeclaration> positionalToTypeCheck =
+        forwardingTarget?.function.positionalParameters ?? positional;
+    final List<VariableDeclaration> namedToTypeCheck =
+        forwardingTarget?.function.namedParameters ?? named;
+
     for (int i = 0; i < typeParameters.length; i++) {
       final typeParameter = typeParameters[i];
       typeLocals[typeParameter] = paramLocals[parameterOffset + i];
     }
     if (!translator.options.omitImplicitTypeChecks) {
-      for (int i = 0; i < typeParameters.length; i++) {
-        final typeParameter = typeParameters[i];
+      for (int i = 0; i < typeParametersToTypeCheck.length; i++) {
+        final typeParameter = typeParametersToTypeCheck[i];
         if (typeParameter.isCovariantByClass &&
             typeParameter.bound != translator.coreTypes.objectNullableRawType) {
           _generateTypeArgumentBoundCheck(typeParameter.name!,
@@ -247,8 +269,12 @@ abstract class AstCodeGenerator
       }
     }
 
-    void setupParamLocal(VariableDeclaration variable, int index,
-        Constant? defaultValue, bool isRequired) {
+    void setupParamLocal(
+        DartType variableTypeToCheck,
+        VariableDeclaration variable,
+        int index,
+        Constant? defaultValue,
+        bool isRequired) {
       w.Local local = paramLocals[implicitParams + index];
       if (defaultValue == ParameterInfo.defaultValueSentinel) {
         // The default value for this parameter differs between implementations
@@ -288,7 +314,10 @@ abstract class AstCodeGenerator
         }
       }
       if (!translator.options.omitImplicitTypeChecks) {
-        if (variable.isCovariantByClass || variable.isCovariantByDeclaration) {
+        if (!translator.canSkipImplicitCheck(variable) &&
+            (variable.isCovariantByDeclaration ||
+                (variable.isCovariantByClass &&
+                    !canSafelyOmitImplicitChecks))) {
           final boxedType = variable.type.isPotentiallyNullable
               ? translator.topInfo.nullableType
               : translator.topInfo.nonNullableType;
@@ -304,7 +333,7 @@ abstract class AstCodeGenerator
           _generateArgumentTypeCheck(
             variable.name!,
             operand.type as w.RefType,
-            variable.type,
+            variableTypeToCheck,
           );
         }
       }
@@ -326,15 +355,17 @@ abstract class AstCodeGenerator
       locals[variable] = local;
     }
 
-    final memberFunction = member.function!;
-    List<VariableDeclaration> positional = memberFunction.positionalParameters;
     for (int i = 0; i < positional.length; i++) {
       final bool isRequired = i < memberFunction.requiredParameterCount;
-      setupParamLocal(positional[i], i, paramInfo.positional[i], isRequired);
+      final typeToCheck = positionalToTypeCheck[i].type;
+      setupParamLocal(
+          typeToCheck, positional[i], i, paramInfo.positional[i], isRequired);
     }
-    List<VariableDeclaration> named = memberFunction.namedParameters;
     for (var param in named) {
-      setupParamLocal(param, paramInfo.nameIndex[param.name]!,
+      final typeToCheck = identical(named, namedToTypeCheck)
+          ? param.type
+          : namedToTypeCheck.singleWhere((n) => n.name == param.name).type;
+      setupParamLocal(typeToCheck, param, paramInfo.nameIndex[param.name]!,
           paramInfo.named[param.name], param.isRequired);
     }
 
@@ -355,19 +386,30 @@ abstract class AstCodeGenerator
     });
   }
 
-  void setupParameters(Reference reference, {bool isForwarder = false}) {
+  void setupParameters(Reference reference,
+      {bool isForwarder = false, bool canSafelyOmitImplicitChecks = false}) {
     Member member = reference.asMember;
     ParameterInfo paramInfo = translator.paramInfoForDirectCall(reference);
 
     int parameterOffset = _initializeThis(reference);
     int implicitParams = parameterOffset + paramInfo.typeParamCount;
 
     _setupLocalParameters(member, paramInfo, parameterOffset, implicitParams,
-        isForwarder: isForwarder);
+        isForwarder: isForwarder,
+        canSafelyOmitImplicitChecks: canSafelyOmitImplicitChecks);
   }
 
   void setupParametersAndContexts(Member member) {
-    setupParameters(member.reference);
+    bool canSafelyOmitImplicitChecks = false;
+    if (member.isInstanceMember) {
+      final selectorInfo =
+          translator.dispatchTable.selectorForTarget(member.reference);
+      canSafelyOmitImplicitChecks =
+          !selectorInfo.hasTearOffUses && !selectorInfo.hasNonThisUses;
+    }
+
+    setupParameters(member.reference,
+        canSafelyOmitImplicitChecks: canSafelyOmitImplicitChecks);
 
     closures.findCaptures(member);
     closures.collectContexts(member);

pkg/dart2wasm/lib/dispatch_table.dart

@@ -42,6 +42,9 @@ class SelectorInfo {
   /// Does this method have any tear-off uses?
   bool hasTearOffUses = false;
 
+  /// Does this method have any non-this uses?
+  bool hasNonThisUses = false;
+
   /// Targets for all concrete classes implementing this selector.
   ///
   /// As a subclass hierarchy often inherits the same target, we associate the
@@ -285,6 +288,7 @@ class DispatchTable {
             isSetter: isSetter));
     assert(selector.isSetter == isSetter);
     selector.hasTearOffUses |= metadata.hasTearOffUses;
+    selector.hasNonThisUses |= metadata.hasNonThisUses;
     selector.paramInfo.merge(paramInfo);
     if (calledDynamically) {
       if (isGetter) {

pkg/dart2wasm/lib/translator.dart

@@ -876,6 +876,10 @@ class Translator with KernelNodes {
     return directCallMetadata[node]?.targetMember;
   }
 
+  bool canSkipImplicitCheck(VariableDeclaration node) {
+    return inferredArgTypeMetadata[node]?.skipCheck ?? false;
+  }
+
   DartType typeOfParameterVariable(VariableDeclaration node, bool isRequired) {
     // We have a guarantee that inferred types are correct.
     final inferredType = _inferredTypeOfParameterVariable(node);