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.

Sign up for GitHub

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

Jump to bottom

[ffigen] Create AST and Visitor classes #1638

Merged
merged 16 commits into from
Oct 21, 2024
Merged

[ffigen] Create AST and Visitor classes #1638

merged 16 commits into from
Oct 21, 2024

Conversation

liamappelbe
Copy link
Contributor

@liamappelbe liamappelbe commented Oct 9, 2024 •
edited
Loading

Don't submit this until ffigen 15.0.0 is published.

Creates these classes:

  • AstNode: Abstract class that all bindings/types etc inherit from
  • Visitation: Abstract class that all visitors implement.
  • Visitor: Wraps a Visitation in DFS logic.

The reason for the Visitor/Visitation distinction is to prevent Visitation implementers from overriding the Visitor's DFS logic, and prevent Visitor callers from calling internal methods in the Visitation. I wrote pretty comprehensive docs on those classes, so read them for more info.

Details:

  • Renamed Member to CompoundMember, since it's only used in the Compound class.
  • Make Binding and Type extend AstNode. This covers most of the existing AST.
  • Also make a bunch of classes that previously weren't formally part of the AST extend AstNode: CompoundMember, Parameter, LibraryImport, ObjCListenerBlockTrampoline, ObjCMsgSendFunc, ObjCProperty, and ObjCMethod.
  • Give every AstNode with children a visitChildren method.
  • Give some AstNodes a visit method. We only need to do this for node Foo if there's a Visitation that wants to visit Foo specifically, rather than one of its supertypes. I'm lazy loading these methods to avoid excessive boilerplate, because there's a lot of AstNodes that no one is ever going to need to visit.
  • Delete the ubiquitous addDependencies method.
    • Most of what this method does has been migrated to ListBindingsVisitation.
    • I was also using it to do some ad-hoc transformations. Some of those have been moved to the constructors of various objects, and the rest have been migrated to CopyMethodsFromSuperTypesVisitation, FillMethodDependenciesVisitation, and FixOverriddenMethodsVisitation
  • The private _isBuiltIn flag that most ObjC nodes have has been renamed isObjCImport and moved up the stack to NoLookupBinding, so that it can be used by ListBindingsVisitation.
  • The iteration order of some of the bindings changed, so a bunch of expectations files had to be updated.

#1259

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 9, 2024 •
edited
Loading

PR Health

Coverage ⚠️
File Coverage
pkgs/ffigen/example/libclang-example/generated_bindings.dart 💔 Not covered
pkgs/ffigen/example/objective_c/avf_audio_bindings.dart 💔 Not covered
pkgs/ffigen/example/swift/swift_api_bindings.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/binding.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/compound.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/constant.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/enum_class.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/func.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/func_type.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/global.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/imports.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_built_in_functions.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/objc_interface.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/objc_methods.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/objc_nullable.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/objc_protocol.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/pointer.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/type.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/typealias.dart 💔 Not covered
pkgs/ffigen/lib/src/header_parser/sub_parsers/compounddecl_parser.dart 💔 Not covered
pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart 💔 Not covered
pkgs/ffigen/lib/src/visitor/ast.dart 💔 Not covered
pkgs/ffigen/lib/src/visitor/copy_methods_from_super_type.dart 💔 Not covered
pkgs/ffigen/lib/src/visitor/fill_method_dependencies.dart 💔 Not covered
pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart 💔 Not covered
pkgs/ffigen/lib/src/visitor/list_bindings.dart 💔 Not covered
pkgs/ffigen/lib/src/visitor/visitor.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 ⚠️ 
// 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/test/header_parser_tests/expected_bindings/_expected_dart_handle_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_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/large_integration_tests/_expected_libclang_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_sqlite_bindings.dart

All source files should start with a license header.

Unrelated files missing license headers
Files
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/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_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_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_varargs_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_cjson_bindings.dart
pkgs/ffigen/test/native_test/_expected_native_test_bindings.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_builder/test_data/native_dynamic_linking/bin/native_dynamic_linking.dart
pkgs/objective_c/lib/src/ns_input_stream.dart
pkgs/swift2objc/lib/src/config.dart
pkgs/swift2objc/lib/src/generate_wrapper.dart
pkgs/swift2objc/lib/src/generator/_core/utils.dart
pkgs/swift2objc/lib/src/generator/generator.dart
pkgs/swift2objc/lib/src/generator/generators/class_generator.dart
pkgs/swift2objc/lib/src/parser/parsers/declaration_parsers/parse_initializer_declaration.dart
pkgs/swift2objc/lib/src/parser/parsers/declaration_parsers/parse_variable_declaration.dart
pkgs/swift2objc/lib/src/transformer/_core/unique_namer.dart
pkgs/swift2objc/lib/src/transformer/_core/utils.dart
pkgs/swift2objc/lib/src/transformer/transformers/transform_globals.dart
pkgs/swift2objc/lib/src/transformer/transformers/transform_variable.dart
pkgs/swift2objc/test/unit/parse_initializer_param_test.dart

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

@coveralls
Copy link

Coverage Status

coverage: 91.126% (+0.8%) from 90.305%
when pulling 59b0b67 on ffigenast
into c638d9f on main.

@liamappelbe liamappelbe changed the title WIP: [ffigen] Create AST and Transformer classes WIP: [ffigen] Create AST and Visitor classes Oct 16, 2024
@liamappelbe liamappelbe changed the title WIP: [ffigen] Create AST and Visitor classes [ffigen] Create AST and Visitor classes Oct 17, 2024
@liamappelbe liamappelbe marked this pull request as ready for review October 17, 2024 00:45
HosseinYousefi
HosseinYousefi approved these changes Oct 17, 2024
View reviewed changes
Copy link
Member

@HosseinYousefi HosseinYousefi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks much nicer! Thanks @liamappelbe.

@liamappelbe liamappelbe merged commit 9d45f18 into main Oct 21, 2024
17 checks passed
@liamappelbe liamappelbe deleted the ffigenast branch October 21, 2024 21:28
@liamappelbe liamappelbe mentioned this pull request Dec 19, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HosseinYousefi HosseinYousefi HosseinYousefi approved these changes

Assignees
No one assigned
Labels
package:ffigen skip-changelog-check
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@liamappelbe @coveralls @HosseinYousefi