From d82be109a2673917e72f0e2eeee31d96faf7dbdd Mon Sep 17 00:00:00 2001
From: Sigmund Cherem <sigmund@google.com>
Date: Fri, 9 Feb 2024 01:59:46 +0000
Subject: [PATCH] [jsinterop] fix lowering of callMethod unchecked calls.

The lowering for external calls works in multiple steps:
- we first expand it into a `js_util.callMethod` call (which uses .apply
  internally)
- which we later refine in to a `js_util._callMethodUncheckedN` call, if
  possible (which calls the member directly)

The second step only happens if we can statically verify the arity of
the method being called and that every argument passed doesn't need a
allow-interop check. The latter is bypassed if we know from the types
that it cannot be a Dart Function.

The new JS interop always satisfies the check, but the second step above
failed to regonize it because it didn't account for extension types. This
CL does the incremental fix to recognize it. Long term, we should
instead change the lowering to use directly the js_interop_unsafe
methods, and tailor the lowering to the new interop.

Fixes https://github.com/dart-lang/sdk/issues/54862

Change-Id: Ieee560e5cd6bd9b6921368477bf8212cae5a1faa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/351221
Commit-Queue: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
---
 .../lib/src/transformations/js_util_optimizer.dart            | 1 +
 pkg/front_end/test/spell_checking_list_tests.txt              | 1 +
 .../extension_types/external.dart.strong.transformed.expect   | 4 ++--
 .../extension_types/external.dart.weak.transformed.expect     | 4 ++--
 .../extension_types/external.dart.strong.transformed.expect   | 4 ++--
 .../extension_types/external.dart.weak.transformed.expect     | 4 ++--
 6 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart b/pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart
index c8cd9e8ea7c..cad1e2525df 100644
--- a/pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart
+++ b/pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart
@@ -671,6 +671,7 @@ class JsUtilOptimizer extends Transformer {
   /// Returns whether the given DartType is guaranteed to be not a function
   /// and therefore allowed to interop with JS.
   bool _allowedInteropType(DartType type) {
+    type = type.extensionTypeErasure;
     if (type is InterfaceType) {
       return type.classNode != _coreTypes.functionClass &&
           type.classNode != _coreTypes.objectClass;
diff --git a/pkg/front_end/test/spell_checking_list_tests.txt b/pkg/front_end/test/spell_checking_list_tests.txt
index 32567bde15e..6c0af482383 100644
--- a/pkg/front_end/test/spell_checking_list_tests.txt
+++ b/pkg/front_end/test/spell_checking_list_tests.txt
@@ -113,6 +113,7 @@ builddir
 bulkcompile
 busy
 busywait
+bx
 bye
 c's
 ca
diff --git a/pkg/front_end/testcases/dart2js/extension_types/external.dart.strong.transformed.expect b/pkg/front_end/testcases/dart2js/extension_types/external.dart.strong.transformed.expect
index fa5a49dcc1f..6439e61aa93 100644
--- a/pkg/front_end/testcases/dart2js/extension_types/external.dart.strong.transformed.expect
+++ b/pkg/front_end/testcases/dart2js/extension_types/external.dart.strong.transformed.expect
@@ -84,14 +84,14 @@ static method method(self::A a) → void {
   a = js_2::getProperty<self::A>(b1, "field");
   js_2::setProperty<self::A>(b1, "field", a);
   a = js_2::_callMethodUnchecked0<self::A>(b1, "method");
-  b2 = js_2::callMethod<self::B% /* = self::A */>(b2, "genericMethod", <dynamic>[b2]);
+  b2 = js_2::_callMethodUnchecked1<self::B% /* = self::A */>(b2, "genericMethod", b2);
   b1 = js_2::getProperty<self::B /* = self::A */>(b2, "getter");
   js_2::setProperty<self::B /* = self::A */>(b1, "setter", b2);
   js_2::setProperty<self::B /* = self::A */>(b1, "property", js_2::getProperty<self::B /* = self::A */>(b2, "property"));
   a = js_2::getProperty<self::A>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticField");
   js_2::setProperty<self::A>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticField", a);
   a = js_2::_callMethodUnchecked0<self::A>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticMethod");
-  b2 = js_2::callMethod<self::B% /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticGenericMethod", <dynamic>[b2]);
+  b2 = js_2::_callMethodUnchecked1<self::B% /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticGenericMethod", b2);
   b1 = js_2::getProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticGetter");
   js_2::setProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticSetter", b2);
   js_2::setProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticProperty", js_2::getProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticProperty"));
diff --git a/pkg/front_end/testcases/dart2js/extension_types/external.dart.weak.transformed.expect b/pkg/front_end/testcases/dart2js/extension_types/external.dart.weak.transformed.expect
index fa5a49dcc1f..6439e61aa93 100644
--- a/pkg/front_end/testcases/dart2js/extension_types/external.dart.weak.transformed.expect
+++ b/pkg/front_end/testcases/dart2js/extension_types/external.dart.weak.transformed.expect
@@ -84,14 +84,14 @@ static method method(self::A a) → void {
   a = js_2::getProperty<self::A>(b1, "field");
   js_2::setProperty<self::A>(b1, "field", a);
   a = js_2::_callMethodUnchecked0<self::A>(b1, "method");
-  b2 = js_2::callMethod<self::B% /* = self::A */>(b2, "genericMethod", <dynamic>[b2]);
+  b2 = js_2::_callMethodUnchecked1<self::B% /* = self::A */>(b2, "genericMethod", b2);
   b1 = js_2::getProperty<self::B /* = self::A */>(b2, "getter");
   js_2::setProperty<self::B /* = self::A */>(b1, "setter", b2);
   js_2::setProperty<self::B /* = self::A */>(b1, "property", js_2::getProperty<self::B /* = self::A */>(b2, "property"));
   a = js_2::getProperty<self::A>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticField");
   js_2::setProperty<self::A>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticField", a);
   a = js_2::_callMethodUnchecked0<self::A>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticMethod");
-  b2 = js_2::callMethod<self::B% /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticGenericMethod", <dynamic>[b2]);
+  b2 = js_2::_callMethodUnchecked1<self::B% /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticGenericMethod", b2);
   b1 = js_2::getProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticGetter");
   js_2::setProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticSetter", b2);
   js_2::setProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticProperty", js_2::getProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticProperty"));
diff --git a/pkg/front_end/testcases/dartdevc/extension_types/external.dart.strong.transformed.expect b/pkg/front_end/testcases/dartdevc/extension_types/external.dart.strong.transformed.expect
index fa5a49dcc1f..6439e61aa93 100644
--- a/pkg/front_end/testcases/dartdevc/extension_types/external.dart.strong.transformed.expect
+++ b/pkg/front_end/testcases/dartdevc/extension_types/external.dart.strong.transformed.expect
@@ -84,14 +84,14 @@ static method method(self::A a) → void {
   a = js_2::getProperty<self::A>(b1, "field");
   js_2::setProperty<self::A>(b1, "field", a);
   a = js_2::_callMethodUnchecked0<self::A>(b1, "method");
-  b2 = js_2::callMethod<self::B% /* = self::A */>(b2, "genericMethod", <dynamic>[b2]);
+  b2 = js_2::_callMethodUnchecked1<self::B% /* = self::A */>(b2, "genericMethod", b2);
   b1 = js_2::getProperty<self::B /* = self::A */>(b2, "getter");
   js_2::setProperty<self::B /* = self::A */>(b1, "setter", b2);
   js_2::setProperty<self::B /* = self::A */>(b1, "property", js_2::getProperty<self::B /* = self::A */>(b2, "property"));
   a = js_2::getProperty<self::A>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticField");
   js_2::setProperty<self::A>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticField", a);
   a = js_2::_callMethodUnchecked0<self::A>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticMethod");
-  b2 = js_2::callMethod<self::B% /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticGenericMethod", <dynamic>[b2]);
+  b2 = js_2::_callMethodUnchecked1<self::B% /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticGenericMethod", b2);
   b1 = js_2::getProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticGetter");
   js_2::setProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticSetter", b2);
   js_2::setProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticProperty", js_2::getProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticProperty"));
