-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[vm/ffi] Support passing Array
to Pointer
arguments in leaf calls
#54739
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
How would the typing work? An There could be an |
We have two types to work with in FFI, so the code would be as follows: @Native<Void Function(Pointer<Int8>)>(isLeaf: true)
external void myFunc(Array<Int8>);
main() {
final Array<Int8> array = ... ;
myFunc(array);
} So we would relax the type checking between the types in |
Our arrays, structs, unions are sort of value types. They are copied when passed as arguments / returned. That's also the semantics in C. What we don't support atm (compared to C) is the address-of operator: e.g. passing a pointer to a struct via @Native<Void Function(Pointer<Int8>)>(isLeaf: true)
external void myFunc(Pointer<Int8>);
main() {
final Array<Int8> array = ...;
myFunc(array.address); // or addressOf(array)
} Now the trouble with that is that If we only allowed this functionality on declarative natives, where we see on call site the native we're calling, we could enforce statically that => That would allow us to express in Dart what one can express in C, but we'd only allow to express it in places where it's safe. It would also be a step into the direction of simplifying the annotations to: @Native(isLeaf: true)
external void myFunc(Pointer<Int8>); i.e. allow leaving native types away if they agree with the types in Dart signature. There's another issue with the proposal i #54739 (comment): A tool like => We want to be able to pass real (The downside would be that we wouldn't allow the address-of operator anywhere else, including when using the non-declarative API, as those operate on arbitrary closures.) The next question is how to model the API, one could think of several use cases one uses in C void takesPointer(void* p);
foo() {
int8 array[4][4];
takesPointer(&array[0][1]);
struct mystruct foo;
takesPointer(&foo.bar);
uniontype bar;
takesPointer(bar.x);
} that we'd may want to be able to expression in Dart. |
We have the same issue with TypedData -> Pointer, FFIgen will just generate a Pointer as Dart type. So we should then maybe consider giving that the same treatment? Probably through an extension method on typed data in All of this would be much more natural if we had a typed-data-base and array/struct/union would be extension types and expose field offset getters (#41237). If we introduce a
I don't really like the idea of diverging the two APIs. Because we're going to end up with a feature request at some point for supporting something similar with the dynamic API. Also, the new API also can be torn-off, and passed around as closure. We only require the definitions to be static, not the call sites. But I kind of do like the idea of Some wild thoughts that don't really work out: What if we'd make |
Seems reasonable that we'd have then an address-of operator for TypedData as well.
Yes, it's not ideal that the declarative API has more powerful capability then the dynamic one, but we hope most people will move towards declarative API anyway. It wouldn't compromise safety when using dynamic api or tear-offs, it just means those callers cannot use the address-of operator. I think the issue with |
We wouldn't need the operator if we had typed-data-base with a type argument: // dart:ffi
///
final class PossiblyDartPointer<T extends NativeType> {
}
And then on leaf-calls we accept that one instead.
Then we have the type carry the fact that it would have a |
Yeah agreed. Because if this we should possibly back out of https://dart-review.googlesource.com/c/sdk/+/338620. |
Looping in @mraleph here as well. I think it's somewhat beneficial if we keep the
It depends what we want the operator to support. If we want to support the C-equivalent to |
Notes from discussion with @mkustermann:
class MyStruct extends Struct implements PossiblyDartPointer<MyStruct> {
@Int8()
external int x;
@Int8()
external int y;
@AddressOf(#y)
external PossiblyDartPointer<Int8> addressOfY;
}
interface class PossiblyDartPointer<T extends NativeType> {
external int operator -(PossiblyDartPointer other);
}
main() {
final MyStruct = Struct.create<MyStruct>();
final offsetOfY = myStruct.addressOfY - myStruct; // Works for TypedDatas as well.
} This code resembles what one would do in C: Maybe if we want to support the same for main() {
final offset = Struct.addressOf<MyStruct>(#y) - Struct.addressOf<MyStruct>(#x);
} And that would work for when you don't actually have an instance of The alternative approach for struct field offsets is to have class MyStruct extends Struct {
@Int8()
external int x;
@Int8()
external int y;
} Then if you'd want to make a pointer to a struct field if you know the struct is backed by a Pointer, you should keep the pointer around: main() {
Pointer<MyStruct> p = calloc();
Pointer<Int8> = Pointer.fromAddress(p.address + Struct.offsetOf<MyStruct>(#y));
} As proposed earlier in: #41237 (comment)
Would macros be able to help with this? |
Had some discussions with @mraleph and we came to the conclusion that for symmetry, expressibility, performance it may be best to have class Foo extends Struct {
@Int8
external int value;
}
@Native<void Function(Pointer<Foo>, Pointer<Int8>)>()
external void myNative(Pointer<Foo> fooP, Pointer<Int8> intP);
main() {
Foo foo = Struct<Foo>.create();
myNative(addressOf(foo), addressOf(foo, #value)); // Could get inferred type argument, restricting `foo` to be `NativeType`
print(offsetOf<Foo>(#value));
} We would
We have discussed (and discarded):
It's kind of weird to use that special type in the signatures now that we already have |
Cycling back to extension Int8ListAddressOfExtension on Int8List {
/// ...
///
/// Only callable in `@Native(isLeaf: true)` calls.
external Pointer<Int8> get address;
} The same can be said to retain the type argument of However, the syntax would be more natural with The
This also uses overloading of |
This reverts: https://dart-review.googlesource.com/c/sdk/+/338620 We'd like to support this use case with a different API. See the discussion in #54739. Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/349340 Cherry-pick-request: #54771 Change-Id: I23ced1bace50a110f590f85a8a62e7f2ca80e304 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/349320 Commit-Queue: Daco Harkes <[email protected]> Reviewed-by: Martin Kustermann <[email protected]>
This reverts: https://dart-review.googlesource.com/c/sdk/+/338620 We'd like to support this use case with a different API. See the discussion in #54739. TEST=tests/ffi Bug: #44589 Bug: #54771 Change-Id: Ic22fbcab14d374bb9c81bba1f1bf6ae2dfc9e674 Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-aot-win-debug-x64c-try,vm-aot-win-release-x64-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-fuchsia-release-x64-try,vm-kernel-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-ubsan-linux-release-x64-try,vm-win-debug-arm64-try,vm-win-debug-x64-try,vm-win-debug-x64c-try,vm-win-release-ia32-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/349340 Commit-Queue: Daco Harkes <[email protected]> Reviewed-by: Martin Kustermann <[email protected]>
Need to be clear about what that means. The "most correct" and still flexible definition would be: Can only be used in tail position of arguments of calls to native leaf functions. A leaf function must not call back into Dart, and it must not store its arguments for later use. Doing so is unsupported and unspecified behavior. While technically, we could allow passing the pointer to any function, as long as it doesn't use it after calling back into Dart or returning to Dart, but there is no way to verify that. But then, there is no way to verify that an An expression being in tail position of another expression means, informally, being a subexpression whose value will be the value of the entire expression, and that no computation happens in the expression after evaluating the tail expression, so the value is not used in any way inside the expression itself. At least if the tail expression gets evaluated at all — it can be in a branch that's not always taken.
That's basically any expression position which is not a receiver or an argument. It deliberately omits Then it's a compile-time error to have an invocation of We can also define tail position of elements, but that shouldn't be needed here, not unless needing to pass a list of pointers to a native function. Which should probably be an That's probably allowable, we'll just have to recognize the form specifically: A pure struct creation expression is a struct creation expression About prefix operators. You have |
I'll go ahead and start prototyping this. To be continued with comments once I run into things. As a small side note. If we have signatures with |
For getting the address of `Struct` fields and `Array` elements it would be useful to access the `_typedDataBase` field in these 'wrappers' in a unified way. This CL makes `Array` extend `_Compound`. Since `Array` is not a `SizedNativeType`, the implements clauses for `Struct` and `Union` are moved to these classes. Moreover, any references to 'compound' which only apply to struct or union are updated. TEST=test/ffi Bug: #44589 Bug: #54739 Bug: #41237 CoreLibraryReviewExempt: No API change, only refactoring. Change-Id: Ib9d8bccd4872df04bcc67731e4052f826ab70af4 Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-aot-win-debug-x64-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-fuchsia-release-x64-try,vm-kernel-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-ubsan-linux-release-x64-try,vm-win-debug-arm64-try,vm-win-debug-x64-try,vm-win-release-ia32-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/350960 Reviewed-by: Martin Kustermann <[email protected]> Reviewed-by: Liam Appelbe <[email protected]>
This makes it explicit on the call site what the integer is, simplifying follow up CLs. TEST=tests/ffi CoreLibraryReviewExempt: implementation change only. Bug: #54739 Change-Id: Ib5bc09f4194e9103cf42eb3851b2308553fd8990 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360640 Commit-Queue: Daco Harkes <[email protected]> Reviewed-by: Tess Strickland <[email protected]>
After #45508,
Array
s of ints and floats can be passed toPointer
arguments of FFI leaf calls.We could consider also accepting
Array<T>
where we have aPointer<T>
argument for leaf calls. We could even allow this for structs/unions.The text was updated successfully, but these errors were encountered: