Skip to content
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

[vm/ffi] Make FfiNatives more concise #50097

Open
Tracked by #50565
dcharkes opened this issue Sep 30, 2022 · 11 comments
Open
Tracked by #50565

[vm/ffi] Make FfiNatives more concise #50097

dcharkes opened this issue Sep 30, 2022 · 11 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi

Comments

@dcharkes
Copy link
Contributor

Our current syntax for "ffi-natives" is still a bit verbose.

@FfiNative<Int64 Function(Int64, Int64)>('sum')
external int sum(int a, int b);

1. Rename FfiNative to Native

We could consider @Ffi<...> and @Native<...>.

sum is an external function.
sum is a native function.
sum is not an "ffi" function, the mechanism for calling sum is "ffi".

This makes it most natural to change the syntax to:

@Native<Int64 Function(Int64, Int64)>('sum')
external int sum(int a, int b);

Courtesy of @mit-mit and @mkustermann.

2. Make the symbol optional

If the symbol in C is identical to the Dart symbol, we don't need to repeat it.

@Native<Int64 Function(Int64, Int64)>()
external int sum(int a, int b);

Wish list. Don't write dart types

The native types uniquely define the Dart types, so it would be nice if we could omit the Dart types

@Native()
external Int64 sum(Int64 a, Int64 b);

@Native()
external Void foo(Pointer<Int64> a, Int64 b);

However, we would need some kind of transformation before any Dart static analysis runs to replace the native types with the corresponding Dart code. Otherwise, calling a function with Dart integers will not pass type checks.

We would likely need macros for this, if our approach for macros could already support this.

Misc

The old natives are deprecated, so we don't have to worry about naming collision (having to designate the new natives as "new native" or "ffi native"). FWIW the syntax of the deprecated old natives:

@pragma("vm:external-name", "File_OpenStdio")
external static int _openStdio(int fd);

See d8d7af1.

@dcharkes dcharkes added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi labels Sep 30, 2022
@mraleph
Copy link
Member

mraleph commented Sep 30, 2022

Wish list. Don't write dart types

I have been thinking about this and discussing this with @leafpetersen couple of times. My original thinking was that we could use views for this, though I don't think the current version of views has all necessary features.

If CFE retained typedef reference in the AST rather than performed full substitution then we could simply write:

// dart:ffi
typedef Void = void;
// ...
typedef Int32 = int;
typedef Int64 = int;
typedef Float = double;
typedef Double = double;

// ...

@Native()
external Int64 sum(Int64 a, Int64 b);

@Native()
external Void foo(Pointer<Int64> a, Int64 b);

Though unfortunately right now CFE is going the opposite way (/cc @johnniwinther @chloestefantsova) and is considering to delete TypedefType from the public AST. This node is not used for non-function type aliases anyway.

@Wdestroier
Copy link

@mraleph Java Native Access works with reflections to get the return and parameter types. Considering static metaprogramming is a replacement for dart:mirrors, and the equivalent of reflections in Dart is dart:mirrors, I believe static metaprogramming would be the most correct feature to help solve this problem.

@mraleph
Copy link
Member

mraleph commented Oct 6, 2022

@Wdestroier I don't think static metaprogramming is going to help us much here. It can't change signatures of the existing methods it can only generate new methods. The problem we are facing here does not really exist in Java - as Java has the full zoo of numeric types. In Dart however there is just int and double.

@leafpetersen
Copy link
Member

@mraleph is the primary shortcoming you see with views the inability to use literals (or in general values) of the representation type as initializers? Or are there other places it falls short?

@mraleph
Copy link
Member

mraleph commented Oct 8, 2022

@leafpetersen I think if we had a way to declare conversions both ways as implicit (both to and from representation type) & CFE preserved view type, rather than fully erasing it, then I think views would do the job.

Essentially we want the following to be a valid code:

@Native()
external Int64 sum(Int64 a, Int64 b);

int useSum(List<int> v) {
  return v.reduce((a, b) => sum(a, b));
  // Maybe even the following, but probably okay if this does not type check. 
  // return v.reduce(sum);
}

