[vm/ffi] Allow subclassing Pointer?

[dcharkes]

dart:ffi provides two ways to instantiate Pointer objects: allocate and fromAddress.

Structs in dart:ffi are modeled as subtypes of Pointer<Void>. For this reason the signature of fromAddress is:

T fromAddress<T extends Pointer>(int ptr);

allocate does not support allocating structs with malloc, as we do not have a way (yet) to know what the size of a struct is. Because of this allocate always returns a Pointer, not a subtype:

Pointer<T> allocate<T extends NativeType>({int count: 1});

With these signatures, allocate can be a factory, static method or top level function, while fromAddress can only be a static method or top level function (as the static return type can be a sub type of Pointer).

We might be able to support allocate of structs in the future, if we have a predefined way to calculate/define the size of structs.

For reference, the current boilerplate of structs:

@ffi.struct
class Coordinate extends ffi.Pointer<ffi.Void> {
  @ffi.Double()
  double x;

  @ffi.Double()
  double y;

  @ffi.Pointer()
  Coordinate next;

  // generated by @ffi.struct annotation
  external static int sizeOf();

  Coordinate offsetBy(int offsetInBytes) =>
      super.offsetBy(offsetInBytes).cast();

  Coordinate elementAt(int index) => offsetBy(sizeOf() * index);

  static Coordinate allocate({int count: 1}) =>
      ffi.allocate<ffi.Uint8>(count: count * sizeOf()).cast();

  /// Allocate a new [Coordinate] in C memory and populate its fields.
  factory Coordinate(double x, double y, Coordinate next) {
    Coordinate result = Coordinate.allocate()
      ..x = x
      ..y = y
      ..next = next;
    return result;
  }
}

Do we prefer a factory constructor for allocate?
Or do we want to keep allocate and fromAddress as similar as possible and change the signature of allocate to support allocating structs as well?
And if so, a top level function or a static method for both?

[lrhn]

Do you plan to write the allocate function manually, or generate it by ffi?

This model looks suspicious to me.

A Coordinate is-a Pointer<Void> and it represents zero or more "coordinates", despite the singular name. The elementAt treats the object as an array, everything else treats it as a single object. That is highly confusing to a reader. The layout and naming suggests that a Coordinate represents one object.
Mixing arrays of objects into the object itself makes modelling problematic. It would be vastly more readable if the Coordinate type only represents a single object. You would then have to do pointer arithmetic outside of the object to get to other objects. (Can you add integers to pointers? And increment their address based on the type of the pointer?).

The allocate function as written doesn't look like it would type-check unless the cast method is magical. You allocate some chunk of memory, then you convert it to a Coordinate object.
In practice, I assume that Coordinate is not the real type of the object, it's just a static adapter type that you can view any pointer as (static extension classes-like functionality).

Is there any possible way this could be written as:

@ffi.struct
class Coordinate {
  @ffi.Double() double x;
  @ffi.Double() double y;
  @ffi.Pointer() Coordinate next;
  extends static in sizeOf();  // why not getter?
  external Pointer<Coordinate> allocate({int count});
  external factory Coordinate.fromAddress(Pointer<Void>);
  /// Write your own factories and custom methods here.
}

which is then automatically compiled as

@ffi.struct
class Coordinate {
  Pointer<Void> get _$address => this as Pointer<Void>;  // my own address.
  double get x => _$address.offsetBy(0).cast<ffi.Double>.load();
  void set x(double value) { _$address.offsetBy(0).cast<ffi.Double>.save(value); }
  ...
  Coordinate get next => _$address.offsetBy(16).cast<ffi.Pointer<Coordinate>>.load();
  void set next(Pointer<Coordinate> value) { _$address.offsetBy(16).cast<ffi.Pointer<Coordinate>>.write(value._$address); }
  ...
  static int sizeOf() => 24;  // Or whatever size we compute from the fields.
  Pointer<Coordinate> allocate({int count}) {
     ffi.allocate<ffi.Uint8>(count: sizeOf() * count).cast<Pointer<Coordinate>>;
  }
  factory Coordinate.fromAddress(Pointer<Void> address) => address as Coordinate;
  // user code.
}

Then we treat the Coordinate type as a zero-size static wrapper around a pointer,
where as Coordinate embeds and as Pointer<Void> projects, without doing any actual conversion.

It's still a little iffy, because Pointer<Coordinate> really does point to a Coordinate. It's really a Pointer<Void>, not a Pointer<Pointer<Void>>. The static type system needs to recognize this, and perhaps even the dynamic type system, so:

