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.

Sign up for GitHub

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

Jump to bottom

Support forward declarations for C-like languages #1028

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Support forward declarations for C-like languages #1028

wants to merge 7 commits into from

Conversation

masnagam
Copy link

@masnagam masnagam commented Nov 6, 2024 •
edited
Loading

Output forward declarations for C-like languages:

  • Definition for opaque types will not output if those are used as pointer types
  • Declaration order is changed in order to solve problems of recursive type definitions

Limitations:

  • No forward declaration will be output when --style=type is specified
  • Forward declarations for template types in C++ are not supported

Fixes #43

@masnagam
Support forward declarations for C-like languages
23fccf2
@masnagam
Use IndexSet instead of HashSet
715707d
@masnagam
Fix the condition
2a59c8c
@masnagam
Fix test failures
6395981
@masnagam
Remove empty lines
38ebcfa
@masnagam
Update the expected results
7b12a62 
The following files are removed

- tests/expectations/std_lib_both.c
- tests/expectations/std_lib_both.compat.c

because opaque type definitions are replaced with forward
declarations.
@masnagam
Add test cases
7fcd29d
emilio
emilio requested changes Jan 15, 2025
View reviewed changes
Copy link
Collaborator

@emilio emilio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So... I'm not sure why we need a separate set around, it seems generally this should be tracked as part of Dependencies?

Also, I don't think we should unconditionally create forward declarations for non-problematic cases as you're doing here (anything that is a pointer gets forward-declared).

src/bindgen/ir/ty.rs
@@ -687,15 +688,54 @@ impl Type {
generic_params: &GenericParams,
library: &Library,
out: &mut Dependencies,
ptr_types: &mut IndexSet<GenericPath>,

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: space. But also rather than having an is_ptr param, it seems this should be handled in Type::Ptr itself?

@masnagam
Copy link
Author

So... I'm not sure why we need a separate set around, it seems generally this should be tracked as part of Dependencies?

Also, I don't think we should unconditionally create forward declarations for non-problematic cases as you're doing here (anything that is a pointer gets forward-declared).

i'll check your points and modify this PR when i have time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilio emilio emilio requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Generate forward declarations for structs with cyclical pointer references
2 participants
@masnagam @emilio