In other words, we don't really need much of advanced view functionality - what we want is a typedef that survives in the AST representation until backend.

@Wdestroier
Copy link

Similar to allowing the overload of implicit and explicit is and as operators...

@dcharkes
Copy link
Contributor Author

@leafpetersen I think if we had a way to declare conversions both ways as implicit (both to and from representation type) & CFE preserved view type, rather than fully erasing it, then I think views would do the job.

Essentially we want the following to be a valid code:

@Native()
external Int64 sum(Int64 a, Int64 b);

int useSum(List<int> v) {
  return v.reduce((a, b) => sum(a, b));
  // Maybe even the following, but probably okay if this does not type check. 
  // return v.reduce(sum);
}

In other words, we don't really need much of advanced view functionality - what we want is a typedef that survives in the AST representation until backend.

This is not really the full picture. We also use the native types (currently subtypes of NativeType) for other types of type checks. For example it is now allowed to assign a Pointer<Int8> to a Pointer<Int16>. If we were using typedefs, these assignments would not be flagged anymore by the type system.

  • Do view disable assignment to other views on the same underlying type?

Moreover, we'd have to change Pointer's type argument bound to be Object. Unless views can implement another type. Then we could let those views implement NativeType.

  • Can views implement another type?

(Side note: we want to stop reifying the type argument of Pointer at runtime (#49935). However, we want to keep all static checks that we currently get from using native types as type arguments.)

@leafpetersen
Copy link
Member

@mraleph

declare conversions both ways as implicit (both to and from representation type)

My general take is that if we want to support this, it should probably be a separate first class feature that can be used for any type.

& CFE preserved view type, rather than fully erasing it

If I understand correctly, this is more of an implementation detail around how the CFE represents these?

@dcharkes

  • Do view disable assignment to other views on the same underlying type?

Yes.

  • Can views implement another type?

Views can be made subtypes of other view types. I'm not sure if that covers your use case or not. We have considered allowing views to implement class types in the case that the underlying representation type also implements that class type. That is currently not in the proposal though.

@dcharkes
Copy link
Contributor Author

  1. Rename FfiNative to Native

We need to add @Native and deprecate @FfiNative so that we don't break dart:ui in Flutter on the roll. Then after a roll, we can migrate dart:ui and then remove @FfiNative.

@dcharkes
Copy link
Contributor Author

If I'm going to modify the API, I might as well take the assets into account as well. I've created #49803 (comment) with an API design (1) and (2).

copybara-service bot pushed a commit that referenced this issue Oct 24, 2022
Bug: #49803
Bug: #50097

Change-Id: Id5b52be88937bcf9245f98e71afa56f079f288f0
Cq-Include-Trybots: luci.dart.try:analyzer-linux-release-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/265085
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
@johnniwinther
Copy link
Member

View types will be part of the AST since we need them to do static reasoning on precompiled code. The implementation will be similar to the current implementation of the experimental ExtensionType:

class ViewType extends DartType {
  final View view;
  final List<DartType> typeArguments;
  final DartType representationType;

  ...
}

where representationType is the type used at runtime. For instance for

view class View<T>(T it) {}
View<int> method() => ...

the representation type of View<int> will by int.

copybara-service bot pushed a commit that referenced this issue Jan 18, 2023
See API design discussion in bugs below.

TEST=tests/ffi/native_assets/*

Design doc: http://go/dart-native-assets

Bug: #49803
Bug: #50097

Change-Id: Id6e6eb94c6eb39ccaaa637448583a40ab6110d12
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-precomp-win-debug-x64c-try,vm-ffi-android-debug-arm64c-try,vm-ffi-android-debug-arm-try,dart2wasm-linux-x64-d8-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/265084
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
copybara-service bot pushed a commit that referenced this issue Feb 6, 2023
Bug: #50097
Change-Id: I8d555e27167d13d21d88a2961a0501155119409b
CoreLibraryReviewExempt: Isn't used in dart2js, is used in dart2wasm.
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279514
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi
Projects
None yet
Development

No branches or pull requests

5 participants