Ffi enums

[Levi-Lesches]

A continuation of #1198, which generated real Dart enums instead of abstract classes. This PR now modifies the return type or argument type of any function that uses an enum to use the actual enum type instead of the value.

Fixes #1201

Given the following C:

typedef enum {
  ok = 1,
  error = 2,
} Status;

Status getStatus() { return ok; }

int getValue(Status status) {
  if (status == ok) return 1;
  else return 2;
}

Now we can simply write:

void main() {
  final lib = NativeLibrary(DynamicLibrary.open("${Directory.current.path}/lib.so"));
  Status status = lib.getStatus();
  int value = lib.getValue(status);
  String message = getMessage(status);
}

String getMessage(Status s) => switch (s) {
  // Comes with full exhaustiveness checking
  Status.ok => "Okay! :)",
  Status.error => "Oops! :(",
};

---

• I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Contribution guidelines:

• See our contributor guide for general expectations for PRs.
• Larger or significant changes should be discussed in an issue before creating a PR.
• Contributions to our repos should follow the Dart style guide and use dart format.
• Most changes should add an entry to the changelog and may need to rev the pubspec package version.
• Changes to packages require corresponding tests.

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

[github-actions]

PR Health

Changelog Entry ✔️

Details

Package	Changed Files

Changes to files need to be accounted for in their respective changelogs.

Coverage ⚠️

Details

File	Coverage
pkgs/ffigen/example/libclang-example/generated_bindings.dart	💔 Not covered
pkgs/ffigen/example/shared_bindings/lib/generated/a_gen.dart	💔 Not covered
pkgs/ffigen/example/shared_bindings/lib/generated/a_shared_b_gen.dart	💔 Not covered
pkgs/ffigen/lib/src/code_generator/enum_class.dart	💔 Not covered
pkgs/ffigen/lib/src/code_generator/objc_interface.dart	💔 Not covered

This check for test coverage is informational (issues shown here will not fail the PR).

This check can be disabled by tagging the PR with skip-coverage-check.

License Headers ⚠️

Details

// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

Files
pkgs/ffigen/example/libclang-example/generated_bindings.dart
pkgs/ffigen/example/shared_bindings/lib/generated/a_gen.dart
pkgs/ffigen/example/shared_bindings/lib/generated/a_shared_b_gen.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_enum_int_mimic_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_forward_decl_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_typedef_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_libclang_bindings.dart

All source files should start with a license header.

Unrelated files missing license headers

Files
pkgs/ffi/example/main.dart
pkgs/ffigen/example/shared_bindings/generate.dart
pkgs/ffigen/example/shared_bindings/lib/generated/base_gen.dart
pkgs/ffigen/example/simple/generated_bindings.dart
pkgs/ffigen/lib/src/config_provider/config_spec.dart
pkgs/ffigen/lib/src/header_parser/clang_bindings/clang_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_decl_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_symbol_address_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_type_name_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_reserved_keyword_collision_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_comment_markup_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_dart_handle_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_functions_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_imported_types_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_native_func_typedef_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_opaque_dependencies_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_packed_structs_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_regress_384_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_struct_fptr_fields_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_unions_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_varargs_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_cjson_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_sqlite_bindings.dart
pkgs/ffigen/test/native_test/_expected_native_test_bindings.dart
pkgs/jni/lib/src/lang/jcharacter.dart
pkgs/jni/lib/src/third_party/generated_bindings.dart
pkgs/jni/lib/src/third_party/global_env_extensions.dart
pkgs/jni/lib/src/third_party/jni_bindings_generated.dart
pkgs/jnigen/android_test_runner/lib/main.dart
pkgs/jnigen/example/in_app_java/lib/android_utils.dart
pkgs/jnigen/example/kotlin_plugin/example/lib/main.dart
pkgs/jnigen/example/kotlin_plugin/lib/kotlin_bindings.dart
pkgs/jnigen/example/kotlin_plugin/lib/kotlin_plugin.dart
pkgs/jnigen/example/pdfbox_plugin/lib/pdfbox_plugin.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/PDDocument.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/PDDocumentInformation.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/_package.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/text/PDFTextStripper.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/text/_package.dart
pkgs/jnigen/lib/src/bindings/descriptor.dart
pkgs/jnigen/lib/src/elements/elements.g.dart
pkgs/jnigen/test/jackson_core_test/third_party/bindings/com/fasterxml/jackson/core/_package.dart
pkgs/jnigen/tool/command_runner.dart
pkgs/native_assets_cli/lib/src/api/builder.dart
pkgs/native_assets_cli/lib/src/api/linker.dart
pkgs/native_assets_cli/test/model/checksum_test.dart

This check can be disabled by tagging the PR with skip-license-check.

Package publish validation ✔️

Details

