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

Remove redundant boxing when posting CObject to Dart #65

Merged
merged 1 commit into from
Mar 10, 2025

Conversation

alexlapa
Copy link
Contributor

@alexlapa alexlapa commented Mar 5, 2025

&mut T can be safely coerced to *mut T without boxing

@shekohex
Copy link
Owner

shekohex commented Mar 5, 2025

Interesting, so with this new change, the code is still working as usual.

I wonder if this fixed your issue here #64?

Could you try to add a test case that covers your scenario?

@alexlapa
Copy link
Contributor Author

alexlapa commented Mar 6, 2025

@shekohex ,

I wonder if this fixed your issue here #64?

No, thats unrelated to my issue. Just a small thing i stumbled upon while debugging whats going on here.

Could you try to add a test case that covers your scenario?

Well, Im currently trying to create an MRE for #64 and that will probably go to dart-lang/native since this looks to be a Dart VM issue more and more.

@alexlapa alexlapa requested a review from shekohex March 6, 2025 16:24
@shekohex
Copy link
Owner

shekohex commented Mar 6, 2025

@fzyzcjy what do you think of this new change? Could you test it out in the fluter bridge?

Copy link
Owner

@shekohex shekohex left a comment

Choose a reason for hiding this comment

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

Looks good to me. Would be nice to get @fzyzcjy opinion here too. Then it is ready to be published.

@fzyzcjy
Copy link
Contributor

fzyzcjy commented Mar 7, 2025

Could you test it out in the fluter bridge?

Feel free to create a PR on flutter_rust_bridge, with minimal changes that bumps https://github.com/fzyzcjy/flutter_rust_bridge/blob/4c91787244b2043886caf471b30c9b5b37287ca2/Cargo.toml#L31 version! Then just check CI to see whether all tests in frb passes.

@alexlapa
Copy link
Contributor Author

alexlapa commented Mar 7, 2025

Feel free to create a PR on flutter_rust_bridge, with minimal changes that bumps

fzyzcjy/flutter_rust_bridge/pull/2596 will ping once CI is green

@alexlapa
Copy link
Contributor Author

@shekohex ,

frb CI is ok: fzyzcjy/flutter_rust_bridge#2596 (Lint::Rust failed due to web_sys update so its not realted)

@shekohex
Copy link
Owner

Awesome! Merging and will publish a new version shortly.

@shekohex shekohex merged commit 3bd502b into shekohex:master Mar 10, 2025
5 checks passed
@shekohex
Copy link
Owner

Released v0.1.27 🎉

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.

3 participants