diff --git a/pkg/front_end/testcases/dartdevc/extension_types/external.dart.weak.transformed.expect b/pkg/front_end/testcases/dartdevc/extension_types/external.dart.weak.transformed.expect
index fa5a49dcc1f..6439e61aa93 100644
--- a/pkg/front_end/testcases/dartdevc/extension_types/external.dart.weak.transformed.expect
+++ b/pkg/front_end/testcases/dartdevc/extension_types/external.dart.weak.transformed.expect
@@ -84,14 +84,14 @@ static method method(self::A a) → void {
   a = js_2::getProperty<self::A>(b1, "field");
   js_2::setProperty<self::A>(b1, "field", a);
   a = js_2::_callMethodUnchecked0<self::A>(b1, "method");
-  b2 = js_2::callMethod<self::B% /* = self::A */>(b2, "genericMethod", <dynamic>[b2]);
+  b2 = js_2::_callMethodUnchecked1<self::B% /* = self::A */>(b2, "genericMethod", b2);
   b1 = js_2::getProperty<self::B /* = self::A */>(b2, "getter");
   js_2::setProperty<self::B /* = self::A */>(b1, "setter", b2);
   js_2::setProperty<self::B /* = self::A */>(b1, "property", js_2::getProperty<self::B /* = self::A */>(b2, "property"));
   a = js_2::getProperty<self::A>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticField");
   js_2::setProperty<self::A>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticField", a);
   a = js_2::_callMethodUnchecked0<self::A>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticMethod");
-  b2 = js_2::callMethod<self::B% /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticGenericMethod", <dynamic>[b2]);
+  b2 = js_2::_callMethodUnchecked1<self::B% /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticGenericMethod", b2);
   b1 = js_2::getProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticGetter");
   js_2::setProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticSetter", b2);
   js_2::setProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticProperty", js_2::getProperty<self::B /* = self::A */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticProperty"));