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

Make TypedefElaborated respect @add_def #321

Merged
merged 4 commits into from
Sep 14, 2021
Merged

Conversation

melonedo
Copy link
Contributor

@melonedo melonedo commented Aug 9, 2021

Fix #320

@Gnimuc
Copy link
Member

Gnimuc commented Aug 9, 2021

This line can be removed as well.

@Gnimuc
Copy link
Member

Gnimuc commented Aug 9, 2021

The @add_def macro was originally designed for adding identifiers in the system headers. After adding multi-platform support, this macro is mainly for skipping something wrong or unexpected. As a result, the current implementation is a mess and needs to be refactored in the future.

@melonedo
Copy link
Contributor Author

This line can be removed as well.

Is this because currently tags are not translated?

@Gnimuc
Copy link
Member

Gnimuc commented Aug 10, 2021

As mentioned above, initially @add_def was for adding identifiers in the system headers, at that time, all the system-header definitions need to be manually wrapped.

Then, when implementing the multi-platform support, @add_def was for extracting specific identifiers in the system headers, for example, @add_def tm can be used to tell the generator to search tm in the system headers and generate a wrapper for tm. This was still very cumbersome because users need to add those @add_defs manually.

Then, the experimental CollectDependentSystemNode pass was added, so all dependent system nodes can be automatically handled. So, @add_def is now only for doing workaround.

This PR adds || haskey(dag.ids_extra, leaf_ty.sym)) for TypedefElaborated to make the behavior consistent with TypedefDefault, which is mainly for fixing the old behavior for extracting typedef identifiers in the system headers.
However, this line was for rewriting the definition of tm to a custom one, which turns out to be a misuse now.

The @add_def macro has two different meanings for typedef-ed identifiers and tag types. It needs to be separated into two macros, one for extracting identifiers @extract_def and one for skipping tag types @skip_def. But it's not trivial to do the refactor.

@visr visr mentioned this pull request Aug 10, 2021
melonedo added a commit to melonedo/Clang.jl that referenced this pull request Sep 13, 2021
@melonedo
Copy link
Contributor Author

I added a test case for this.

@melonedo melonedo closed this Sep 13, 2021
@melonedo melonedo reopened this Sep 13, 2021
@melonedo
Copy link
Contributor Author

The tests failures are with Julia 1.7. New CI scripts is not enabled.

@Gnimuc
Copy link
Member

Gnimuc commented Sep 13, 2021

need to rebase this PR onto master.

melonedo added a commit to melonedo/Clang.jl that referenced this pull request Sep 13, 2021
@melonedo
Copy link
Contributor Author

Oops this keeps having conflicts as new tests are added.

@Gnimuc Gnimuc merged commit 85106cd into JuliaInterop:master Sep 14, 2021
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.

Unable to wrap stat
2 participants