[Breaking Change Request] [vm/ffi] Disallow invoking ffi methods with generics

[dcharkes]

Update 2021-01-22: We will deprecate invoking the methods with generics in Dart 2.12 and remove support in 2.13.

Summary

Disallow invoking the following methods in dart:ffi with a generic and with Pointer with a generic type argument.

sdk/sdk/lib/ffi/ffi.dart

Lines 24 to 27 in 897b1a2

	/// Number of bytes used by native type T.
	///
	/// Includes padding and alignment of structs.
	external int sizeOf<T extends NativeType>();

sdk/sdk/lib/ffi/ffi.dart

Lines 63 to 64 in 897b1a2

	/// Pointer arithmetic (takes element size into account).
	external Pointer<T> elementAt(int index);

sdk/sdk/lib/ffi/ffi.dart

Lines 535 to 547 in 897b1a2

	/// Creates a reference to access the fields of this struct backed by native
	/// memory at [address].
	///
	/// The [address] must be aligned according to the struct alignment rules of
	/// the platform.
	external T get ref;
	/// Creates a reference to access the fields of this struct backed by native
	/// memory at `address + sizeOf<T>() * index`.
	///
	/// The [address] must be aligned according to the struct alignment rules of
	/// the platform.
	external T operator [](int index);

And introduce the following to reduce boilerplate:

/// Manages memory on the native heap.
abstract class Allocator {
  /// This interface is meant to be implemented, not extended or mixed in.
  Allocator._() {
    throw UnsupportedError("Cannot be instantiated");
  }

  /// Allocates [byteCount] bytes of memory on the native heap.
  ///
  /// If [alignment] is provided, the allocated memory will be at least aligned
  /// to [alignment] bytes.
  ///
  /// Throws an [ArgumentError] if the number of bytes or alignment cannot be
  /// satisfied.
  Pointer<T> allocate<T extends NativeType>(int byteCount, {int? alignment});

  /// Releases memory allocated on the native heap.
  ///
  /// Throws an [ArgumentError] if the memory pointed to by [pointer] cannot be
  /// freed.
  void free(Pointer pointer);
}

/// Extension on [Allocator] to provide allocation with [NativeType].
extension AllocatorAlloc on Allocator {
  /// Allocates `sizeOf<T>() * count` bytes of memory using
  /// `this.allocate`.
  ///
  /// This extension method must be invoked with a compile-time constant [T].
  external Pointer<T> call<T extends NativeType>([int count = 1]);
}

Motivation

This will enable us to do multiple things.

1. Tree shaking of user-defined subclasses of Struct ([vm/ffi] Support treeshaking of FFI structs #38721).
2. Optimize invocations of this code by directly inlining the size ([vm/ffi] Optimize Pointer<Struct> operations #38648).
3. Standardize user-defined allocators: arena style allocation (sample), bump pointer allocation ([vm/ffi] Support treeshaking of FFI structs #38721 (comment)), and wrappers to track allocation.

Impact

package:ffi's allocate<T extends NativeType>() will stop working.

User code using sizeOf, .ref, and elementAt generically will stop working. However, at this moment we are not aware of any code written in such way. This breaking change request is to assess whether code such as that is out there.

This requires changes API to take an int argument for the size or offset and let their callees invoke sizeOf with a non-generic type argument.

// Before breaking change.
final pointer = allocate<Int8>(count: 10);

// After breaking change (without boilerplate reduction).
final pointer = allocate<Int8>(sizeOf<Int8>() * 10);

Boilerplate reduction for allocation

Repeating the type argument is inconvenient. To address this we introduce the Allocator interface and call extension method on Allocator to dart:ffi (see summary).

// Before breaking change.
final pointer = allocate<Int8>(count: 10);

// After breaking change.
final pointer = allocator<Int8>(10);

Proposed changes: https://dart-review.googlesource.com/c/sdk/+/177705/10/sdk/lib/ffi/allocation.dart

Migration of package:ffi

We remove allocate from package:ffi.

We introduce CallocAllocator and CallocAllocator which implement the Allocator interface and consts calloc and calloc.

class CallocAllocator implements Allocator {
  Pointer<T> allocate<T extends NativeType>(int numBytes);

  void free(Pointer pointer);
}

const calloc = CallocAllocator();

Proposed changes: dart-archive/ffi@b072465#diff-33eeafb9384f3239bb7f203cab73b7d099a5caa20a2033ef0a28b8019a57d647

Migration of users of package:ffi

// Before breaking change.
final pointer = allocate<Int8>(count: 10);
free(pointer);

// After breaking change.
final pointer = calloc<Int8>(10);
calloc.free(pointer);

Example migrations:

• dart-archive/ffi@b072465#diff-fbcc582ff238b1dda707c40a42b3f55c1a767262b72f72fbd824c31ddd8ff42a (the rest of the changed files)
• https://dart-review.googlesource.com/c/sdk/+/177706

Discussion

Zero-initialize memory

We have two options for initializing memory to zero.

1. Add calloc besides malloc to package:ffi. This would use the calloc on all non-Windows OSes, and HEAP_ZERO_MEMORY flag for HeapAlloc on Windows. (This is the option that is prototyped.)
2. Add a named argument bool zeroInitialize = false to allocate and Allocator.allocate.

In the first one, the implementor of the Allocator has control over whether something is zero-initialized. In the latter one, the caller of allocate has control over whether something is zero-initialized.

Requires-constant-type-arguments

The need to disallow .ref on Pointer<T extends Struct> stems from the fact that Dart does not support calling new T();

We could explore adding a pragma that would inline function bodies, or duplicate them with instantiated type arguments, as a more general solution to this problem.

@pragma('requires-constant-type-arguments')
T instantiateSomethingGeneric<T extends Object>(){
  // The following would be allowed because of the pragma.
  return new T();
}

class MyClass {}

void main(){
  final myClass = instantiateSomethingGeneric<MyClass>();
  print(myClass);
}

When exploring this we should check what this kind of logic would to to code size.

Thanks to @mraleph for this suggestion.

[dcharkes]

https://dart-review.googlesource.com/c/sdk/+/177705/8/sdk/lib/ffi/allocation.dart#24

@lrhn suggested using an extension method instead of a top level method for allocate:

// Before breaking change.
final pointer = allocate<Int8>(count: 10);
free(pointer);

// After breaking change with extension method `alloc` (`allocate` conflicts with instance method).
final pointer = malloc.alloc<Int8>(10);
malloc.free(pointer);

// After breaking change with extension method `call`.
final pointer = malloc<Int8>(10);
malloc.free(pointer);

The call option would result in the following with arena allocation and bump allocation (#38721 (comment)):

final pointer = pool<Int8>(10);
pool.freeAll();

final pointer = bumpAllocator<Int8>(10);
bumpAllocator.free(pointer);

We have 3 options:

1. Allocator.allocate instance method + Malloc.call extension method (prototyped in linked CLs).
2. Allocator.allocate instance method + Malloc.alloc extension method.
3. Allocator.allocateBytes instance method + Malloc.allocate extension method.

Any preferences?

[franklinyow]

@dcharkes Please make an announcement to https://groups.google.com/a/dartlang.org/g/announce

cc @Hixie @matanlurey @vsmenon for review and approval.

[Hixie]

I have no opinion on this and thus no objection. I would encourage this change to be amply documented, in particular with the error message someone would see included in the documentation so that if they google for it they will find it.

[vsmenon]

lgtm

fyi - @leafpetersen - if we expect more general use for compile-time known type params.

[matanlurey]

I don't work in this area, no comment.

[dcharkes]

@dcharkes Please make an announcement to https://groups.google.com/a/dartlang.org/g/announce

Announcement sent: https://groups.google.com/a/dartlang.org/g/announce/c/kZOGrc6-rIg

[franklinyow]

Approved

[dcharkes]

We have landed deprecation hints for invoking the methods with d74b2f4 and f74c9d4.

Turning these hints into errors and making use of the static types to do tree shaking (#38721) and optimizations (#38648) is tracked in the respective issues. Closing this issue.

[pezi]

I am honest - I am Dart nebiew and for my first little hobby project
https://pub.dev/packages/dart_periphery
I use ffi to maniplate low level structs - e.g. i2c_msg for the I2C bus.

Currently I build complete native structs on the Dart side. It will possible to migrate this code according the new ffi changes?
Sorry for my missing knowlege to understand the upcoming changes in their full range.

/// Helper class mapped to the C struct `
class NativeI2Cmsg extends Struct {
  @Int16()
  int addr;
  @Int16()
  int flags;
  @Int16()
  int len;
  Pointer<Int8> buf;
  factory NativeI2Cmsg.allocate() => allocate<NativeI2Cmsg>().ref;
}

static Pointer<NativeI2Cmsg> _toNative(List<I2Cmsg> list) {
    final ptr = allocate<NativeI2Cmsg>(count: list.length);
    var index = 0;
    for (var data in list) {
      var msg = ptr.elementAt(index++);
      msg.ref.addr = data.addr;
      msg.ref.len = data.len;
      var flags = 0;
      if (data.flags.isNotEmpty) {
        for (var f in data.flags) {
          flags |= I2CmsgFlags2Int(f);
        }
      }
      msg.ref.flags = flags;
      msg.ref.buf = allocate<Int8>(count: data.len);
      if (data.predefined.isNotEmpty) {
        var count = 0;
        for (var value in data.predefined) {
          msg.ref.buf.elementAt(count++).value = value;
        }
      }
    }
    return ptr;
  }

[dcharkes]

Yes, just fire up the IDE with the latest beta release to get guidance from the analyzer hints.

/// Helper class mapped to the C struct `
class NativeI2Cmsg extends Struct {
  @Int16()
  external int addr;
  @Int16()
  external int flags;
  @Int16()
  external int len;
  external Pointer<Int8> buf;
}

Pointer<NativeI2Cmsg> _toNative(List<I2Cmsg> list) {
  final ptr = calloc<NativeI2Cmsg>(list.length);
  var index = 0;
  for (var data in list) {
    final msg = ptr.elementAt(index++);
    msg.ref.addr = data.addr;
    msg.ref.len = data.len;
    var flags = 0;
    if (data.flags.isNotEmpty) {
      for (var f in data.flags) {
        flags |= I2CmsgFlags2Int(f);
      }
    }
    msg.ref.flags = flags;
    msg.ref.buf = calloc<Int8>(data.len);
    if (data.predefined.isNotEmpty) {
      var count = 0;
      for (var value in data.predefined) {
        msg.ref.buf.elementAt(count++).value = value;
      }
    }
  }
  return ptr;
}

[maks]

Sorry @dcharkes its very late in the game to ask about this, but just to double check, now that this change has been made, allocating for generics as described in: https://stackoverflow.com/questions/68018694/type-safe-memory-allocation-with-dart-ffi is now not possible?

[dcharkes]

@maks We already support that use case now with providing your own Allocator class. Your specific use case is already supported with the Arena allocator from package:ffi. https://stackoverflow.com/questions/68018694/type-safe-memory-allocation-with-dart-ffi/69611982#69611982