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] Revamp structs API to separate structs and pointers #37229

Closed
sjindel-google opened this issue Jun 11, 2019 · 1 comment
Closed

[vm/ffi] Revamp structs API to separate structs and pointers #37229

sjindel-google opened this issue Jun 11, 2019 · 1 comment
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi

Comments

@sjindel-google
Copy link
Contributor

sjindel-google commented Jun 11, 2019

Problem

The representation of structs in the existing API leaves much to be desired:

C:

struct Str {
  int thing;
};

Dart:

@struct
class Str extends Pointer<Str> {
  @IntPtr()
  int thing;
}

Struct classes are defined as pointers, which conflates the concepts of the struct as a composite value and as a pointer to memory. When by-value structs are supported, there would be no (type-level) way to differentiate between value structs and pointers to structs. For example, the following two signatures in C would correspond to the same signature in Dart:

C:

void thing1(Str* x);
void thing2(Str x);

Dart:

typedef Void Function(Str) thing1Type;
typedef Void Function(Str) thing2Type;

Since struct classes are pointers, they feature the load() and store() operations. Unfortunately, load() and store() load/store a pointer recursively (since the target type of the load is Str, which is itself a pointer), and that's incorrect. Rather, load() should return a reference to the struct, as in C++:

Dart:

Str x = // ...
Str y = x.load();  // Loads "thing" from x as a Pointer, obviously incorrect.

C:

Str* x = //...
Str y = *x; // Loads a reference from "x" (and dereferences it to populate "y").

Pointer has a corresponding storage class in the VM defined (RawPointer), but can be subclassed by user code. We have to prohibit (non-FFI) fields from the subclasses to ensure the memory layout is consistent, but this design works against the VM architecture and there are complications with serialization and GC.

New API

Defining structs

Struct definitions are changed as follows:

// Base class of all structs.
class Struct<S> {
  final Pointer<S> addressOf;
};

// Example struct.
struct Str extends Struct<Str> {
  @Int8()
  int thing;
};

The type argument to Struct<S> will be removed when we can define addressOf as an extension method on Struct.

Changes to Pointer

We will disallow subclassing or implementing Pointer (#35782). We will also replace load() and store() to reduce syntactic overhead for working with structs and distinguish between loading a value vs. a reference from pointers (#37284):

class Pointer<T extends NativeType> {
  // ...
  dynamic get val;  // Cannot be used if T extends Struct.
  void set val(dynamic _);  // Cannot be used if T extends Struct.
  T get ref;  // Can *only* be used if T extends Struct.
  // ...
}

val and ref will become extension methods when extension methods are supported.

Accessing fields

Instead of representing pointers, instances of the Struct subclasses will represent references. A reference is always backed by a pointer, but exposes additional methods on top of it. load() (ref) against a pointer to a struct returns a reference backed by the receiving pointer. When "by-value" structs are supported, it will also be possible to store() (val=) an entire struct into a pointer by passing in a reference. For example:

Dart:

Pointer<Str> x = Pointer.allocate<Str>();
x.thing;  // Not allowed.
x.ref.thing;  // Allowed.
x.ref.thing = 1;  // Allowed
Str xref = x.ref;
Str y = someFn();
x.val = y;  // After #36730
x.free();
xref.thing;  // Undefined behavior: backing pointer was released.

Other changes

This change in struct representation allows us to fix other warts of our API:

Migration

There are other use-cases for extending Pointer besides structs, which will need to be handled differently in the new API. For example, the CString class currently looks like:

class CString extends Pointer<Uint8> {
  static String fromUtf8(CString str) { ... }
  factory CString(String str) { ... }
}

In the new API, this might instead look like:

class Utf8 extends Struct<Utf8> {
  @ffi.Uint8();
  int char;

  String toString();
  static Pointer<Utf8> fromString(String str);
}

When extension methods are available, this could be written more gracefully as follows:

class Utf8 extends Struct<Utf8> {
  @ffi.Uint8();
  int char;
}

extension ConvertToUtf8 on String {
  Pointer<Utf8> toUtf8();
}

extension ConvertFromUtf8 on Pointer<Utf8> {
  String toString();
}
@sjindel-google sjindel-google self-assigned this Jun 11, 2019
@sjindel-google sjindel-google added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi labels Jun 11, 2019
@sjindel-google sjindel-google changed the title [vm/ffi] Update Struct API to enable passing/returning structs by value [vm/ffi] Update Struct API to enable future support for passing/returning structs by value Jun 13, 2019
@sjindel-google sjindel-google changed the title [vm/ffi] Update Struct API to enable future support for passing/returning structs by value [vm/ffi] Revamp structs API to separate structs and pointers Jun 25, 2019
@dcharkes
Copy link
Contributor

Thanks for the great write up of all the changes we decided on! 👍

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

2 participants