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 issue with anon union in generated code #69

Closed
wants to merge 2 commits into from
Closed

Fix issue with anon union in generated code #69

wants to merge 2 commits into from

Conversation

watzon
Copy link

@watzon watzon commented Jun 9, 2020

Ok I still don't completely understand the root cause of this issue, but I documented my discovery in olbat/libgen#59. Basically in trying to generate bindings for ruby.h I was running into an issue where I was getting the error Bug: missing struct name. I eventually tracked the error to this line and this line in ruby/intern.h where there is an anonymous union inside some generated code. In trying to generate a Crystal type for the anonymous union it threw an error because there was no name to assign the type to.

So I did something simple that could probably be improved upon, and added a counter, removed the raise call that was causing things to break, and if all the other conditions in TypeMapper#check_anonymous_name fail, it returns a generated name anon_#{@anon_count += 1}. So now instead of breaking it gives that union a crystal type name Anon1.

Let me know if there is anything I can do to improve this, but I personally see it as an acceptable solution. This looks like an edge case that probably won't pop up much. I couldn't even figure out how to reproduce the bug for the specs.

@bcardiff
Copy link
Member

@watzon I think we would need a small spec at least for this. Otherwise it will get broken easy in the future.

@watzon
Copy link
Author

watzon commented Jun 10, 2020

If you can help me come up with something that reproduces that behavior I'd be happy to add one. I'm having a hard time understanding what's going on in that C header that was causing things to break. I tried creating a minimal example for a spec, but was unable to.

@olbat
Copy link
Contributor

olbat commented Jun 12, 2020

👋, here is a short code snippet containing a function declaration to reproduce the issue:

void func(union{ char x; int i; });

@watzon
Copy link
Author

watzon commented Jun 12, 2020

Ok I added a parser spec and fixed a couple warnings coming from the 0.35.0 update. I think to really reproduce the original issue I might have to add another spec to the lib_body_transformer_spec though.

This pull request was closed.
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.

None yet

3 participants