[Breaking Change Request] [vm/ffi] Disallow empty structs

[dcharkes]

Update 2021-01-22: We will deprecate empty structs as of Dart 2.12 and remove support for them in 2.13.

Summary

Disallow defining subclasses of Struct without fields.

sdk/tests/ffi/vmspecific_static_checks_test.dart

Line 486 in 897b1a2

class EmptyStruct extends Struct {}

Introduce the Opaque native type to represent opaque C types.

/// [Opaque]'s subtypes represent opaque types in C.
///
/// [Opaque]'s subtypes are not constructible in the Dart code and serve purely
/// as markers in type signatures.
abstract class Opaque extends NativeType {}

Motivation

1. Simpler error messages. Currently we have error messages on using empty structs in nested structs and trampolines. Having these error messages on the empty structs themselves would be easier.
2. Prevent people shooting themselves in the foot by calling allocate<Utf8>() and allocating 0 bytes of native memory ([vm/ffi] Deprecate empty structs #43974, Making Utf8/Utf16 a subclass of Struct is misleading dart-archive/ffi#34).

Impact

Definitions need to be migrated from extends Struct to extends Opaque.

Use sites can no longer use .ref to pass around 'references' with types such as Utf8. Instead, Pointer<Utf8> needs to be passed around.

Migration of package:ffi

Proposed changes:

• Utf8
• Utf16

Migration of users of package:ffi

Example migrations:

• dart-archive/ffi@afb6b2c#diff-d0d91a39cf94c8fa778373c4bcafa5a41b94ed40f7eef1428889867a76739f7e
• https://dart-review.googlesource.com/c/sdk/+/177710/14/samples/ffi/sqlite/lib/src/database.dart

Discussion

We could change the NativeType hierarchy to the following and reduce the amount of custom checks in the analyzer and common frontend (for sizeOf, Pointer.elementAt and Allocator.call).

• NativeType
  • Opaque
    • Void
    • NativeFunction
    • (user defined types)
  • Sized (only accept these in sizeOf, Pointer.elementAt (would become an extension method) and Allocator.call)
    • NativeInteger
      • ...
    • NativeDouble
    • Struct
      • (user defined types)

[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.

[vsmenon]

lgtm

[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

[simolus3]

Is there a rough ETA for these changes? Considering that the apis haven't landed yet and that Dart 2.12 was announced for "early 2021", I'm a bit worried about how little time we might have to migrate.

Is there any way to get the Opaque interface into the next beta?

[franklinyow]

Approved

[dcharkes]

Is there a rough ETA for these changes?

I've landed the new API in master.

Deprecating the old API will take some time, because this has to roll into Flutter first.

[dcharkes]

Is there a rough ETA for these changes? Considering that the apis haven't landed yet and that Dart 2.12 was announced for "early 2021", I'm a bit worried about how little time we might have to migrate.

Is there any way to get the Opaque interface into the next beta?

@simolus3 We will deprecate support for empty structs and add Opaque in Dart 2.12, and remove support for empty structs in 2.13.

[simolus3]

That's relieving to hear, thank you!

[dcharkes]

We have landed deprecation hints for this in c3b30df.

Turning these hints into errors is tracked in #43974. Closing this issue.