[vm/ffi] variance of functions in load/store

[dcharkes]

Why are all the arguments (and result) of the function type invariant?

/cc @sjindel-google

Currently we enforce that the arguments and return type of functions are invariant on trampolines.

3cce6fc changed the semantics of Pointer.store and Pointer.load to be consistent with the Dart language semantics (notably implicit downcasts).

We should investigate the use of Pointer<NativeType> as argument or return value, document it's semantics, and write tests for it.

[sjindel-google]

Can we just remove the Dart signature from asFunction and fromFunction altogether?

[dcharkes]

I think there are multiple questions to answer here:

1. Does passing or binding functions with Pointer<NativeType> as argument or return value at all?
2. Should asFunction and fromFunction be invariant (after DartRepresentationOf conversion)?
3. Do the FFI trampolines work properly if you pass sub types as arguments, or does it crash?

RE: 1

Yes, it does. We need to add regression tests.

RE: 2

I think it should be invariant, I don't see any reason to support the following: (see follow up comments)

typedef Int64PointerParamOp = Void Function(Pointer<Int64>);
typedef NaTyPointerParamOp = Void Function(Pointer<NativeType>);

final fp = ffiTestFunctions.lookup<NativeFunction<NaTyPointerParamOp>>(paramOpName);
final f = fp.asFunction<Int64PointerParamOp>();

Can we just remove the Dart signature from asFunction and fromFunction altogether?

For asFunction we could possibly drop the Dart type, as converting from native type to Dart type is a many to one conversion. Then the return type would be dynamic (similar to load).
However, we need to make sure TFA still has enough information to do things efficiently in AOT. Because we're basically moving the invention of the Dart type from the program source to somewhere in the compiler.

For fromFunction does not have a Dart type in the signature. We cannot drop the native type, because we cannot guess native integer sizes.

RE: 3

The Dart type system allows one to pass subtypes of the declared types as argument or return value at runtime.

We invent the return value ourselves on return, so that should be fine. Edit: this also seems to work.
For the arguments we should write tests that pass subtypes of the declared types.

[dcharkes]

4. What should the variance be on load() and store() on Pointer<Pointer<NativeFunction<...>>>?

[sjindel-google]

Re. 2: Following the principle of being consistent with the rest of the language, argument types should be covariant and return types should be contravariant. I don't think the question should be about whether we should "support" something -- we should fit in with the language, so rules which apply to other language constructs need to apply to the FFI as well.

[dcharkes]

T2 == DartRepresentationOf(T1) on Pointer<NativeFunction<T1>>.asFunction<T2> and fromFunction<T1>(T2 f) are arbitrary extra constraints. The language constructs used in asFunction and fromFunction do not mandate something here.

If we do T2 == DartRepresentationOf(T1), then you can still assign the result of asFunction to a T3 with the normal assignability rules (T2 is assignable to T3).

typedef Int64PointerReturnOp = Pointer<Int64> Function();
typedef NaTyPointerReturnOp = Pointer<NativeType> Function();

final fp = ffiTestFunctions.lookup<NativeFunction<Int64PointerReturnOp>>(name);
final f = fp.asFunction<Int64PointerReturnOp>(); // DartRepresentationOf conversion applies.
final NaTyPointerReturnOp f2 = f1; // Normal Dart assignability rules apply.

There is one case in which this would be inconvenient though:

final fp = ffiTestFunctions.lookup<NativeFunction<Int64PointerReturnOp>>(name);
final NaTyPointerReturnOp f2 = fp.asFunction<Int64PointerReturnOp>();

If you leave out the type argument on asFunction it will be inferred and rejected in the above example.

(If we decide to make the return type of asFunction dynamic, then the static type would be dynamic and the dynamic type would be Int64PointerReturnOp in this example.)

If we don't go for invariant, the normal assignability relation applies:

• DartRepresentationOf(T1) is assignable to T2 for Pointer<NativeFunction<T1>>.asFunction<T2>
• T2 is assignable to DartRepresentationOf(T1) for fromFunction<T1>(T2 f)

(Note the inversion of the assignment direction between the types.)

[sjindel-google]

It sounds like you make a good case for why we should remove this arbitrary invariance constraint. Is there any point above which should be interpreted otherwise?

[sjindel-google]

Currently we enforce that the arguments and return type of functions are invariant on trampolines.

Actually, we don't always use invariance when treating function types. Here's an example where the current treatment is incorrect.

This program is accepted:

import 'dart:ffi';

main() {
  Pointer<NativeFunction<Void Function(Pointer<NativeType>)>> fn = fromAddress(0);
  Pointer<Pointer<NativeFunction<Void Function(Pointer<Double>)>>> ptr = allocate();
  ptr.store(fn);
}

[dcharkes]

Currently we enforce that the arguments and return type of functions are invariant on trampolines.

Actually, we don't always use invariance when treating function types. Here's an example where the current treatment is incorrect.

I meant for asFunction and fromFunction. But you're right, this is what I meant with point 4, and we have to fix this as well.

It looks like that the example that you wrote should actually be accepted. It is analogous with:

final void Function(Pointer<NativeType>) fn; // = ...
final List<void Function(Pointer<Double>)> container = [];
container.add(fn); // This is fine.
final fn2 = container.first; // Static type `void Function(Pointer<Double>)`.
final arg = Pointer<Double> p = allocate();
fn2(p); // This is fine.

But we should reject the following cases:

Pointer<NativeFunction<Void Function(Pointer<Double>)>> fn = fromAddress(0);
Pointer<Pointer<NativeFunction<Void Function(Pointer<NativeType>)>>> ptr = allocate();
ptr.store(fn);

Pointer<NativeFunction<Pointer<NativeType> Function()>> fn = fromAddress(0);
Pointer<Pointer<NativeFunction<Pointer<Double> Function()>>> ptr = allocate();
ptr.store(fn);

[dcharkes]

RE 2 & 3: I started writing some tests for these, and I realized we have a lot of cases:

• parameter subtyping (and implicit downcasts) in asFunction
• parameter subtyping (and implicit downcasts) in trampoline from Dart to C
• return value subtyping (and implicit downcasts) in asFunction
• return value subtyping (and implicit downcasts) in trampoline from Dart to C
• parameter subtyping (and implicit downcasts) in fromFunction
• parameter subtyping (and implicit downcasts) in trampoline from C to Dart
• return value subtyping (and implicit downcasts) in fromFunction
• return value subtyping (and implicit downcasts) in trampoline from C to Dart

For the first two groups (parameters subtyping in calls from Dart to C):

Pointer<NativeFunction<Void Function(Pointer<T>)>> fp;
void Function(Pointer<S>) f = fp.asFunction<S>();
Pointer<Q> arg;
f(arg);

T, S, and Q can all be Int64 or NativeType statically.
If Q is NativeType statically, it can be Int64 dynamically.
If S and T is Int64 statically, they can be NativeType dynamically (arguments are contravariant).
But, S cannot vary dynamically from the static type in asFunction.

This gives us 6 cases for asFunction and 9 for the function application.

We'll have a similar amount of cases for return values, for parameters in callbacks, and return values (and error return values) in callbacks.

/cc @lrhn @mkustermann @mraleph

[dcharkes]

Maybe we should disallow Pointer<NativeType> (and Pointer<Struct>, Pointer<NativeInteger>, Pointer<NativeDouble>, and Pointer<...<NativeType>...> etc.) instantiation at runtime. We regard those as abstract. That way one could still write generic code, but always have something specific at runtime. This would cut out a bunch of cases in these tests.

We would need to constrain trampoline return values (in asFunction), callback trampoline arguments (in fromFunction), fromAddress, and allocate to exclude those types statically. Those are the operations that create Pointers at runtime from the static type arguments. We do not need to constrain load, as it only uses the runtime type arguments.

(edit: I remember that's why I originally disallowed NativeType statically in some places months ago. Then we re-allowed NativeType statically to enable writing generic code. However, we also re-allowed having NativeType at runtime in 3cce6fc.)

[sjindel-google]

Implicit downcasts only apply to values -- not to type parameters, which is the subject of this issue.

I don't think we should be re-implementing the type system here. How can we remove the Dart signature and only use the native one? asFunction can return dynamic and any casts are implemented by the normal type-system implementation of the VM.

[dcharkes]

Implicit downcasts only apply to values -- not to type parameters, which is the subject of this issue.

Incorrect.

  List<List<int>> n = [
    [1, 2, 3]
  ];
  List<List<Object>> n2 = n;
  n2.add([3.4]);

Unhandled exception:
type 'List<Object>' is not a subtype of type 'List<int>' of 'value'
#0      List.add (dart:core-patch/growable_array.dart)
...

How can we remove the Dart signature and only use the native one?

That would make asFunction and fromFunction both T2 == DartRepresentationOf(T1) (invariant) by construction.
For asFunction we could return dynamic - but then TFA cannot optimize it, you lose IDE support, but gain conciseness.
For fromFunction I don't really know what we could do. We have to specify the C signature somewhere.
The trampolines themselves have to be able to deal with subtypes (implicit casts for super types are inserted by the compiler).

I don't think we should be re-implementing the type system here.

I agree, but how? :)

[sjindel-google]

Implicit downcasts only apply to values -- not to type parameters, which is the subject of this issue.

Incorrect.

  List<List<int>> n = [
    [1, 2, 3]
  ];
  List<List<Object>> n2 = n;
  n2.add([3.4]);

Unhandled exception:
type 'List<Object>' is not a subtype of type 'List<int>' of 'value'
#0      List.add (dart:core-patch/growable_array.dart)
...

That is still an example of a value-level downcast -- the parameter to add, which is a value. But your example illustrates exactly what I mean :)

Here's a similar example with function types instead:

int foo(int x) => 0;

main() {
  int Function(int) fn0 = foo;
  fn0(3);  // No check
  dynamic fn1 = foo;
  fn1("abc");  // Check fails
}

My point is just that a statically-typed closure call-site does not need to perform any dynamic checks. Therefore, one could write their FFI bindings like this:

DynamicLibrary lib = null;  // ...

int Function(int) thing = lib.lookupFunction<Int64 Function(Int64)>("thing");

main() {
  thing(10);  // No checks performed (uses "unchecked" entry-point from MEP).
}

I don't understand the problems you're imagining here. There are no subclasses of Pointer, int or double, so the trampolines can still make assumptions about the exact class IDs of the inputs. The Dart signature is calculated as a function of the native one, and any checks between the actual type of the closure and the calculated Dart signature is done as usual. TFA does not struggle with functions that return dynamic -- it can infer its own (often more precise) return type. Although it doesn't support function types at all yet :)

[sjindel-google]

As discussed offline, we will lock out all dynamic invocations of Pointer-related methods, so the runtime checks will go away.

[lrhn]

Implicit downcasts only apply to values -- not to type parameters, which is the subject of this issue.

Incorrect.

Nitpick: What you see here is not an implicit downcast, it's a covariant parameter check.

An implicit down-cast happens when the static type of an expression is a proper supertype of the static context type it appears in. We then do an implicit down-cast to the context type. This test can then fail at run-time.

A covariant parameter check occurs when a method parameter is covariant, either because it's declared using covariant or because it's using a type parameter covariantly. That's not guaranteed to be type-safe: A List<int> can be used at static type List<Object>, and it's add method's covariant parameter cannot accept any Object as argument. So the method adds an extra check to ensure that its argument is actually of the correct type. This test can fail at run-time.

Completely different, same result :)

With NNBD, we will disallow implicit downcasts - we will no longer insert the cast, but just make the code a compile-time error. We will not change covariant parameter checks.

[dcharkes]

The scope of this issue is fixing the variance of function types in load and store.

Issue # 4 (asFunction and fromFunction API) moves to its own issue: #37464

[dcharkes]

When we solve #35902, this issue will be a non issue.

[sjindel-google]

This bug can be easily fixed on its own, or else will be addressed in #37464.

[dcharkes]

This is part of the CL that addresses Pointer optimizations (#38172), and is currently in review.