Skip to content
This repository has been archived by the owner on Jan 28, 2024. It is now read-only.

Support nested structs #126

Closed
wants to merge 2 commits into from
Closed

Conversation

jpnurmi
Copy link
Contributor

@jpnurmi jpnurmi commented Jan 2, 2021

Dart SDK master supports nested structs.
@mannprerak2
Copy link
Contributor

We can also target dart-lang/native#540 along with this.

And It is better to put these behind a flag in config for users to enable this, since this will only work on dart master branch as of now.

I think this should be held off until we merge #122 .

@dcharkes
Copy link
Contributor

dcharkes commented Jan 5, 2021

Super cool stuff! 👍

I second @mannprerak2, we should not merge this into the master branch until nested structs also becomes available in Dart stable. If we merge it into master we can no longer make any changes to package:ffigen and release them for Dart stable in the mean time.
(We could make a new branch dart-master which we build against the master branch of Dart as you did in this PR in test-package.yml. And release a 1.3.0-nested-structs.0 version. However, you can also just use a git dependency to depend on your own branch for the time being. Any preference @jpnurmi ?)

@mannprerak2 do you want to do the reviewing for this PR?

@jpnurmi
Copy link
Contributor Author

jpnurmi commented Jan 5, 2021

Sure, I was planning to get back to this after #122 has been merged. This can be kept in an experimental branch or made configurable as @mannprerak2 suggested. There's no rush from my side to have this upstream, I got the files already generated. :)

@mannprerak2
Copy link
Contributor

@mannprerak2 do you want to do the reviewing for this PR?

Sure.

@mannprerak2 mannprerak2 self-assigned this Jan 5, 2021
@mannprerak2
Copy link
Contributor

mannprerak2 commented Jan 5, 2021

@jpnurmi I now feel maybe it's better if we do create an experimental branch for this (dart-master as Daco suggested). Otherwise, we'd need to add and then later remove those configs, modify tests to run according to certain configurations (lots of stuff which would need to be reverted later on).

Also, we can target dart-lang/native#540 as a separate PR, since it needs to know if a struct had been properly generated to be able to pass it by value correctly.

@mannprerak2
Copy link
Contributor

@dcharkes I think you would need to create a dart-master branch so that @jpnurmi can make a PR on it.

@dcharkes
Copy link
Contributor

dcharkes commented Jan 5, 2021

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Support for Nested Structures
3 participants