Fixes and tests for listener block native bindings #1250

liamappelbe · 2024-07-02T02:45:50Z

#1203 added a small ObjC trampoline to some listener blocks. This PR fixes 3 bugs in that generated ObjC code that were found while working on the large ObjC integration test, and also reported by @brianquinlan.

For listener blocks with nullable arguments, the _Nullable annotation was being placed in the wrong spot (DummyObject* arg0 _Nullable instead of DummyObject* _Nullable arg0). Fixes Missgenerated objc-bindings for nullable types #1230.
If a struct is declared like struct Foo {} instead of typedef struct {} Foo, we have to refer to it (in ObjC code) as struct Foo instead of just Foo. Same for unions. This distinction wasn't surfaced at all in the existing Struct/Union class.
- The type spelling string of the declaration contains this distinction, being either "struct Foo" or "Foo", so we could check if this string starts with "struct" and set a bool flag on the Struct object. But since this string is already formatted exactly as I want, I'm just plumbing it through directly.
The generated ObjC code needs to #include the entrypoint(s), so that it has access to all the classes etc declared by the entrypoint header or its dependencies.

Now that the generated ObjC bindings include the entrypoint, my hack of putting all the ObjC code for each test in a single header and using that as the ffigen entrypoint doesn't work. I had to split a header file off of block_test.m to resolve duplicate symbol errors during linking.

github-actions · 2024-07-02T02:47:19Z

PR Health

Coverage ⚠️

Details

File	Coverage
pkgs/ffigen/lib/src/code_generator/compound.dart	💔 Not covered
pkgs/ffigen/lib/src/code_generator/library.dart	💔 Not covered
pkgs/ffigen/lib/src/code_generator/objc_block.dart	💔 Not covered
pkgs/ffigen/lib/src/code_generator/objc_nullable.dart	💔 Not covered
pkgs/ffigen/lib/src/code_generator/struct.dart	💔 Not covered
pkgs/ffigen/lib/src/code_generator/union.dart	💔 Not covered
pkgs/ffigen/lib/src/code_generator/writer.dart	💔 Not covered
pkgs/ffigen/lib/src/config_provider/spec_utils.dart	💔 Not covered
pkgs/ffigen/lib/src/header_parser/parser.dart	💔 Not covered
pkgs/ffigen/lib/src/header_parser/sub_parsers/compounddecl_parser.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
no missing headers

All source files should start with a license header.

Unrelated files missing license headers

Files
pkgs/ffi/example/main.dart
pkgs/ffigen/example/libclang-example/generated_bindings.dart
pkgs/ffigen/example/shared_bindings/generate.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/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_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_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_typedef_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_libclang_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
pkgs/swiftgen/swift2objc/lib/src/ast/ast.dart
pkgs/swiftgen/swift2objc/lib/src/ast/declarations/built_in/built_in_declaration.dart

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	already published at pub.dev
package:jnigen	0.10.0-wip	WIP (no publish necessary)
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.

coveralls · 2024-07-02T02:54:41Z

coverage: 91.792% (+0.8%) from 91.024%
when pulling 0b20cfc on nattypes
into bafe220 on main.

dcharkes · 2024-07-02T08:55:11Z

The type spelling string of the declaration contains this distinction, being either "struct Foo" or "Foo", so we could check if this string starts with "struct" and set a bool flag on the Struct object. But since this string is already formatted exactly as I want, I'm just plumbing it through directly.

We'd have similar issues in C/C++ but we don't generate any C/C++ code ever. However if we would in the future, this logic should not live in some objective-c specific place. From the code it looks like it just lives in the compound which is used for both C and Objective-C. 👍

dcharkes

LGTM! 🔥

pkgs/ffigen/lib/src/code_generator/library.dart

coveralls · 2024-07-02T23:48:04Z

coverage: 91.792% (+0.8%) from 91.024%
when pulling 339aa85 on nattypes
into bafe220 on main.

* Resolve conflicts in native block wrapper names. Also remove nullable annotation from native code. It doesn't do anything and is sometimes invalid. * Use <> style imports for framework headers in ObjC native code Also, use #import instead of #include * Test framework header parser * More tests * Revert renaming of originalName. Add Func.useNameForLookup instead * nits

Fixes and tests for listener block native bindings

0b20cfc

liamappelbe requested a review from dcharkes July 2, 2024 02:45

github-actions bot added the package:ffigen label Jul 2, 2024

liamappelbe added the skip-changelog-check label Jul 2, 2024

dcharkes approved these changes Jul 2, 2024

View reviewed changes

pkgs/ffigen/lib/src/code_generator/library.dart Outdated Show resolved Hide resolved

Rename entryPoints field to nativeEntryPoints.

339aa85

liamappelbe merged commit 249d665 into main Jul 2, 2024
17 checks passed

liamappelbe deleted the nattypes branch July 2, 2024 23:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes and tests for listener block native bindings #1250

Fixes and tests for listener block native bindings #1250

liamappelbe commented Jul 2, 2024

github-actions bot commented Jul 2, 2024 •

edited

Loading

coveralls commented Jul 2, 2024

dcharkes commented Jul 2, 2024

dcharkes left a comment

coveralls commented Jul 2, 2024

Fixes and tests for listener block native bindings #1250

Fixes and tests for listener block native bindings #1250

Conversation

liamappelbe commented Jul 2, 2024

github-actions bot commented Jul 2, 2024 • edited Loading

PR Health

Coverage ⚠️

License Headers ✔️

Package publish validation ✔️

coveralls commented Jul 2, 2024

dcharkes commented Jul 2, 2024

dcharkes left a comment

Choose a reason for hiding this comment

coveralls commented Jul 2, 2024

github-actions bot commented Jul 2, 2024 •

edited

Loading