Revert "[ddc] Refactor visitInstanceGetterInvocation()"

This reverts commit 776c058.

Reason for revert: Broke test in google3

Original change's description:
> [ddc] Refactor `visitInstanceGetterInvocation()`
>
> Moves the remaining special case logic out of `_emitMethodCall()`
> simplifying the reasoning about where so hot reload soundness checks
> can be added.
>
> Change-Id: I13e2f451e61f6e067ea689bcc35039cf92946492
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/423603
> Reviewed-by: Mark Zhou <[email protected]>
> Reviewed-by: Nate Biggs <[email protected]>
> Commit-Queue: Nicholas Shahan <[email protected]>

Change-Id: I67d35d085e9b9510547aff8fb16ac0a8ed44d6df
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425404
Reviewed-by: Srujan Gaddam <[email protected]>
Commit-Queue: Nicholas Shahan <[email protected]>
Bot-Commit: Rubber Stamper <[email protected]>

Author: nshahan

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

@@ -5293,7 +5293,8 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
 
   @override
   js_ast.Expression visitInstanceInvocation(InstanceInvocation node) {
-    var invocation = _emitInstanceInvocation(node);
+    var invocation = _emitMethodCall(
+        node.receiver, node.interfaceTarget, node.arguments, node);
     return _isNullCheckableJsInterop(node.interfaceTarget)
         ? _wrapWithJsInteropNullCheck(invocation)
         : invocation;
@@ -5302,64 +5303,13 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
   @override
   js_ast.Expression visitInstanceGetterInvocation(
       InstanceGetterInvocation node) {
-    if (node.functionType == null) {
-      // A `null` here implies the receiver must be typed as `dynamic` or
-      // `Function`. There isn't any more type information available at compile
-      // time to know this invocation is sound so a dynamic call will handle the
-      // checks at runtime.
-      return _emitDynamicInvocation(
-          node.receiver, node.name.text, node.arguments);
-    }
-    var getterInvocation = _emitInstanceGetterInvocation(node);
+    var getterInvocation = _emitMethodCall(
+        node.receiver, node.interfaceTarget, node.arguments, node);
     return _isNullCheckableJsInterop(node.interfaceTarget)
         ? _wrapWithJsInteropNullCheck(getterInvocation)
         : getterInvocation;
   }
 
-  js_ast.Expression _emitInstanceGetterInvocation(
-      InstanceGetterInvocation node) {
-    var receiver = _visitExpression(node.receiver);
-    var arguments =
-        _emitArgumentList(node.arguments, target: node.interfaceTarget);
-    if (node.name.text == 'call') {
-      // Erasing the extension types here to support existing callable behavior
-      // on the old style JS interop types that are callable. This should be
-      // safe as it is a compile time error to try to dynamically invoke a call
-      // method that is inherited from an extension type.
-      var receiverType =
-          node.receiver.getStaticType(_staticTypeContext).extensionTypeErasure;
-      if (_isDirectCallable(receiverType)) {
-        // Call methods on function types should be handled as function calls.
-        return js_ast.Call(receiver, arguments);
-      }
-    }
-    var memberName =
-        _emitMemberName(node.name.text, member: node.interfaceTarget);
-    // We must erase the extension type to potentially find the `call` method.
-    // If the extension type has a runtime representation with a `call`:
-    //
-    // ```
-    // extension type Ext(C c) implements C {...}
-    // class C {
-    //   call() {...}
-    // }
-    // ```
-    //
-    // We can always erase eagerly because:
-    //  - Extension types that do not implement an interface that exposes a
-    //    `call` method will result in a static error at the call site.
-    //  - Calls to extension types that implement their own call method are
-    //    lowered by the CFE to top level static method calls.
-    var erasedGetterType = node.interfaceTarget.getterType.extensionTypeErasure;
-    if (erasedGetterType is InterfaceType) {
-      var callName = _implicitCallTarget(erasedGetterType);
-      if (callName != null) {
-        return js.call('#.#.#(#)', [receiver, memberName, callName, arguments]);
-      }
-    }
-    return js.call('#.#(#)', [receiver, memberName, arguments]);
-  }
-
   @override
   js_ast.Expression visitLocalFunctionInvocation(LocalFunctionInvocation node) {
     assert(node.name.text == 'call');
@@ -5380,15 +5330,20 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
         negated: false);
   }
 
