Skip to content

[vm/ffi] Introduce SizedNativeType #54542

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

Open
1 of 2 tasks
dcharkes opened this issue Jan 8, 2024 · 3 comments
Open
1 of 2 tasks

[vm/ffi] Introduce SizedNativeType #54542

dcharkes opened this issue Jan 8, 2024 · 3 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi P3 A lower priority bug or feature request triaged Issue has been triaged by sub team

Comments

@dcharkes
Copy link
Contributor

dcharkes commented Jan 8, 2024

Split off from discussion on:

TODO:

  • Use in sizeOf and AllocatorAlloc.call.
  • Use in Struct/Union documentation and error messages.
@dcharkes dcharkes added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi P3 A lower priority bug or feature request labels Jan 8, 2024
@a-siva a-siva added the triaged Issue has been triaged by sub team label Jan 11, 2024
copybara-service bot pushed a commit that referenced this issue Jan 12, 2024
Both `sizeOf` and `AllocatorAlloc.call` use the new type as bound.
This prevents runtime errors on trying to call these two static
methods with unsized native types. All tests testing for these runtime
errors have been deleted.

The `NativeTypes` implementing `SizedNativeType` are as follows:
* The native integer types, `Float`, and `Double`.
* `AbiSpecificInteger` and it's subtypes.
* `Struct` and `Union` and their subtypes. The

The `NativeTypes` not implementing `SizedNativeType` are as follows:
* `Void` has no size.
* `Opaque` and subtypes have unknown size.
* `Handle` is considered opaque. Cannot be used as field in compounds.
* `Array` does not carry a size in its type. Can be used as fields in
  compounds with an annotation specifying the size.
* `NativeFunction` is considered opaque. Would have variable size.
* `VarArgs` is only a marker in function signatures.

`Struct`s and `Union`s can have only `SizedNativeType`s and `Array`s
as fields. Documentation for these is in flux in another CL, so we
should update it there.

This CL also replaces a bunch of `extends NativeType` with
`implements` clauses and made `NativeType` itself `abstract`.

TEST=Dart SDK build
TEST=ffi test suite

Bug: #54542
CoreLibraryReviewExempt: VM and dart2wasm feature only.
Change-Id: Ib4f6b58f7204bd063ace20133162798d8c9483e8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345221
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Daco Harkes <[email protected]>
@rainyl
Copy link

rainyl commented Jul 25, 2024

I am wondering whether common operations or properties like Pointer<Uint8>.value, Pointer<Uint8> operator + (int offset) can also be added to SizedNativeType? This will simplify the usage like:

num getValue(int i0, int i1) {
  final ffi.Pointer<ffi.Uint8> pdata = dataPtr.$1;
  final (int, int, int) step = this.step;
  final MatType type = this.type;

  // https://github.com/opencv/opencv/blob/71d3237a093b60a27601c20e9ee6c3e52154e8b1/modules/core/include/opencv2/core/mat.inl.hpp#L894
  final ptr = pdata + i0 * step.$1;
  return switch (type.depth) {
    MatType.CV_8U => (ptr.cast<ffi.Uint8>() + i1).value,
    MatType.CV_8S => (ptr.cast<ffi.Int8>() + i1).value,
    MatType.CV_16U => (ptr.cast<ffi.Uint16>() + i1).value,
    MatType.CV_16S => (ptr.cast<ffi.Int16>() + i1).value,
    MatType.CV_32S => (ptr.cast<ffi.Int>() + i1).value,
    MatType.CV_32F => (ptr.cast<ffi.Float>() + i1).value,
    MatType.CV_64F => (ptr.cast<ffi.Double>() + i1).value,
    MatType.CV_16F => float16((ptr.cast<ffi.Uint16>() + i1).value),
    _ => throw UnsupportedError("Unsupported type: $type")
  };
}

As you can see, without common operations/fields defined in SizedNativeType, I have to test the desired pointer type to cast, but if they were defined, the above code can be simplified to:

num getValue<T extends ffi.SizedNativeType>(int i0, int i1) {
  final pdata = dataPtr.$1;
  final step = this.step;
  // https://github.com/opencv/opencv/blob/71d3237a093b60a27601c20e9ee6c3e52154e8b1/modules/core/include/opencv2/core/mat.inl.hpp#L894
  final ptr = pdata + i0 * step.$1;
  return (ptr.cast<T>() + i1).value;
}

@dcharkes
Copy link
Contributor Author

I am wondering whether common operations or properties like Pointer<Uint8>.value, Pointer<Uint8> operator + (int offset) can also be added to SizedNativeType? This will simplify the usage like:

That will unfortunately not work. We cannot branch on the runtime type argument of Pointer. The type argument is always Never at runtime, because our goal is that a Pointer is just an int in the Dart runtime:

@rainyl
Copy link

rainyl commented Jul 25, 2024

get it, thx~

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 P3 A lower priority bug or feature request triaged Issue has been triaged by sub team
Projects
None yet
Development

No branches or pull requests

3 participants