fix #33876, only use Object method null helpers if signature matches

toString and noSuchMethod are dispatched in a way that allows `null` to
be correctly handled. This code path was not checking if the argument
signature was compatible with the Object methods.

Change-Id: I25b5f11c32961b529660e2d49916053e1fab11df
Reviewed-on: https://dart-review.googlesource.com/65282
Commit-Queue: Jenny Messerly <[email protected]>
Reviewed-by: Bob Nystrom <[email protected]>

pkg/dev_compiler/lib/src/analyzer/code_generator.dart

@@ -3717,7 +3717,8 @@ class CodeGenerator extends Object
   }
 
   JS.Expression _emitMethodCall(Expression target, MethodInvocation node) {
-    var args = _emitArgumentList(node.argumentList);
+    var argumentList = node.argumentList;
+    var args = _emitArgumentList(argumentList);
     var typeArgs = _emitInvokeTypeArguments(node);
 
     var type = getStaticType(target);
@@ -3734,12 +3735,14 @@ class CodeGenerator extends Object
             _emitTargetAccess(jsTarget, jsName, element, node.methodName);
         jsName = null;
       }
-      return _emitDynamicInvoke(
-          jsTarget, typeArgs, jsName, args, node.argumentList);
+      return _emitDynamicInvoke(jsTarget, typeArgs, jsName, args, argumentList);
     }
 
     JS.Expression jsTarget = _emitTarget(target, element, isStatic);
-    if (_isObjectMemberCall(target, name)) {
+
+    // Handle Object methods that are supported by `null`.
+    if (_isObjectMethodCall(name, argumentList.arguments) &&
+        isNullable(target)) {
       assert(typeArgs == null); // Object methods don't take type args.
       return runtimeCall('#(#, #)', [name, jsTarget, args]);
     }
@@ -5168,16 +5171,6 @@ class CodeGenerator extends Object
     }
   }
 
-  /// Everything in Dart is an Object and supports the 4 members on Object,
-  /// so we have to use a runtime helper to handle values such as `null` and
-  /// native types.
-  ///
-  /// For example `null.toString()` is legal in Dart, so we need to generate
-  /// that as `dart.toString(obj)`.
-  bool _isObjectMemberCall(Expression target, String memberName) {
-    return isObjectMember(memberName) && isNullable(target);
-  }
-
   List<JS.Expression> _getTypeArgs(Element member, DartType instantiated) {
     DartType type;
     if (member is ExecutableElement) {
@@ -5225,8 +5218,8 @@ class CodeGenerator extends Object
     }
 
     JS.Expression result;
-    if (_isObjectMemberCall(receiver, memberName)) {
-      if (_isObjectMethod(memberName)) {
+    if (isObjectMember(memberName) && isNullable(receiver)) {
+      if (_isObjectMethodTearoff(memberName)) {
         result = runtimeCall('bind(#, #)', [jsTarget, jsName]);
       } else {
         result = runtimeCall('#(#)', [memberName, jsTarget]);
@@ -6152,10 +6145,7 @@ class CodeGenerator extends Object
   }
 
   /// Return true if this is one of the methods/properties on all Dart Objects
-  /// (toString, hashCode, noSuchMethod, runtimeType).
-  ///
-  /// Operator == is excluded, as it is handled as part of the equality binary
-  /// operator.
+  /// (toString, hashCode, noSuchMethod, runtimeType, ==).
   bool isObjectMember(String name) {
     // We could look these up on Object, but we have hard coded runtime helpers
     // so it's not really providing any benefit.
@@ -6170,9 +6160,18 @@ class CodeGenerator extends Object
     return false;
   }
 
-  bool _isObjectMethod(String name) =>
+  bool _isObjectMethodTearoff(String name) =>
       name == 'toString' || name == 'noSuchMethod';
 
+  bool _isObjectMethodCall(String name, List<Expression> args) {
+    if (name == 'toString') {
+      return args.isEmpty;
+    } else if (name == 'noSuchMethod') {
+      return args.length == 1 && args[0] is! NamedExpression;
+    }
+    return false;
+  }
+
   // TODO(leafp): Various analyzer pieces computed similar things.
   // Share this logic somewhere?
   DartType _getExpectedReturnType(ExecutableElement element) {

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

@@ -3647,7 +3647,7 @@ class ProgramCompiler extends Object
         // If the receiver is nullable, use a helper so calls like
         // `null.hashCode` and `null.runtimeType` will work.
         // Also method tearoffs like `null.toString`.
-        if (_isObjectMethod(memberName)) {
+        if (_isObjectMethodTearoff(memberName)) {
           return runtimeCall('bind(#, #)', [jsReceiver, jsName]);
         }
         return runtimeCall('#(#)', [memberName, jsReceiver]);
@@ -3768,8 +3768,9 @@ class ProgramCompiler extends Object
     }
 
     var jsName = _emitMemberName(name, member: target);
-    if (isObjectMember(name)) {
-      assert(arguments.types.isEmpty); // Object methods don't take type args.
+
+    // Handle Object methods that are supported by `null`.
+    if (_isObjectMethodCall(name, arguments)) {
       if (isNullable(receiver)) {
         // If the receiver is nullable, use a helper so calls like
         // `null.toString()` will work.
@@ -5055,5 +5056,16 @@ bool isObjectMember(String name) {
   return false;
 }
 
-bool _isObjectMethod(String name) =>
+bool _isObjectMethodTearoff(String name) =>
     name == 'toString' || name == 'noSuchMethod';
+
+bool _isObjectMethodCall(String name, Arguments args) {
+  if (name == 'toString') {
+    return args.positional.isEmpty && args.named.isEmpty && args.types.isEmpty;
+  } else if (name == 'noSuchMethod') {
+    return args.positional.length == 1 &&
+        args.named.isEmpty &&
+        args.types.isEmpty;
+  }
+  return false;
+}