Pointer<Coordinate> p = ...;
var p2  = p.offsetBy(2);  // p.cast<Void>.offsetBy(2 * Coordinate.sizeOf()).cast<Coordinate>
Coordinate o = p2.load();  // actually = p2.cast<Void>() as Coordinate

It should be possible to statically detect pointer.offsetBy where the static type is a struct type.
It might be tricky if it's supposed to work generically:

void forEach<T>(int count, Pointer<T> o, void action(T value)) {
  for (int i = 0; i < count; i++) { 
    action(o.offsetBy(i).load());
  }
}

There is no way that will work for things where the type is not known dynamically. The VM might need to be tricky and recognize, at run-time, that a type argument like Coordinate refers to a struct, and embed code to find the size of that.

(I guess this all boils down to how user friendly you want the API to be. If it's a low-level API where you are expected to write all conveniences yourself, then you might not care that the API is idiosyncratic and inconsistent, as long as it allows user code to be simple and efficient. If you want users to use the structs directly in their normal code, they should work as close as possible to a Dart object with the same signature, and should be designed to look like idiomatic classes).

[sjindel-google]

• Subclassing pointer requires us to invent values (boxing) of a type without calling a constructor -- places strange restrictions on subclasses, which might be hard to apply completely
• Subclassing pointer is not as necessary if generalized typedefs are introduced
• If Pointers cannot be subclassed, we don't need to use a boxed representation for the pointer address
• However, it allows more ergonomic native signatures to be written, e.g.:

class Statement extends Pointer<Void> {
  // ...
}

Statement prepare(String query);

[dcharkes]

Quite often the Dart api does not coincide with Pointer-wrapper-classes

class Result extends IterableBase<Row> implements ClosableIterable<Row> {
  final StatementPointer _statement;
  // ...

https://github.com/dart-lang/sdk/blob/master/samples/ffi/sqlite/lib/src/database.dart#L122-L125

This would then have to be:

class StatementPointer {
  Pointer<Void> pointer;
}

[dcharkes]

No we will not allow subtyping, the API will be:

class Statement extends NativeType { }

Pointer<Statement> p;

[ds84182]

Will class Foo extends Void {} be allowed? Or is it limited to only non-zero-sized native types?

[sjindel-google]

I think we're planning to only allow extending Struct for now. However, structs can have size 0 if there are no fields.

[sjindel-google]

When this is implemented, we should ensure that the following code doesn't crash:

import 'dart:ffi';
import 'dart:async';

class DbOptionsPtr extends Pointer<Void> {}

main(args) async {
  DbOptionsPtr opts = fromAddress(0);
  await Isolate.spawn(print, opts);
}

[dcharkes]

@sjindel-google Any thoughts on what we do with CString = Pointer<Uint8>?

See our use of CString in the SQLite sample: samples/ffi/sqlite/lib/src/bindings/signatures.dart and samples/ffi/sqlite/lib/src/bindings/bindings.dart.

Conceptually I would say:

CString = char* = Pointer<Uint8>.

With generalized typedefs and extension methods we could write these things as extension methods:

typedef CString = Pointer<Uint8>
extension CStringMethods on CString {
  // ..
}

You've proposed the following in the CL disallowing subclassing Pointer:

class CString extends Struct {}
Pointer<CString> = Pointer<Uint8>

This seems to equate CString to char instead of char*.

Alternatively we could require manual wrapping, but that does not enable us to write CString in function signatures.

class CString {
  final Pointer<Uint8> chars;
}

[sjindel-google]

My feeling is that typedef/extension method is "obviously" the right solution.

What I proposed (intended to be a temporary workaround) is actually:

class CString extends Struct<CString> {
  Uint8 char;
}
Pointer<CString> = Pointer<Uint8>

Which captures the intuitive connection between memory behind a char* and an inline C array (char foo[...], which open appear in structs).

[dcharkes]

I'm not an expert on the commonalities between inline C arrays and char pointers, but the sqlite3 api just mentions char*.

If we want to keep the mental load of writing signatures and bindings low, we should maybe opt for Pointer<Char>, where we use a class Char extends Struct. The use sites would need to interact primarily through the Pointer instead of the ref:

Pointer<Char> myString;
myString.load<Char>().someMethod();

[sjindel-google]

If we add .ref, this would instead look like:

Pointer<Utf8> myString;
myString.ref.toString();