Skip to content

[ffi] Use inline classes for pointer arithmetic? #52320

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
dcharkes opened this issue May 9, 2023 · 13 comments
Open

[ffi] Use inline classes for pointer arithmetic? #52320

dcharkes opened this issue May 9, 2023 · 13 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 type-enhancement A request for a change that isn't a bug

Comments

@dcharkes
Copy link
Contributor

dcharkes commented May 9, 2023

We could consider using inline classes for pointer arithmetic in dart:ffi.

  • sizeOf<IntPtr>() could return a Size.
  • Pointer.address could return an Address.
  • Size can be multiplied with integers and returns a Size.
  • Address and Size can be added which results in an Address.
  • [vm/ffi] Expose struct field offsets #41237 would return an Offset
  • Offset can be added to an Address resulting in a Address.
  • Pointer.fromAddress takes an Address.

However, this would be a breaking change.

@leafpetersen do we have a migration path to using inline classes?

@dcharkes dcharkes added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi labels May 9, 2023
@eernstg
Copy link
Member

eernstg commented May 9, 2023

It should certainly be possible to add some static type checks on the use of ints as addresses etc. as described.

There is no way to emulate inline classes in the case where it is used to get additional static checks. However, you could migrate to an approach using inline classes by introducing typedef Address = int; etc. now, and then replace those type aliases by inline class declarations later on. This implies that source code could be prepared for the switch, but there wouldn't be any static type checks (so maybe it's worse than nothing).

That's the only notion of a migration path that I can think of here.

There is one interesting twist: The description of the operator + on Address seems to call for static overloading or union types, such that it is possible to have both myAddress + aSize and myAddress + anOffset. However, the following would suffice, if it doesn't disrupt other parts of the envisioned design:

// Using primary constructors for conciseness. That feature
// has not yet been accepted by the language team.

inline class AddressDistance(int value) {}
inline class Offset(int value) implements AddressDistance {}
inline class Size(int value) implements AddressDistance {}

inline class Address(int value) {
  Address operator +(AddressDistance distance) => Address(value + distance.value);
}

@dcharkes
Copy link
Contributor Author

dcharkes commented May 9, 2023

This implies that source code could be prepared for the switch

We have no clue how much code is out there, and no way to signal it using typedefs.

Should we consider using language versioning on this like with class modifiers for the standard libraries?
E.g. all types that are inline classes in Dart 3.x, are considered the type they wrap in Dart 3.(x-1). Then we can (1) give error messages to prompt users, and (2) we don't break users the moment that Dart 3.x comes out.

@leafpetersen
Copy link
Member

How is this currently done?

@dcharkes
Copy link
Contributor Author

dcharkes commented May 9, 2023

How is this currently done?

Just int everywhere.

@leafpetersen
Copy link
Member

Just brainstorming - If we were to provide the ability to make inline classes assignable to/from their representation types, you could potentially change things over to be open inline classes, which would be less (but still at least somewhat) breaking (anywhere anyone is currently assigning to an Address to an int would continue to work). You could then have a long-term deprecation period after which you close the inline classes and make them abstract (possibly you could add some custom warnings in the interim to nudge people over). In general, migration of core libraries is hard.

@dcharkes
Copy link
Contributor Author

dcharkes commented May 9, 2023

Just brainstorming - If we were to provide the ability to make inline classes assignable to/from their representation types, you could potentially change things over to be open inline classes, which would be less (but still at least somewhat) breaking (anywhere anyone is currently assigning to an Address to an int would continue to work). You could then have a long-term deprecation period after which you close the inline classes and make them abstract (possibly you could add some custom warnings in the interim to nudge people over). In general, migration of core libraries is hard.

That sounds like a plausible migration path. I'm assuming people using //@dart=3.0 would get support for the automatic boxing/unboxing of open inline classes in that case.

If open is a keyword, then where do we put the @Deprecated to tell them that this thing is going to be a closed inline class later?

@leafpetersen
Copy link
Member

I'm assuming people using //@dart=3.0 would get support for the automatic boxing/unboxing of open inline classes in that case.

I don't know, I think that's more of a custom compiler thing, right?

If open is a keyword, then where do we put the @Deprecated to tell them that this thing is going to be a closed inline class later?

There's no built in support for this, I think you'd have to look into a custom analyzer warning or hint.

Note that we have not committed to (or even really come up with a design for) the ability to have both open and closed inline classes yet, but it's a thing we've discussed.

cc @sigmundch

@eernstg
Copy link
Member

eernstg commented May 9, 2023

If we do support the subtyping relation Address <: int by an implements relationship (which is a choice that we've discussed for quite some time) then we'd have something like the following:

inline class Address(int value) implements int {
  ...
}

void main() {
  Address a = Address(7658765);
  int i = a; // OK, `Address <: int`.
}

If we support assignability for the opposite conversion using implicit constructors then it could be something like this:

// Library 'ffi_something.dart'.

static extension AddressExtension on Address {
  // Make the `Address` constructor available implicitly.
  implicit factory Address.fromInt(int value) = Address.new;
}

// Library 'main.dart'.
import 'ffi_something.dart';

void main() {
  Address a = 7658765; // OK, because the implicit constructor is imported.
}

This means that it's easy to exit the abstraction (int i = myAddress; is accepted everywhere, and so is List<int> xs = <Address>[];). It is not quite as easy to enter the abstraction (the static extension must be imported directly in order to enable the implicit constructors that it defines, and List<Address> xs = <int>[]; is an error. But <int>[] as List<Address> could force it, because every inline class is erased to the representation type at run time so the cast will succeed).

It could be interesting to hear if you consider this profile ("out is easy, in is a bit harder") to be a good fit.

Of course, if "out is easy" is only used during the migration period then int will be deleted from the implements clause of the declaration of Address at some point. int i = myAddress; is then a compile-time error, and so is List<int> xs = <Address>[]; (and we can still force it using an explicit cast).

@sigmundch
Copy link
Member

This is an interesting use case.

We came across something similar. Recently @srujzs, @joshualitt, and I were discussing what to do with a set of marker interface types in dart:js_interop. We ended up implementing them using the same stack we have for @staticInterop, which gives us something stronger than typedefs, but weaker than inline classes (I guess they act as open inline classes, if I understand things correctly). Moving them to inline-classes would be a breaking change, but adding openness may make that transition possible.

@srujzs
Copy link
Contributor

srujzs commented May 9, 2023

Yeah, there is some overlap here. We've talked about assignability before, but I don't think we've talked about this case with Leaf and Erik before.

For context, we've been using @staticInterop types as a prototype for inline classes, where statically, they're different types, but they get erased to the same representation type in the backends. They're still classes, so you can have things like inheritance between @staticInterop classes.

We're using this prototype to implement JS types (like JSFunction, JSArray, etc.) today. We can move them to inline classes later so users can implement them to get assignability, but this breaks any users who are using @staticInterop types to inherit the existing JS types. So, if we want to keep them as @staticInterop types and still get assignability for users who want to use inline classes, we'd need "open" inline classes or be able to implement a non-inline class (the @staticInterop JS types).

IIUC, the difference is that we're trying to change the implementation, but keep the name the same (e.g. JSString with @staticInterop -> JSString with inline classes), and FFI is trying to change the name as well (e.g. int -> Address).

@eernstg Naturally, the implicit proposal there for the other direction might resolve our desire for implicit conversions as well. Briefly thinking about it, I don't think the limitations mentioned (generics and importing extensions) are an issue.

Cool to see we have some similar goals as dart:ffi!

@eernstg
Copy link
Member

eernstg commented May 10, 2023

@srujzs wrote:

I don't think the limitations mentioned (generics and importing extensions) are an issue.

I think I should mention that those properties are design parameters rather than unavoidable consequences of the overall approach. So we could just remove or change those restrictions, if desired.

For example, we could allow an implicit factory constructor returning instances of C to be declared in the class C, and we could allow implicit construction to take place in any location where the target type C occurs as a context type.

Similarly, we could specify that an inline class I with representation type T is both a subtype and a supertype of T, perhaps based on a modifier like open inline class I; this would enable List<Address> xs = <int>[];.

But the current proposals (the accepted one for the core of inline classes plus some not-yet-landed proposals, including the one about implicit construction) don't do that. Here's a bit of rationale for the two properties you mention:

As proposed, implicit constructors can only be declared in a static extension on the target class (not in the target class itself), and the static extension must be directly imported in order to enable implicit construction using the implicit factory constructors that this static extension declares.

  • The first rationale is that implicit construction makes it more difficult to read and reason about source code, because any expression whatsoever (in particular, subexpressions of other expressions) could be subject to this kind of conversion, and it could change the type and semantics of that expression, and there is no visual clue that this is happening. So the proposal is that implicit construction must be enabled by a direct import of the enclosing static extension. We might even require that there is an explicit show MyStaticExtension on the import in order to enable implicit construction using declarations in that static extension (Scala does something along those lines). The point is that the experience from other languages show that it is necessary to be careful in this area.
  • The next rationale is that it should be possible to specify implicit construction that converts an A to a B without having edit access to the declaration of A or B. A kind of extension mechanism is a good fit for this, so we're introducing static extension declarations, which are being requested anyway. They can add static members and factory constructors to existing classes, similarly to the way regular extension declarations can add instance members.
  • Finally, we're in a good position to relax and generalize this approach if it turns out to be useful to do so. It would be more difficult if we start up using a very general and permissive model, and then it turns out to be a huge footgun that implicit construction occurs in a lot of situations where somebody just created a type mismatch by mistake.

Wrt the subtype relationships, an inline type is unrelated to its representation type by default. So with an inline class Address and representation type int, all of Address a = 1;, int i = myAddress;, List<int> xs = <Address>[];, and List<Address> xs = <int>[]; are errors. We are considering to add a small feature (used in my earlier comments), which is that the inline class can declare a subtype relationship to the representation type (or any supertype thereof). In the running example we'd use implements int in the declaration of Address. The rationale is it may be convenient for some inline classes to allow exiting the abstraction smoothly, but it should be a choice rather than the default.

In general, these rules and defaults tend to make inline classes a bit less convenient, but, arguably, a bit safer, yielding code which is a bit easier to read and understand and maintain.

@srujzs
Copy link
Contributor

srujzs commented May 10, 2023

Couple of comments:

  • Reading through, it isn't clear if the implicit constructor can be declared only in extensions of inline class, or any classes. If the former, then this resolves the case where Dart types can be assigned to JS types, but not the other way around. Subtyping using implements doesn't solve the other way around, since there is no conversion happening when subtyping.
  • I think for our use case, we actually don't want implicit constructors in a general class. For example, if we want implicit conversions between JSString (which might end up being implemented as an inline class) and String, we can't actually declare an implicit constructor on the latter unless we can use an extension. You touch on this in your second bullet point.
  • Re: static extension declarations, great! I think we were interested in this, but don't remember the specific case where we wanted it.

All in all, I think having to explicitly import the extensions is a small price to pay.

@eernstg
Copy link
Member

eernstg commented May 11, 2023

it isn't clear if the implicit constructor can be declared only in extensions of inline class

Any kind of class. In fact, it can be allowed on any declaration that introduces an interface type. For instance, we could use this feature to add a factory constructor to a mixin: As long as the constructor returns an object whose type implements that mixin, we're good. Being implicit is orthogonal—when we can have a factory constructor in some static extension, it can also be made implicit.

if we want implicit conversions between JSString (which might end up being implemented as an inline class) and String

... then you might do this:

static extension E1 on JSString {
  implicit JSString.fromString(String s) => ...;
}

static extension E2 on String {
  implicit String.fromJSString(JSString jsString) => ...;
}

static extension declarations, great!

Cf. dart-lang/language#723, with 597 thumbs up and 3 down. 😄

@a-siva a-siva added type-enhancement A request for a change that isn't a bug P3 A lower priority bug or feature request triaged Issue has been triaged by sub team labels Dec 7, 2023
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 type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants