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.

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

(Possible) workaround for structs/unions that have unexposed FirstAttr fields #250

Merged
merged 1 commit into from
Feb 25, 2020

Conversation

wsphillips
Copy link
Contributor

Attempt at fix for issue #249

@Gnimuc
Copy link
Member

Gnimuc commented Feb 1, 2020

This makes me feel again that libclang is not an appropriate tool for building a wrapper generator. It looks like the major audience of libclang is IDE-tools-writer, that's why it doesn't expose enough APIs we needed and hence some subroutines of the wrap!() function look too hairy.

I think one day we should rewrite the package to be a true wrapper over Clang's C++ API instead of libclang's C API. Cxx.jl is a very promising tool for this job, but it lacks good maintenance for now. I would expect this to be done in the Julia-v1.4~v1.5 period. But before that, I guess it's good to go with libclang + hairy code.

@Gnimuc Gnimuc marked this pull request as ready for review February 1, 2020 15:50
@Gnimuc
Copy link
Member

Gnimuc commented Feb 1, 2020

Better to add some tests in this PR. :)

@wsphillips
Copy link
Contributor Author

This makes me feel again that libclang is not an appropriate tool for building a wrapper generator. It looks like the major audience of libclang is IDE-tools-writer, that's why it doesn't expose enough APIs we needed and hence some subroutines of the wrap!() function look too hairy.

Yeah, I've just adopted existing wrap!() routines as a starting point/template in most cases. To be fair, it's partly to do with sometimes poorly written C APIs!

I will be sure to come back and add tests in a couple days once I have this current mess out of the way.

@Gnimuc
Copy link
Member

Gnimuc commented Feb 25, 2020

I'm going to merge this, tests can be added in another PR in the future. Thanks for contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants