[vm/ffi] Closure callbacks for async callbacks

This change is almost trivial. The closure is stored on the callback's
RawReceivePort, not in the VM. So we can basically just remove the CFE
check and it pretty much works. The only problem is that we can't set
function.FfiCallbackTarget anymore, so most of the CL is dealing with
that.

A few places were deciding whether an FFI trampoline was a call or a
callback based on whether function.FfiCallbackTarget() was null. But
now the target will be null for async callbacks. So instead I've added
a new value to the FfiCallbackKind enum (and renamed it), and changed
those checks.

Sync callback closures will be a separate CL, because they're more
complicated.

Bug: #52689
Change-Id: I8e5dfb557362e679f66195b735c3c382e6792840
TEST=async_void_function_callbacks_test.dart
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/316160
Commit-Queue: Liam Appelbe <[email protected]>
Reviewed-by: Daco Harkes <[email protected]>

pkg/vm/lib/transformations/ffi/use_sites.dart

@@ -141,9 +141,6 @@ mixin _FfiUseSiteTransformer on FfiTransformer {
         final Expression func = node.arguments.positional[0];
         final DartType dartType = func.getStaticType(staticTypeContext!);
 
-        _ensureIsStaticFunction(
-            func, nativeCallableListenerConstructor.name.text);
-
         ensureNativeTypeValid(nativeType, node);
         ensureNativeTypeToDartType(nativeType, dartType, node);
 
@@ -599,7 +596,7 @@ mixin _FfiUseSiteTransformer on FfiTransformer {
   // void _handler(List args) => target(args[0], args[1], ...)
   // final _callback = NativeCallable<T>._(_handler, debugName);
   // _callback._pointer = _pointerAsyncFromFunction<NativeFunction<T>>(
-  //       _nativeAsyncCallbackFunction<T>(target), _callback._rawPort);
+  //       _nativeAsyncCallbackFunction<T>(), _callback._rawPort);
   // expression result: _callback;
   Expression _replaceNativeCallableListenerConstructor(
       ConstructorInvocation node) {
@@ -622,9 +619,13 @@ mixin _FfiUseSiteTransformer on FfiTransformer {
           functionType: Substitution.fromInterfaceType(listType)
               .substituteType(listElementAt.getterType) as FunctionType));
     }
-    final target = _getStaticFunctionTarget(node.arguments.positional[0]);
-    final handlerBody =
-        ExpressionStatement(StaticInvocation(target, Arguments(targetArgs)));
+    final target = node.arguments.positional[0];
+    final handlerBody = ExpressionStatement(FunctionInvocation(
+      FunctionAccessKind.FunctionType,
+      target,
+      Arguments(targetArgs),
+      functionType: targetType,
+    ));
     final handler = FunctionNode(handlerBody,
         positionalParameters: [args], returnType: VoidType())
       ..fileOffset = node.fileOffset;
@@ -635,7 +636,7 @@ mixin _FfiUseSiteTransformer on FfiTransformer {
             nativeCallablePrivateConstructor,
             Arguments([
               FunctionExpression(handler),
-              StringLiteral('NativeCallable(${target.name})'),
+              StringLiteral('NativeCallable($target)'),
             ], types: [
               targetType,
             ])),
@@ -644,12 +645,12 @@ mixin _FfiUseSiteTransformer on FfiTransformer {
       ..fileOffset = node.fileOffset;
 
     // _callback._pointer = _pointerAsyncFromFunction<NativeFunction<T>>(
-    //       _nativeAsyncCallbackFunction<T>(target), _callback._rawPort);
+    //       _nativeAsyncCallbackFunction<T>(), _callback._rawPort);
     final pointerValue = StaticInvocation(
         pointerAsyncFromFunctionProcedure,
         Arguments([
-          StaticInvocation(
-              nativeAsyncCallbackFunctionProcedure, node.arguments),
+          StaticInvocation(nativeAsyncCallbackFunctionProcedure,
+              Arguments([], types: [targetType])),
           InstanceGet(InstanceAccessKind.Instance, VariableGet(nativeCallable),
               nativeCallablePortField.name,
               interfaceTarget: nativeCallablePortField,
@@ -892,16 +893,6 @@ mixin _FfiUseSiteTransformer on FfiTransformer {
     throw FfiStaticTypeError();
   }
 
-  Procedure _getStaticFunctionTarget(Expression node) {
-    if (node is StaticGet) {
-      return node.target as Procedure;
-    }
-    if (node is ConstantExpression) {
-      return (node.constant as StaticTearOffConstant).target;
-    }
-    throw ArgumentError.value(node, "node", "Not a static function");
-  }
-
   /// Returns the class that should not be implemented or extended.
   ///
   /// If the superclass is not sealed, returns `null`.

pkg/vm/testcases/transformations/ffi/native_callable.dart

@@ -6,6 +6,7 @@ import 'dart:ffi';
 
 void main() {
   testNativeCallableListener();
+  testNativeCallableListenerClosure();
 }
 
 void printInt(int i) => print(i);
@@ -15,3 +16,11 @@ void testNativeCallableListener() {
   print(callback.nativeFunction);
   callback.close();
 }
+
+void testNativeCallableListenerClosure() {
+  int j = 123;
+  void closure(int i) => print(i + j);
+  final callback = NativeCallable<Void Function(Int32)>.listener(closure);
+  print(callback.nativeFunction);
+  callback.close();
+}

pkg/vm/testcases/transformations/ffi/native_callable.dart.aot.expect

@@ -8,19 +8,33 @@ import "dart:ffi";
 
 static method main() → void {
   self::testNativeCallableListener();
+  self::testNativeCallableListenerClosure();
 }
 static method printInt(core::int i) → void
   return [@vm.inferred-type.metadata=dart.core::Null? (value: null)] core::print(i);
 static method testNativeCallableListener() → void {
   final ffi::NativeCallable<(ffi::Int32) → ffi::Void> callback = block {
     final ffi::NativeCallable<(ffi::Int32) → ffi::Void> #t1 = new ffi::NativeCallable::_<(ffi::Int32) → ffi::Void>((final core::List<dynamic> args) → void
-      self::printInt(args.{core::List::[]}(0){(core::int) → dynamic});
-);
-    [@vm.call-site-attributes.metadata=receiverType:dart.ffi::NativeCallable<dart.ffi::Void Function(dart.ffi::Int32)>] [@vm.direct-call.metadata=dart.ffi::NativeCallable._pointer] #t1.{ffi::NativeCallable::_pointer} = [@vm.inferred-type.metadata=dart.ffi::Pointer] ffi::_pointerAsyncFromFunction<ffi::NativeFunction<(ffi::Int32) → ffi::Void>>(ffi::_nativeAsyncCallbackFunction<(ffi::Int32) → ffi::Void>(#C1), [@vm.direct-call.metadata=dart.ffi::NativeCallable._port] [@vm.inferred-type.metadata=dart.isolate::_RawReceivePort] #t1.{ffi::NativeCallable::_port}{iso::RawReceivePort});
+      #C1(args.{core::List::[]}(0){(core::int) → dynamic}){(ffi::Int32) → ffi::Void};
+, "NativeCallable(ConstantExpression(printInt))");
+    [@vm.call-site-attributes.metadata=receiverType:dart.ffi::NativeCallable<dart.ffi::Void Function(dart.ffi::Int32)>] [@vm.direct-call.metadata=dart.ffi::NativeCallable._pointer] #t1.{ffi::NativeCallable::_pointer} = [@vm.inferred-type.metadata=dart.ffi::Pointer] ffi::_pointerAsyncFromFunction<ffi::NativeFunction<(ffi::Int32) → ffi::Void>>(ffi::_nativeAsyncCallbackFunction<(ffi::Int32) → ffi::Void>(), [@vm.direct-call.metadata=dart.ffi::NativeCallable._port] [@vm.inferred-type.metadata=dart.isolate::_RawReceivePort] #t1.{ffi::NativeCallable::_port}{iso::RawReceivePort});
   } =>#t1;
   core::print([@vm.direct-call.metadata=dart.ffi::NativeCallable.nativeFunction] [@vm.inferred-type.metadata=dart.ffi::Pointer] callback.{ffi::NativeCallable::nativeFunction}{ffi::Pointer<ffi::NativeFunction<(ffi::Int32) → ffi::Void>>});
   [@vm.direct-call.metadata=dart.ffi::NativeCallable.close] [@vm.inferred-type.metadata=!? (skip check)] callback.{ffi::NativeCallable::close}(){() → void};
 }
+static method testNativeCallableListenerClosure() → void {
+  core::int j = 123;
+  function closure(core::int i) → void
+    return core::print([@vm.direct-call.metadata=dart.core::_IntegerImplementation.+] [@vm.inferred-type.metadata=int (skip check)] i.{core::num::+}(j){(core::num) → core::int});
+  final ffi::NativeCallable<(ffi::Int32) → ffi::Void> callback = block {
+    final ffi::NativeCallable<(ffi::Int32) → ffi::Void> #t2 = new ffi::NativeCallable::_<(ffi::Int32) → ffi::Void>((final core::List<dynamic> args) → void
+      closure(args.{core::List::[]}(0){(core::int) → dynamic}){(ffi::Int32) → ffi::Void};
+, "NativeCallable(VariableGetImpl(closure))");
+    [@vm.call-site-attributes.metadata=receiverType:dart.ffi::NativeCallable<dart.ffi::Void Function(dart.ffi::Int32)>] [@vm.direct-call.metadata=dart.ffi::NativeCallable._pointer] #t2.{ffi::NativeCallable::_pointer} = [@vm.inferred-type.metadata=dart.ffi::Pointer] ffi::_pointerAsyncFromFunction<ffi::NativeFunction<(ffi::Int32) → ffi::Void>>(ffi::_nativeAsyncCallbackFunction<(ffi::Int32) → ffi::Void>(), [@vm.direct-call.metadata=dart.ffi::NativeCallable._port] [@vm.inferred-type.metadata=dart.isolate::_RawReceivePort] #t2.{ffi::NativeCallable::_port}{iso::RawReceivePort});
+  } =>#t2;
+  core::print([@vm.direct-call.metadata=dart.ffi::NativeCallable.nativeFunction] [@vm.inferred-type.metadata=dart.ffi::Pointer] callback.{ffi::NativeCallable::nativeFunction}{ffi::Pointer<ffi::NativeFunction<(ffi::Int32) → ffi::Void>>});
+  [@vm.direct-call.metadata=dart.ffi::NativeCallable.close] [@vm.inferred-type.metadata=!? (skip check)] callback.{ffi::NativeCallable::close}(){() → void};
+}
 constants  {
   #C1 = static-tearoff self::printInt
 }

pkg/vm/testcases/transformations/ffi/native_callable.dart.expect

@@ -8,19 +8,33 @@ import "dart:ffi";
 
 static method main() → void {
   self::testNativeCallableListener();
+  self::testNativeCallableListenerClosure();
 }
 static method printInt(core::int i) → void
   return core::print(i);
 static method testNativeCallableListener() → void {
   final ffi::NativeCallable<(ffi::Int32) → ffi::Void> callback = block {
     final ffi::NativeCallable<(ffi::Int32) → ffi::Void> #t1 = new ffi::NativeCallable::_<(ffi::Int32) → ffi::Void>((final core::List<dynamic> args) → void
-      self::printInt(args.{core::List::[]}(0){(core::int) → dynamic});
-, "NativeCallable(printInt)");
-    [@vm.call-site-attributes.metadata=receiverType:dart.ffi::NativeCallable<dart.ffi::Void Function(dart.ffi::Int32)>] #t1.{ffi::NativeCallable::_pointer} = ffi::_pointerAsyncFromFunction<ffi::NativeFunction<(ffi::Int32) → ffi::Void>>(ffi::_nativeAsyncCallbackFunction<(ffi::Int32) → ffi::Void>(#C1), #t1.{ffi::NativeCallable::_port}{iso::RawReceivePort});
+      #C1(args.{core::List::[]}(0){(core::int) → dynamic}){(ffi::Int32) → ffi::Void};
+, "NativeCallable(ConstantExpression(printInt))");
+    [@vm.call-site-attributes.metadata=receiverType:dart.ffi::NativeCallable<dart.ffi::Void Function(dart.ffi::Int32)>] #t1.{ffi::NativeCallable::_pointer} = ffi::_pointerAsyncFromFunction<ffi::NativeFunction<(ffi::Int32) → ffi::Void>>(ffi::_nativeAsyncCallbackFunction<(ffi::Int32) → ffi::Void>(), #t1.{ffi::NativeCallable::_port}{iso::RawReceivePort});
   } =>#t1;
   core::print(callback.{ffi::NativeCallable::nativeFunction}{ffi::Pointer<ffi::NativeFunction<(ffi::Int32) → ffi::Void>>});
   callback.{ffi::NativeCallable::close}(){() → void};
 }
+static method testNativeCallableListenerClosure() → void {
+  core::int j = 123;
+  function closure(core::int i) → void
+    return core::print(i.{core::num::+}(j){(core::num) → core::int});
+  final ffi::NativeCallable<(ffi::Int32) → ffi::Void> callback = block {
+    final ffi::NativeCallable<(ffi::Int32) → ffi::Void> #t2 = new ffi::NativeCallable::_<(ffi::Int32) → ffi::Void>((final core::List<dynamic> args) → void
+      closure(args.{core::List::[]}(0){(core::int) → dynamic}){(ffi::Int32) → ffi::Void};
+, "NativeCallable(VariableGetImpl(closure))");
+    [@vm.call-site-attributes.metadata=receiverType:dart.ffi::NativeCallable<dart.ffi::Void Function(dart.ffi::Int32)>] #t2.{ffi::NativeCallable::_pointer} = ffi::_pointerAsyncFromFunction<ffi::NativeFunction<(ffi::Int32) → ffi::Void>>(ffi::_nativeAsyncCallbackFunction<(ffi::Int32) → ffi::Void>(), #t2.{ffi::NativeCallable::_port}{iso::RawReceivePort});
+  } =>#t2;
+  core::print(callback.{ffi::NativeCallable::nativeFunction}{ffi::Pointer<ffi::NativeFunction<(ffi::Int32) → ffi::Void>>});
+  callback.{ffi::NativeCallable::close}(){() → void};
+}
 constants  {
   #C1 = static-tearoff self::printInt
 }