Package	Version	Status
package:ffi	2.1.2	already published at pub.dev
package:ffigen	13.0.0-wip	WIP (no publish necessary)
package:jni	0.9.3-wip	WIP (no publish necessary)
package:jnigen	0.9.2	already published at pub.dev
package:native_assets_cli	0.6.1-wip	WIP (no publish necessary)
package:objective_c	1.1.0-wip	WIP (no publish necessary)
package:swiftgen	0.0.1-wip	WIP (no publish necessary)

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

[Levi-Lesches]

So close! Two tests are failing on Mac for Objective-C:

❌ loading test/native_objc_test/block_test.dart (failed)
  Failed to load "test/native_objc_test/block_test.dart":
  test/native_objc_test/block_bindings.dart:416:28: Error: A value of type 'int' can't be returned from a function with return type 'NSQualityOfService'.
   - 'NSQualityOfService' is from 'test/native_objc_test/block_bindings.dart'.
      return _objc_msgSend_15(this.pointer, _sel_qualityOfService);

❌ loading test/native_objc_test/swift_class_test.dart (failed)
  Failed to load "test/native_objc_test/swift_class_test.dart":
  test/native_objc_test/swift_class_bindings.dart:1944:28: Error: A value of type 'int' can't be returned from a function with return type 'NSAttributedStringMarkdownInterpretedSyntax'.
   - 'NSAttributedStringMarkdownInterpretedSyntax' is from 'test/native_objc_test/swift_class_bindings.dart'.
      return _objc_msgSend_42(this.pointer, _sel_interpretedSyntax);

    - (more similar errors)

At least it seems the return type of the bindings is being changed to the enum type NSQualityOfService rather than a plain integer, I'm just not sure why it didn't generate a .fromValue() wrapper for these two tests in particular. I also don't have a Mac device to debug with.

@dcharkes What do you recommend?

[Levi-Lesches]

Knowing nothing about Objective-C, I found these lines that matched NSQualityOfService:

https://github.com/dart-lang/native/blob/ecb8b7eca32143348ce645d57eab3890e10b046f/pkgs/ffigen/example/swift/swift_api_bindings.dart#L20225-L20251

I think I may have narrowed down the problem to these lines, which seem to generate those bindings:

native/pkgs/ffigen/lib/src/code_generator/objc_built_in_functions.dart

Lines 162 to 173 in ecb8b7e

	@override
	BindingString toBindingString(Writer w) {
	final cType = NativeFunc(type).getCType(w);
	final dartType = type.getFfiDartType(w, writeArgumentNames: false);
	final pointer = variant.pointer.gen(w);
	final bindingString = '''
	final $name = $pointer.cast<$cType>().asFunction<$dartType>();
	''';
	return BindingString(type: BindingStringType.func, string: bindingString);
	}

Not entirely sure where the fix is from here, and hopefully someone else knows a bit more than I do. @liamappelbe, I saw you had committed to that file a few times. Any ideas?

[liamappelbe]

You seem to have implemented convertFfiDartTypeToDartType correctly, so the bug must be that some binding is not calling the converter when it should.

I think I may have narrowed down the problem to these lines, which seem to generate those bindings:

That function creates the binding for the FFI function that dispatches a method call to an ObjC class. I think that's one layer too deep. The missing conversion is probably in the next layer up, where that dispatch function is used in the class method. Those methods are generated by code_generator/objc_interface.dart.

I wonder if ObjCInterface._needsConverting is just out of date? I think I wrote that helper before I added sameDartAndFfiDartType/convertFfiDartTypeToDartType etc. ObjCInterface._needsConverting probably should just return type.sameDartAndFfiDartType.

[dcharkes]

LGTM once the Objective-C is working.

Thanks @Levi-Lesches! 🎸

[dcharkes]

I also don't have a Mac device to debug with.

@liamappelbe Would you mind taking a look?

[coveralls]

coverage: 92.123% (+0.07%) from 92.049%
when pulling e78f5cf on LeviLesches-KDAB:ffi-enums
into 125af4a on dart-lang:main.

[Levi-Lesches]

@dcharkes Using this comment and GitHub Actions to test, I managed to fix it. Also merged in the enum-integer type feature and fixed a test because of it, but it should be good to merge now!

[dcharkes]

This suspiciously look like flags that have to be combined.

Hopefully no one uses enums flags in their C APIs, because then we cannot pass 1 & 2 from Dart anymore, as 3 is not a valid enum value. 😄

[Levi-Lesches]

Oh, that's an interesting point. Is that an issue? It would only be an issue if the function itself accepts an enum, because if it accepted any int we can use CXSymbolRole_Call.value. But even in C, 3 wouldn't be a valid enum value anyway, so I would assume this isn't an actual issue.

[dcharkes]

Yeah that's what I assume too. But C probably allows you to bitwise combine enums as ints and pass in an int into an enum. 🙈

If someone runs into this, we'll find out soon enough I assume. 🙃