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

Remove setup phase #135

Merged
merged 14 commits into from
Jan 8, 2021
Merged

Conversation

mannprerak2
Copy link
Contributor

@mannprerak2 mannprerak2 commented Jan 8, 2021

Closes dart-lang/native#502

  • Remove the usage of _wrap functions from code.
  • Remove wrapper.c and setup phase for ffigen.

Follow up for #134.

@mannprerak2 mannprerak2 requested a review from dcharkes January 8, 2021 11:44
@mannprerak2
Copy link
Contributor Author

@dcharkes This helped remove 1 bug from #134, I think we can merge that now.

For task2 (removing setup phase) we need to have the user input the location of libclang's dynamic library. I am thinking we can have a few default locations to check, but if we can't find it there, we make the user enter the path/to/llvm/lib so that we can find the dynamic library there.

@mannprerak2
Copy link
Contributor Author

This is complete now, I've added a new config key - llvm-lib which is required if we can't find it at the default locations.

@dcharkes
Copy link
Contributor

dcharkes commented Jan 8, 2021

@mannprerak2 this is marked as draft, do you want me to start reviewing?

And do you want to merge this in before releasing 2.0.0-dev.1 (edit: I see this is dev.2, so I published the previous one).

@mannprerak2 mannprerak2 marked this pull request as ready for review January 8, 2021 17:17
@mannprerak2
Copy link
Contributor Author

do you want me to start reviewing?

Yes, I have added a merge commit with dart-dev branch, so that should be easier now.

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.

Cool! Really cool to see that wrapper gone! 🥳

This shows structs by value is a nice dart:ffi feature to have! 😃

@dcharkes dcharkes merged commit 028a02d into dart-archive:dart-dev Jan 8, 2021
@mannprerak2 mannprerak2 deleted the remove-wrapper branch January 8, 2021 18:03
mannprerak2 added a commit to mannprerak2/ffigen that referenced this pull request Mar 1, 2021
dcharkes pushed a commit that referenced this pull request Mar 1, 2021
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.

2 participants