runtime/vm/app_snapshot.cc

@@ -1386,7 +1386,7 @@ class FfiTrampolineDataSerializationCluster : public SerializationCluster {
       AutoTraceObject(data);
       WriteFromTo(data);
       s->Write<int32_t>(data->untag()->callback_id_);
-      s->Write<uint8_t>(data->untag()->callback_kind_);
+      s->Write<uint8_t>(data->untag()->trampoline_kind_);
     }
   }
 
@@ -1415,7 +1415,7 @@ class FfiTrampolineDataDeserializationCluster : public DeserializationCluster {
                                      FfiTrampolineData::InstanceSize());
       d.ReadFromTo(data);
       data->untag()->callback_id_ = d.Read<int32_t>();
-      data->untag()->callback_kind_ = d.Read<uint8_t>();
+      data->untag()->trampoline_kind_ = d.Read<uint8_t>();
     }
   }
 };

runtime/vm/canonical_tables.h

@@ -425,7 +425,7 @@ struct CanonicalFfiCallbackFunctionTraits {
             f1.FfiCSignature() == f2.FfiCSignature() &&
             f1.FfiCallbackExceptionalReturn() ==
                 f2.FfiCallbackExceptionalReturn() &&
-            f1.GetFfiCallbackKind() == f2.GetFfiCallbackKind());
+            f1.GetFfiTrampolineKind() == f2.GetFfiTrampolineKind());
   }
   static bool ReportStats() { return false; }
 };

