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

Safe mode for collisions #2279

Open
aran opened this issue Sep 5, 2024 · 7 comments
Open

Safe mode for collisions #2279

aran opened this issue Sep 5, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@aran
Copy link
Contributor

aran commented Sep 5, 2024

Is your feature request related to a problem? Please describe.
I see a scary warning:

[2024-09-05T05:34:59.048Z INFO /Users/aran/.cargo/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.3.0/src/library/codegen/parser/hir/flat/transformer/merge_duplicate_transformer/mod.rs:72] There are still multiple objects with same key after merging, thus randomly pick one. This is an issue only if the object is indeed used.

I am using the object. In my flutter crate I pub use it, and then create afrb(mirror) on it. I'm not sure why this type has this error but some of my other mirrored types don't. With this info message, I am wondering if I have a bug that can bite me later.

Describe the solution you'd like

I want to run FRB in a very rust-y 'safe'/'strict' mode where if it could do the wrong thing, that's an error, with info on how to fix.

Describe alternatives you've considered
N/A

@aran aran added the enhancement New feature or request label Sep 5, 2024
@fzyzcjy
Copy link
Owner

fzyzcjy commented Sep 5, 2024

I'm not sure why this type has this error but some of my other mirrored types don't.

Could you please provide a minimal reproducible sample? Without example, I guess maybe you have this same name somewhere else. Usually mirroring should not trigger this warning.

I want to run FRB in a very rust-y 'safe'/'strict' mode where if it could do the wrong thing, that's an error, with info on how to fix.

Feel free to PR to add this! Currently I choose to make it INFO message (instead of a hard error), because it is mostly harmless (especially if you do not use it), and even if it does cause problem, picking the wrong type seems to cause the generated code have wrong type, then Rust compiler will stop you from compiling.

@aran
Copy link
Contributor Author

aran commented Sep 5, 2024

Sounds good, I will report back if I can figure out what exactly is causing it.

@aran
Copy link
Contributor Author

aran commented Sep 5, 2024

With a bit more looking, and your explanation comment on another issue, I think this specific issue is actually related to the third party crates feature, i.e. that the whole third-party crate gets scanned, not just the module listed in rust_input.

So my theory is:

api.rs has a pub use foo::bar::baz and mirrors it.
later, add a rust_input third party crate import for foo::bizz. Now the entire 'foo' crate gets scanned differently. FRB now has a different code path to generate foo::bar::baz as part of the overall crate scan for 'foo'.

I think the broader point is it's hard to have both convenient, zero-configuration scanning of Rust imports, and also fine-grained control over what Dart code is being generated and how. To do it, FRB would need to canonicalize all the Rust types in the system with the same logic as the Rust itself, and project that into Dart.

As for a safe mode, it is reassuring to know we would get a compile error if there is a bug. The error doesn't have to come from FRB, I just want to avoid doing anything that could hide bugs.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Sep 6, 2024

To do it, FRB would need to canonicalize all the Rust types in the system with the same logic as the Rust itself, and project that into Dart.

Yes, and that would be quite nontrivial, since Rust resolution logic is quite complex... (That's partially why this feature is marked as experimental)

As for a safe mode, it is reassuring to know we would get a compile error if there is a bug. The error doesn't have to come from FRB, I just want to avoid doing anything that could hide bugs.

Yes I agree (see my comments above)

@fzyzcjy
Copy link
Owner

fzyzcjy commented Sep 12, 2024

I thought about it a bit more, and seems that if we create a safe mode, then it is not very useful, because we cannot easily remove that hint for types that are scanned but not used.

@aran
Copy link
Contributor Author

aran commented Sep 12, 2024

Makes sense. Seems hard to do much better without being able to resolve Rust types the same way as Rust itself.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Sep 13, 2024

Totally agree...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants