Skip to content
This repository has been archived by the owner on Jan 28, 2024. It is now read-only.

Improvements to generating code, support for typedef. #224

Merged
merged 14 commits into from
Jun 3, 2021

Conversation

mannprerak2
Copy link
Contributor

@mannprerak2 mannprerak2 commented May 25, 2021

Closes dart-lang/native#359, Closes dart-lang/native#363

  • All declarations that are not included by the user are now only included if being used somewhere.
    Context. - earlier, declarations that were used by another declaration were included even if the declaration that included it was removed due to it not being supported by dart:ffi
  • Improved struct/union include/exclude
    Contest - earlier, it was a bit complicated to target structs, since they could have multiple names (from typedefs). With typedef support users can target structs by their true name (or incase they are anonymous, by the first typedef that refers to them.
  • Use inline types for function types
    Context - earlier, we generated typedefs like _c_func and _dart_func for specifying function type. This has now been replaced with an inline function type declaration. Note that if the user exposes a function pointer using function->symbol-address we still create a typedef for that, but not otherwise. Since these types are only referred to once, using inline types reduces the number of lines generated.
  • Support for typedefs
    Context - Typedefs in C are now directly mapped as it is when possible. Only typedefs that were referred somewhere are generated. Users can rename these and exclude generating them (referred typedefs only). If a typedef is excluded we simply use the underlying type.
  • Added /usr/lib64 to the default dynamic library search paths on Linux.
  • Updated/Added tests, Updated changelog, version.
  • Fixed cJSON example.

@mannprerak2 mannprerak2 marked this pull request as ready for review May 26, 2021 10:24
@mannprerak2 mannprerak2 requested a review from dcharkes May 26, 2021 10:24
@mannprerak2
Copy link
Contributor Author

Hey @dcharkes, this is probably the biggest PR since #2, please take your time to review this. I've tried to clarify the reasons for the changes in the above #224 (comment).

Copy link
Contributor

@dcharkes dcharkes left a comment

Choose a reason for hiding this comment

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

First round of comments. (This is mostly the public API/docs. I'll go through the implementation code later.)

CHANGELOG.md Outdated Show resolved Hide resolved
example/c_json/pubspec.yaml Show resolved Hide resolved
lib/src/code_generator/typealias.dart Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
Copy link
Contributor

@dcharkes dcharkes left a comment

Choose a reason for hiding this comment

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

Please also add tests that generate intptr_t -> IntPtr() now that we can recognize the typedefs. In the past we just saw the int32 or int64 of the underlying size correct? We should be able to now support this properly.

CHANGELOG.md Outdated Show resolved Hide resolved
lib/src/header_parser/includer.dart Outdated Show resolved Hide resolved
@mannprerak2 mannprerak2 requested a review from dcharkes June 3, 2021 10:07
@dcharkes
Copy link
Contributor

dcharkes commented Jun 3, 2021

Please also add tests that generate intptr_t -> IntPtr() now that we can recognize the typedefs. In the past we just saw the int32 or int64 of the underlying size correct? We should be able to now support this properly.

Did you see this @mannprerak2 ?

@mannprerak2
Copy link
Contributor Author

Please also add tests that generate intptr_t -> IntPtr() now that we can recognize the typedefs. In the past we just saw the int32 or int64 of the underlying size correct? We should be able to now support this properly.
Did you see this @mannprerak2 ?

Ah, my bad, didn't see this before.

We already have support for IntPtr. Using the use-supported-typedefs config (true by default) We map intptr_t to IntPtr. Test - Map intptr_t to IntPtr, Output => C code & Dart Code.

@dcharkes
Copy link
Contributor

dcharkes commented Jun 3, 2021

I must have misremembered that then. 😄

@mannprerak2
Copy link
Contributor Author

mannprerak2 commented Jun 3, 2021

I must have misremembered that then.

Well, you weren't than far. We are still using platform dependant types (Uint32 or Uint64) for types such as size_t. But the user can map them to whatever they like using typedef-map in config.

Do we have a cross platform solution for this in the working for dart:ffi?

@dcharkes
Copy link
Contributor

dcharkes commented Jun 3, 2021

Do we have a cross platform solution for this in the working for dart:ffi?

It's on the list, but I've been busy with other stuff.

@dcharkes dcharkes merged commit 98b2d4f into dart-archive:master Jun 3, 2021
@mannprerak2 mannprerak2 deleted the typedef_major branch June 3, 2021 11:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Linux: llvm-path: add lib64 and/or adjust search Typedef support
2 participants