runtime/vm/compiler/aot/precompiler.cc

@@ -3451,7 +3451,7 @@ void PrecompileParsedFunctionHelper::FinalizeCompilation(
   }
 
   if (function.IsFfiTrampoline() &&
-      function.FfiCallbackTarget() != Function::null()) {
+      function.GetFfiTrampolineKind() != FfiTrampolineKind::kCall) {
     compiler::ffi::SetFfiCallbackCode(thread(), function, code);
   }
 }

runtime/vm/compiler/backend/il_serializer.cc

@@ -835,10 +835,10 @@ void FlowGraphSerializer::WriteTrait<const Function&>::Write(
       s->Write<const Function&>(Function::Handle(zone, x.FfiCallbackTarget()));
       s->Write<const FunctionType&>(
           FunctionType::Handle(zone, x.FfiCSignature()));
-      if (x.FfiCallbackTarget() != Object::null()) {
+      if (x.GetFfiTrampolineKind() != FfiTrampolineKind::kCall) {
         s->Write<const Instance&>(
             Instance::Handle(zone, x.FfiCallbackExceptionalReturn()));
-        s->Write<uint8_t>(static_cast<uint8_t>(x.GetFfiCallbackKind()));
+        s->Write<uint8_t>(static_cast<uint8_t>(x.GetFfiTrampolineKind()));
       } else {
         s->Write<const String&>(String::Handle(zone, x.name()));
         s->Write<const FunctionType&>(
@@ -925,7 +925,8 @@ const Function& FlowGraphDeserializer::ReadTrait<const Function&>::Read(
       const FunctionType& c_signature = d->Read<const FunctionType&>();
       if (!callback_target.IsNull()) {
         const Instance& exceptional_return = d->Read<const Instance&>();
-        FfiCallbackKind kind = static_cast<FfiCallbackKind>(d->Read<uint8_t>());
+        FfiTrampolineKind kind =
+            static_cast<FfiTrampolineKind>(d->Read<uint8_t>());
         return Function::ZoneHandle(
             zone, compiler::ffi::NativeCallbackFunction(
                       c_signature, callback_target, exceptional_return, kind));

runtime/vm/compiler/ffi/call.cc

@@ -47,6 +47,7 @@ FunctionPtr TrampolineFunction(const String& name,
 
   function.SetFfiCSignature(c_signature);
   function.SetFfiIsLeaf(is_leaf);
+  function.SetFfiTrampolineKind(FfiTrampolineKind::kCall);
 
   return function.ptr();
 }

runtime/vm/compiler/ffi/callback.cc

@@ -18,7 +18,7 @@ namespace ffi {
 FunctionPtr NativeCallbackFunction(const FunctionType& c_signature,
                                    const Function& dart_target,
                                    const Instance& exceptional_return,
-                                   FfiCallbackKind kind) {
+                                   FfiTrampolineKind kind) {
   Thread* const thread = Thread::Current();
   Zone* const zone = thread->zone();
   Function& function = Function::Handle(zone);
@@ -28,9 +28,12 @@ FunctionPtr NativeCallbackFunction(const FunctionType& c_signature,
   // Create a new Function named '<target>_FfiCallback' and stick it in the
   // 'dart:ffi' library. Note that these functions will never be invoked by
   // Dart, so they may have duplicate names.
-  const auto& name = String::Handle(
-      zone, Symbols::FromConcat(thread, Symbols::FfiCallback(),
-                                String::Handle(zone, dart_target.name())));
+  const auto& name =
+      kind == FfiTrampolineKind::kSyncCallback
+          ? String::Handle(zone, Symbols::FromConcat(
+                                     thread, Symbols::FfiCallback(),
+                                     String::Handle(zone, dart_target.name())))
+          : Symbols::FfiAsyncCallback();
   const Library& lib = Library::Handle(zone, Library::FfiLibrary());
   const Class& owner_class = Class::Handle(zone, lib.toplevel_class());
   auto& signature = FunctionType::Handle(zone, FunctionType::New());
@@ -47,7 +50,7 @@ FunctionPtr NativeCallbackFunction(const FunctionType& c_signature,
   // the body.
   function.SetFfiCSignature(c_signature);
   function.SetFfiCallbackTarget(dart_target);
-  function.SetFfiCallbackKind(kind);
+  function.SetFfiTrampolineKind(kind);
 
   // We need to load the exceptional return value as a constant in the generated
   // function. Even though the FE ensures that it is a constant, it could still

runtime/vm/compiler/ffi/callback.h

@@ -23,7 +23,7 @@ namespace ffi {
 FunctionPtr NativeCallbackFunction(const FunctionType& c_signature,
                                    const Function& dart_target,
                                    const Instance& exceptional_return,
-                                   FfiCallbackKind kind);
+                                   FfiTrampolineKind kind);
 
 // Builds a mapping from `callback-id` to code object / ...
 //

runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc

@@ -3386,9 +3386,9 @@ Fragment StreamingFlowGraphBuilder::BuildStaticInvocation(TokenPosition* p) {
     case MethodRecognizer::kFfiAsFunctionInternal:
       return BuildFfiAsFunctionInternal();
     case MethodRecognizer::kFfiNativeCallbackFunction:
-      return BuildFfiNativeCallbackFunction(FfiCallbackKind::kSync);
+      return BuildFfiNativeCallbackFunction(FfiTrampolineKind::kSyncCallback);
     case MethodRecognizer::kFfiNativeAsyncCallbackFunction:
-      return BuildFfiNativeCallbackFunction(FfiCallbackKind::kAsync);
+      return BuildFfiNativeCallbackFunction(FfiTrampolineKind::kAsyncCallback);
     case MethodRecognizer::kFfiLoadAbiSpecificInt:
       return BuildLoadAbiSpecificInt(/*at_index=*/false);
     case MethodRecognizer::kFfiLoadAbiSpecificIntAtIndex:
@@ -6321,19 +6321,20 @@ Fragment StreamingFlowGraphBuilder::BuildFfiAsFunctionInternal() {
 }
 
 Fragment StreamingFlowGraphBuilder::BuildFfiNativeCallbackFunction(
-    FfiCallbackKind kind) {
+    FfiTrampolineKind kind) {
   // The call-site must look like this (guaranteed by the FE which inserts it):
   //
-  // FfiCallbackKind::kSync:
+  // FfiTrampolineKind::kSyncCallback:
   //   _nativeCallbackFunction<NativeSignatureType>(target, exceptionalReturn)
   //
-  // FfiCallbackKind::kAsync:
-  //   _nativeAsyncCallbackFunction<NativeSignatureType>(target)
+  // FfiTrampolineKind::kAsyncCallback:
+  //   _nativeAsyncCallbackFunction<NativeSignatureType>()
   //
   // The FE also guarantees that both arguments are constants.
 
   // Target, and for kSync callbacks, the exceptional return.
-  const intptr_t expected_argc = kind == FfiCallbackKind::kSync ? 2 : 1;
+  const intptr_t expected_argc =
+      kind == FfiTrampolineKind::kSyncCallback ? 2 : 0;
 
   const intptr_t argc = ReadUInt();  // Read argument count.
   ASSERT(argc == expected_argc);
@@ -6352,19 +6353,21 @@ Fragment StreamingFlowGraphBuilder::BuildFfiNativeCallbackFunction(
   ASSERT(positional_count == expected_argc);
 
   // Read target expression and extract the target function.
-  code += BuildExpression();  // Build first positional argument (target).
-  Definition* target_def = B->Peek();
-  ASSERT(target_def->IsConstant());
-  const Closure& target_closure =
-      Closure::Cast(target_def->AsConstant()->value());
-  ASSERT(!target_closure.IsNull());
-  Function& target = Function::Handle(Z, target_closure.function());
-  ASSERT(!target.IsNull() && target.IsImplicitClosureFunction());
-  target = target.parent_function();
-  code += Drop();
-
+  Function& target = Function::Handle(Z, Function::null());
   Instance& exceptional_return = Instance::ZoneHandle(Z, Instance::null());
-  if (kind == FfiCallbackKind::kSync) {
+  if (kind == FfiTrampolineKind::kSyncCallback) {
+    // Build first positional argument (target).
+    code += BuildExpression();
+    Definition* target_def = B->Peek();
+    ASSERT(target_def->IsConstant());
+    const Closure& target_closure =
+        Closure::Cast(target_def->AsConstant()->value());
+    ASSERT(!target_closure.IsNull());
+    target = target_closure.function();
+    ASSERT(!target.IsNull() && target.IsImplicitClosureFunction());
+    target = target.parent_function();
+    code += Drop();
+
     // Build second positional argument (exceptionalReturn).
     code += BuildExpression();
     Definition* exceptional_return_def = B->Peek();

runtime/vm/compiler/frontend/kernel_binary_flowgraph.h

@@ -398,7 +398,7 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper {
 
   // Build FG for '_nativeCallbackFunction'. Reads an Arguments from the
   // Kernel buffer and pushes the resulting Function object.
-  Fragment BuildFfiNativeCallbackFunction(FfiCallbackKind kind);
+  Fragment BuildFfiNativeCallbackFunction(FfiTrampolineKind kind);
 
   // Piece of a StringConcatenation.
   // Represents either a StringLiteral, or a Reader offset to the expression.