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

Fix unhandled non-field struct children #479

Merged
merged 1 commit into from
Apr 6, 2024

Conversation

serenity4
Copy link
Contributor

@serenity4 serenity4 commented Apr 5, 2024

Unhandled non-field child nodes were sometimes present in children(cursor), and were already handled in printing but not in codegen, which triggered an assertion error at

@assert length(child_nodes) == length(fields)

where fields built from the codegened expression had an extra field for a forward struct declaration.

Here is an exemple header where the error appeared:

typedef struct _ExampleStruct {
  struct _ForwardA *a;
} ExampleStruct;

typedef struct _ForwardA {
  int a;
} ForwardA;

and codegen produced something like

struct _ExampleStruct
  _ForwardA::_ForwardA
  a::Ptr{_ForwardA}
end

Now we don't have that weird spurious _ForwardA::_ForwardA field.

It happened to me while wrapping a larger header (winuser.h) using this pattern, but I couldn't come up with a minimal reproducer. For some reason, fields(Clang.getCursorType(cursor)) for _ExampleStruct was empty in that larger header and Clang.jl fell back to using children to extract fields, while in any example I have tried for a minimal reproducer fields(Clang.getCursorType(cusor)) succeeded in returning the only field present as expected.

@Gnimuc
Copy link
Member

Gnimuc commented Apr 6, 2024

_emit_getproperty_ptr! may also need to be patched.

@Gnimuc Gnimuc merged commit d92f9c9 into JuliaInterop:master Apr 6, 2024
11 checks passed
@Gnimuc
Copy link
Member

Gnimuc commented Apr 6, 2024

Could be related to the upstream changes discussed in #472

@Gnimuc
Copy link
Member

Gnimuc commented Apr 6, 2024

let's wait for more bug reports.

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