-  js_ast.Expression _emitInstanceInvocation(InstanceInvocation node) {
+  js_ast.Expression _emitMethodCall(Expression receiver, Member target,
+      Arguments arguments, InvocationExpression node) {
+    var name = node.name.text;
+
     /// Returns `true` when [node] represents an invocation of `List.add()` that
     /// can be optimized.
     ///
     /// The optimized add operation can skip checks for a growable or modifiable
     /// list and the element type is known to be invariant so it can skip the
     /// type check.
-    bool isNativeListInvariantAdd(InstanceInvocation node) {
-      if (node.isInvariant && node.name.text == 'add') {
+    bool isNativeListInvariantAdd(InvocationExpression node) {
+      if (node is InstanceInvocation &&
+          node.isInvariant &&
+          node.name.text == 'add') {
         // The call to add is marked as invariant, so the type check on the
         // parameter to add is not needed.
         var receiver = node.receiver;
@@ -5418,10 +5373,6 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
       return false;
     }
 
-    var name = node.name.text;
-    var receiver = node.receiver;
-    var arguments = node.arguments;
-    var target = node.interfaceTarget;
     if (isOperatorMethodName(name) && arguments.named.isEmpty) {
       var argLength = arguments.positional.length;
       if (argLength == 0) {
@@ -5431,33 +5382,75 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
             receiver, target, arguments.positional[0], node);
       }
     }
+
     var jsReceiver = _visitExpression(receiver);
     var args = _emitArgumentList(arguments, target: target);
+
     if (isNativeListInvariantAdd(node)) {
       return js.call('#.push(#)', [jsReceiver, args]);
     }
+
+    var isCallingDynamicField = target.hasGetter &&
+        // Erasing extension types here doesn't make sense. If there is an
+        // extension type on dynamic or Function it will only be callable if it
+        // defines a call method which would be invoked statically.
+        _isDynamicOrFunction(target.getterType);
     if (name == 'call') {
       // Erasing the extension types here to support existing callable behavior
       // on the old style JS interop types that are callable. This should be
       // safe as it is a compile time error to try to dynamically invoke a call
       // method that is inherited from an extension type.
       var receiverType =
           receiver.getStaticType(_staticTypeContext).extensionTypeErasure;
+      if (isCallingDynamicField) {
+        return _emitDynamicInvocation(receiver, name, arguments);
+      }
       if (_isDirectCallable(receiverType)) {
-        // Handle call methods on function types as function calls.
+        // Call methods on function types should be handled as function calls.
         return js_ast.Call(jsReceiver, args);
       }
     }
-    if (_isObjectMethodCall(name, arguments) &&
-        _shouldCallObjectMemberHelper(receiver)) {
-      // Handle Object methods that are supported by `null` and possibly
-      // JavaScript interop values with static helper methods.
-      // The names of the static helper methods in the runtime must match the
-      // names of the Object instance members.
-      return _runtimeCall('#(#, #)', [name, jsReceiver, args]);
-    }
-    // Otherwise generate this as a normal typed method call.
+
     var jsName = _emitMemberName(name, member: target);
+
+    // Handle Object methods that are supported by `null` and potentially
+    // JavaScript interop values.
+    if (_isObjectMethodCall(name, arguments)) {
+      if (_shouldCallObjectMemberHelper(receiver)) {
+        // The names of the static helper methods in the runtime must match the
+        // names of the Object instance members.
+        return _runtimeCall('#(#, #)', [name, jsReceiver, args]);
+      }
+      // Otherwise generate this as a normal typed method call.
+    } else if (isCallingDynamicField) {
+      return _emitDynamicInvocation(receiver, name, arguments);
+    }
+    // TODO(jmesserly): remove when Kernel desugars this for us.
+    // Handle `o.m(a)` where `o.m` is a getter returning a class with `call`.
+    if (target is Field || target is Procedure && target.isAccessor) {
+      // We must erase the extension type to find the `call` method.
+      // If the extension type has a runtime representation with a `call`:
+      //
+      // ```
+      // extension type Ext(C c) implements C {...}
+      // class C {
+      //   call() {...}
+      // }
+      // ```
+      //
+      // We can always erase eagerly because:
+      //  - Extension types that do not implement an interface that exposes a
+      //    `call` method will result in a static error at the call site.
+      //  - Calls to extension types that implement their own call method are
+      //    lowered by the CFE to top level static method calls.
+      var fromType = target.getterType.extensionTypeErasure;
+      if (fromType is InterfaceType) {
+        var callName = _implicitCallTarget(fromType);
+        if (callName != null) {
+          return js.call('#.#.#(#)', [jsReceiver, jsName, callName, args]);
+        }
+      }
+    }
     return js.call('#.#(#)', [jsReceiver, jsName, args]);
   }
 
@@ -5512,6 +5505,11 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     return null;
   }
 
+  bool _isDynamicOrFunction(DartType t) =>
+      DartTypeEquivalence(_coreTypes, ignoreTopLevelNullability: true)
+          .areEqual(t, _coreTypes.functionNonNullableRawType) ||
+      t == const DynamicType();
+
   js_ast.Expression _emitUnaryOperator(
       Expression expr, Member? target, InvocationExpression node) {
     var op = node.name.text;