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

Re-design struct API to facilitate reference semantics #35840

Closed
dcharkes opened this issue Feb 4, 2019 · 4 comments
Closed

Re-design struct API to facilitate reference semantics #35840

dcharkes opened this issue Feb 4, 2019 · 4 comments
Assignees
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-ffi type-design

Comments

@dcharkes
Copy link
Contributor

dcharkes commented Feb 4, 2019

Structs are a bit different from the primitives supported in the FFI. Since structs are value types in C++, and Dart does not have value types, structs cannot be used with load and store. Instead, we provide a way to load from and store into the fields of the structs.

We can represent structs in different ways:

  • struct Coordinate extends Pointer<Void>
  • struct Coordinate extends NativeType and use it as Pointer<Coordinate>
  • struct Coordinate extends NativeType and wraps Pointer<Void>
  • struct Coordinate extends NativeType and type wraps Pointer<Void>

Coordinate extends Pointer<Void>

We define Coordinate as:

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

  external static int sizeOf();
  external Coordinate allocate({int count});
  external factory Coordinate.fromAddress(int address);

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

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

  factory Coordinate(double x, double y, Coordinate next) {
    Coordinate result = Coordinate.allocate()
      ..x = x
      ..y = y
      ..next = next;
    return result;
  }
}

which we can then use as objects:

Coordinate c1 = Coordinate(10.0, 10.0, null);
Coordinate c2 = Coordinate(20.0, 20.0, c1);
Coordinate c3 = Coordinate(30.0, 30.0, c2);
c1.next = c3;

or as C++ pointer:

Coordinate c1 = Coordinate.allocate(count: 3);
Coordinate c2 = c1.elementAt(1);
Coordinate c3 = c1.elementAt(2);
c1.x = 10.0;
c1.y = 10.0;
c1.next = c3;
// ...

Pros:

  • concise use of struct (field access)
  • reasonably concise definition of structs

Cons:

  • Conflates the C++ type Coordinate and the Dart pointer to a Coordinate in a single type, which is confusing.
  • offsetBy and elementAt return Pointer<Void> and thus have to be wrapped -> this could be solved by having a StructMixin<Coordinate> that makes them return the right type -> but this doesn't work for static methods and factories

Coordinate extends NativeType

We define Coordinate as:

@struct
class Coordinate extends NativeType {
  @Double() double x;
  external static double getX(Pointer<Coordinate> pointer);
  external static void setX(Pointer<Coordinate> pointer, double x);

  @Double() double y;
  external static double getY(Pointer<Coordinate> pointer);
  external static void setY(Pointer<Coordinate> pointer, double y);

  @Pointer() Coordinate next;
  external static Pointer<Coordinate> getNext(Pointer<Coordinate> pointer);
  external static void setNext(Pointer<Coordinate> pointer, Pointer<Coordinate> next);

  external static int sizeOf();
  external static Pointer<Coordinate> allocate({int count});
  external static Pointer<Coordinate> fromAddress(int address);

  Pointer<Coordinate> elementAt(Pointer<Coordinate> pointer, int index) =>
    pointer.offsetBy(sizeOf() * index);

  static Pointer<Coordinate>(double x, double y, Coordinate next) {
    Pointer<Coordinate> result = Coordinate.allocate();
    setX(result, x);
    setY(result, y);
    setNext(result, next);
    return result;
  }
}

Pros:

  • Coordinate represents C++ struct, Pointer<Coordinate> is a Dart wrapper
  • offsetBy works properly out of the box

Cons:

  • Verbose use site of structs (static methods) -> extension methods would help here @mit-mit
  • Verbose definition of structs (extra static methods for fields)

Coordinate extends NativeType and wraps Pointer<Void>

@struct
class Coordinate extends NativeType {
  Pointer<Void> pointer;

  // ...
}

Pros:

  • Coordinate represents the Dart wrapper with access to the fields, while the field Pointer<Void> provides the C++ view with pointer arithmetic

Cons:

  • use sites are less concise as they have to switch between Dart view and C++ view
  • overhead of having two objects to wrap a C++ struct instead of one

Coordinate as type wrapper of Pointer<Void>

Suggested by @lrhn in #35782 (comment)

Pros & cons: same as Coordinate wraps Pointer<Void> but without the overhead of an extra object.

Extra con: Dart has no facilities to provide type wrappers, so we would have to program this by hand.

Looping in @lrhn, @mit-mit, and @sjindel-google

@dcharkes dcharkes added the area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. label Feb 4, 2019
@dcharkes
Copy link
Contributor Author

To align the struct and non struct api's and to support struct by value (#36730), we have settled on the following:

@struct Coordinate extends NativeType {
  @Int32 int x;
  @Int32 int y;
}

And uses will look like the following

Pointer<Coordinate> p;
Coordinate c = p.load(); // Which creates a 'reference', it does not actually load the value.
int x = c.x;             // This loads the value.

We're still debating whether in some cases we do want to load the whole struct.

In any case, we have settled on the api and syntax, so I'm closing this issue.

@sjindel-google sjindel-google self-assigned this Jun 3, 2019
@sjindel-google sjindel-google changed the title dart:ffi Struct extends Pointer<Void> vs Pointer<Struct> vs Struct wraps Pointer<Void> Re-design struct API to facilitate reference semantics Jun 3, 2019
@sjindel-google
Copy link
Contributor

sjindel-google commented Jun 3, 2019

In order to load the address of a struct, we need at least:

class Struct<T> {
  Pointer<T> get addressOf;
}

class SomeStruct extends Struct<SomeStruct> {
// ...
}

@MarvinHannott
Copy link

To align the struct and non struct api's and to support struct by value (#36730), we have settled on the following:

@struct Coordinate extends NativeType {
  @Int32 int x;
  @Int32 int y;
}

And uses will look like the following

Pointer<Coordinate> p;
Coordinate c = p.load(); // Which creates a 'reference', it does not actually load the value.
int x = c.x;             // This loads the value.

We're still debating whether in some cases we do want to load the whole struct.

In any case, we have settled on the api and syntax, so I'm closing this issue.

But this is not yet fully implemented, I presume?

@sjindel-google
Copy link
Contributor

@MarvinHannott

That remark is old, the new API (which is implemented) is outlined in #37229

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-ffi type-design
Projects
None yet
Development

No branches or pull requests

3 participants