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

Skip methods that have incomplete types #412

Merged
merged 4 commits into from
Jul 6, 2022
Merged

Skip methods that have incomplete types #412

merged 4 commits into from
Jul 6, 2022

Conversation

liamappelbe
Copy link
Contributor

@liamappelbe liamappelbe commented Jun 29, 2022

Fixes dart-lang/native#247

Also change how method return types are parsed to fix dart-lang/native#246, since I couldn't write the test otherwise. This also fixes dart-lang/native#260, fixes dart-lang/native#281, fixes dart-lang/native#267, and fixes dart-lang/native#312

Also change how method return types are parsed
@liamappelbe liamappelbe requested a review from dcharkes June 29, 2022 22:43
@liamappelbe liamappelbe marked this pull request as draft June 29, 2022 22:53
@liamappelbe liamappelbe removed the request for review from dcharkes June 29, 2022 22:54
@liamappelbe
Copy link
Contributor Author

liamappelbe commented Jun 29, 2022

Seems to be making some method return types too generic. I'll have to investigate a bit more.

@liamappelbe
Copy link
Contributor Author

Looks like the tests are failing to find Foundation/NSObject.h, which is weird. I don't think I've changed anything relevant to that, and I can't repro this locally. So I'm re-running CI at head to see if something has changed there: https://github.com/dart-lang/ffigen/actions/runs/2562399073

@dcharkes
Copy link
Contributor

Foundation/NSObject.h

That was the same issue as I had earlier when I tried to re-order finding libclang.dylib on MacOS. Maybe something on the MacOS machines in the Github workflow changed?

Though, I could repro it locally (when trying to use libclang from XCode instead of one of the others).

@liamappelbe
Copy link
Contributor Author

liamappelbe commented Jun 30, 2022

Yep, the mac bot is failing at head: https://github.com/dart-lang/ffigen/actions/runs/2562399073

Filed dart-lang/native#249

@liamappelbe liamappelbe marked this pull request as ready for review June 30, 2022 20:40
@liamappelbe liamappelbe requested a review from dcharkes July 1, 2022 16:55
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.

Changes are LGTM.

I'm missing some test cases for the bugs that are closed. Should we add those cases to a test to make sure we don't regress them in the future?

@liamappelbe
Copy link
Contributor Author

I'm missing some test cases for the bugs that are closed. Should we add those cases to a test to make sure we don't regress them in the future?

They're covered by bad_method_test. They're also covered by the objc example I'm writing. Once I get that working reliably, I think it would make sense to turn that into a test. It'd be good to have some integration tests that are using real mac APIs.

@liamappelbe liamappelbe merged commit 09714ae into master Jul 6, 2022
@liamappelbe liamappelbe deleted the method_skip branch July 6, 2022 16:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
2 participants