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 type forward declaration in the bindings #457

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

Conversation

damienmarchal
Copy link
Contributor

@damienmarchal damienmarchal commented Sep 23, 2024

The problem:
Depending on the definition order of the binded classes, binding "Base", and BaseData registered after then the use of BaseData in the function signature in Base will have invalid string name. This prevent stubgen to make their work.

In the PR I propose a way to fix that.

…nows about them.

The problem:
Depending on the definition order of the binded classes, there may have
incorrect types if Base is useing BaseData... but BaseData is only binded after Base.

The PR propose a solution for that using a decidcated "forward" registration patter.
@hugtalbot hugtalbot changed the title Example on how fix type forward declaration in the binding. Example on how fix type forward declaration in the binding Oct 2, 2024
@damienmarchal
Copy link
Contributor Author

Rebased and passing tests.

@alxbilger alxbilger mentioned this pull request Nov 6, 2024
@hugtalbot
Copy link
Contributor

PR looks clean 👍 but I would need some insight (the link with #456)

@damienmarchal
Copy link
Contributor Author

damienmarchal commented Nov 8, 2024

In #456 was a preliminary version of the current one on BaseData. The current PR covers the whole plugin. So to me it is to merge if CI pass.

@damienmarchal damienmarchal changed the title Example on how fix type forward declaration in the binding Fix type forward declaration in the bindings Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants