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

[experimental-feature] Error when running third-party scanner #2198

Open
pattobrien opened this issue Jul 12, 2024 · 14 comments
Open

[experimental-feature] Error when running third-party scanner #2198

pattobrien opened this issue Jul 12, 2024 · 14 comments
Labels
awaiting Waiting for responses, PR, further discussions, upstream release, etc bug Something isn't working

Comments

@pattobrien
Copy link

pattobrien commented Jul 12, 2024

Describe the bug

I'm not able to run the third party scanner on package yrs, due to the below error.

Further in the debug logs, I see the message "struct with unit fields are not supported yet, what about using struct DefaultProtocol {} or #[frb(opaque)] struct DefaultProtocol; instead" - but the struct in question cannot be modified, since it's a part of third party code.

[2024-07-12T09:20:51.188Z ERROR /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.0/src/library/utils/logs.rs:55] panicked at /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.0/src/library/codegen/ir/mir/ty/structure.rs:36:32:
no entry found for key=MirStructIdent(NamespacedName { namespace: Namespace { joined_path: "yrs::sync::protocol" }, name: "DefaultProtocol" })
thread 'main' panicked at /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.0/src/library/codegen/ir/mir/ty/structure.rs:36:32:
no entry found for key=MirStructIdent(NamespacedName { namespace: Namespace { joined_path: "yrs::sync::protocol" }, name: "DefaultProtocol" })

Steps to reproduce

  1. clone the https://github.com/fzyzcjy/flutter_rust_bridge/tree/master/frb_example/dart_minimal example package.
  2. add yrs as a dependency.
  3. modify flutter_rust_bridge.yaml with rust_input: crate::api,yrs.
  4. run the generator via flutter_rust_bridge_codegen generate

Logs

[2024-07-12T09:22:47.475Z INFO /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.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. (key="Values", objects={"name":"yrs::types::map/Values","visibility":"Public","sources":["Normal"],"mirror":true}, {"name":"yrs::iter/Values","visibility":"Restricted","sources":["Normal"],"mirror":true})
[2024-07-12T09:22:47.475Z INFO /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.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. (key="Doc", objects={"name":"crate::api::yrs/Doc","visibility":"Public","sources":["Normal"],"mirror":true}, {"name":"yrs::doc/Doc","visibility":"Public","sources":["Normal"],"mirror":true})
[2024-07-12T09:22:47.475Z INFO /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.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. (key="Options", objects={"name":"yrs::doc/Options","visibility":"Public","sources":["Normal"],"mirror":true}, {"name":"yrs::undo/Options","visibility":"Public","sources":["Normal"],"mirror":true})
[2024-07-12T09:22:47.475Z INFO /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.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. (key="Event", objects={"name":"yrs::sync::awareness/Event","visibility":"Public","sources":["Normal"],"mirror":true}, {"name":"yrs::undo/Event","visibility":"Public","sources":["Normal"],"mirror":true})
[2024-07-12T09:22:47.475Z INFO /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.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. (key="Iter", objects={"name":"yrs::types/Iter","visibility":"Restricted","sources":["Normal"],"mirror":true}, {"name":"yrs::branch/Iter","visibility":"Restricted","sources":["Normal"],"mirror":true})
[2024-07-12T09:22:47.476Z INFO /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.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. (key="Error", objects={"name":"yrs::encoding::read/Error","visibility":"Public","sources":["Normal"],"mirror":true}, {"name":"yrs::sync::awareness/Error","visibility":"Public","sources":["Normal"],"mirror":true}, {"name":"yrs::sync::protocol/Error","visibility":"Public","sources":["Normal"],"mirror":true})
[2024-07-12T09:22:47.491Z INFO /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.0/src/library/codegen/parser/mir/parser/ty/enum_or_struct.rs:73] Skip parsing enum_or_struct `NamespacedName { namespace: Namespace { joined_path: "yrs::sync::protocol" }, name: "DefaultProtocol" }` because of error (e=struct with unit fields are not supported yet, what about using `struct DefaultProtocol {}` or `#[frb(opaque)] struct DefaultProtocol;` instead

Stack backtrace:
   0: std::backtrace::Backtrace::create
   1: anyhow::error::<impl anyhow::Error>::msg
   2: lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::ty::structure::<impl lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::ty::TypeParserWithContext>::parse_struct
   3: lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::ty::enum_or_struct::EnumOrStructParser::parse
   4: lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::ty::path::<impl lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::ty::TypeParserWithContext>::parse_type_path
   5: lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::ty::ty::<impl lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::ty::TypeParserWithContext>::parse_type
   6: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::try_fold
   7: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter
   8: core::iter::adapters::try_process
   9: lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::trait_impl::parse
  10: lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::parse
  11: lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parse
  12: lib_flutter_rust_bridge_codegen::library::codegen::parser::early_generator::execute
  13: lib_flutter_rust_bridge_codegen::library::codegen::generate_once
  14: lib_flutter_rust_bridge_codegen::library::codegen::controller::run
  15: lib_flutter_rust_bridge_codegen::library::codegen::generate
  16: flutter_rust_bridge_codegen::main
  17: std::sys_common::backtrace::__rust_begin_short_backtrace
  18: std::rt::lang_start::{{closure}}
  19: std::rt::lang_start_internal
  20: _main)
[2024-07-12T09:22:47.524Z INFO /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.0/src/library/codegen/parser/mir/parser/ty/enum_or_struct.rs:73] Skip parsing enum_or_struct `NamespacedName { namespace: Namespace { joined_path: "yrs::block" }, name: "Unused" }` because of error (e=struct with unit fields are not supported yet, what about using `struct Unused {}` or `#[frb(opaque)] struct Unused;` instead

Stack backtrace:
   0: std::backtrace::Backtrace::create
   1: anyhow::error::<impl anyhow::Error>::msg
   2: lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::ty::structure::<impl lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::ty::TypeParserWithContext>::parse_struct
   3: lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::ty::enum_or_struct::EnumOrStructParser::parse
   4: lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::ty::path::<impl lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::ty::TypeParserWithContext>::parse_type_path
   5: lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::ty::ty::<impl lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::ty::TypeParserWithContext>::parse_type
   6: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::try_fold
   7: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter
   8: core::iter::adapters::try_process
   9: lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::trait_impl::parse
  10: lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parser::parse
  11: lib_flutter_rust_bridge_codegen::library::codegen::parser::mir::parse
  12: lib_flutter_rust_bridge_codegen::library::codegen::parser::early_generator::execute
  13: lib_flutter_rust_bridge_codegen::library::codegen::generate_once
  14: lib_flutter_rust_bridge_codegen::library::codegen::controller::run
  15: lib_flutter_rust_bridge_codegen::library::codegen::generate
  16: flutter_rust_bridge_codegen::main
  17: std::sys_common::backtrace::__rust_begin_short_backtrace
  18: std::rt::lang_start::{{closure}}
  19: std::rt::lang_start_internal
  20: _main)
[2024-07-12T09:22:47.526Z ERROR /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.0/src/library/utils/logs.rs:55] panicked at /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.0/src/library/codegen/ir/mir/ty/structure.rs:36:32:
no entry found for key=MirStructIdent(NamespacedName { namespace: Namespace { joined_path: "yrs::sync::protocol" }, name: "DefaultProtocol" })

Expected behavior

I would've expected two different outcomes:

  • that no error is seen (i.e. support for unit structs)
  • that I'm able to easily overwrite / ignore a particular struct or module from the config, in order to temporarily workaround the bug

Generated binding code

No response

OS

No response

Version of flutter_rust_bridge_codegen

No response

Flutter info

No response

Version of clang++

No response

Additional context

No response

@pattobrien pattobrien added the bug Something isn't working label Jul 12, 2024
Copy link

welcome bot commented Jul 12, 2024

Hi! Thanks for opening your first issue here! 😄

@fzyzcjy
Copy link
Owner

fzyzcjy commented Jul 12, 2024

Hmm, feel free to PR to support that, which may not be super hard (since we already support struct Something {}). I may also work on it later.

For workaround, you can always fallback to the manual mode. In other words, just write down a simple wrapper of the API.

@fzyzcjy fzyzcjy added the awaiting Waiting for responses, PR, further discussions, upstream release, etc label Jul 12, 2024
@pattobrien
Copy link
Author

Hmm, feel free to PR to support that, which may not be super hard (since we already support struct Something {}). I may also work on it later.

I actually don't believe that turned out to be the issue; the verbose logging created too much noise around unit structs, and after forking the yrs repo and modifying all unit structs, I was still getting the same cryptic errors.

[2024-07-12T16:22:28.637Z ERROR /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.0/src/library/utils/logs.rs:55] panicked at /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.0/src/library/codegen/ir/mir/ty/structure.rs:36:32:
no entry found for key=MirStructIdent(NamespacedName { namespace: Namespace { joined_path: "yrs::sync::protocol" }, name: "DefaultProtocol" })
thread 'main' panicked at /Users/pattobrien/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_codegen-2.1.0/src/library/codegen/ir/mir/ty/structure.rs:36:32:

I forked the frb repo, added yrs to the config and cargo on the integrate_third_party example. If you're interested, you should be able to see the same error that I'm experiencing.

I wish I could be more useful here, but I'm not familiar enough with Rust to debug this further myself!

I'm going to try to use the package with manual bindings in the mean time :)

@fzyzcjy
Copy link
Owner

fzyzcjy commented Jul 12, 2024

Ok I see. IIRC that error can happen when the package has too fancy use/mod statements (it may not be the case here, but just one example). I hope I can check that and make frb understand the fancy code in yrs in the future, but maybe not soon, since the fix may be time-consuming and it is an experimental feature and has workarounds. Thus yes, maybe try to use manual bindings as a workaround (a lot of bridge-like packages only provide that way iirc).

@pattobrien
Copy link
Author

pattobrien commented Jul 13, 2024

Thanks so much for looking into the issue!

The manual bindings have been working well, though the process is slow for such a large package. I think it would be very helpful to be able to configure globs/types to include in the parsing process (similar toffigen and jnigen) so that I could auto-generate most types, and do the rest manually... but thats a topic for another ticket :)

Please feel free to re-word the description/title for the "fancy use/mod statements", or close if there's an existing issue. I can keep that repository alive so that you have something to test against.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Jul 13, 2024

The manual bindings have been working well, though the process is slow for such a large package

I think so, that is why I made the automatic way - to save time especially for large packages.

be able to configure globs/types to include in the parsing process

What about rust_input: crate::api,yrs::hello::this::is::subpackage

Please feel free to re-word the description/title for the "fancy use/mod statements", or close if there's an existing issue. I can keep that repository alive so that you have something to test against.

No worries, maybe we can keep it like that for a bit of time, since I have not checked it and cannot guarantee causes

Again, also feel free to PR!

@pattobrien
Copy link
Author

What about rust_input: crate::api,yrs::hello::this::is::subpackage

Filtering doesn't work on third-party packages. No matter what combination of submodules you put there, the entire third-party crate seems to be parsed.

So there's no work around when there's any compilation errors, besides manually writing ALL third-party package code (which has turned out to be too much manually work for such a large package).

If filtering were to work, then this would be a lot less impactful of an issue.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Jul 18, 2024

I see. Again, feel free to PR for this! Alternatively I may work on it but again not very soon, since that feature is considered experimental and I do not have a lot of time implementing more now.

Another hacky workaround: Maybe fork the yrs package, and modify its content like:

/// flutter_rust_bridge:ignore
mod something_that_does_not_work_well;

By doing so, iirc that mod is not parsed at all.

@pattobrien
Copy link
Author

By doing so, iirc that mod is not parsed at all.

Yeah that seems to prevent the modules from being parsed, but then the module isn't imported into the generated "frb_generated.rs" module and therefore the rust program fails to compile.

It seems like /// flutter_rust_bridge:ignore and rust_input: some::module are intended to do similar things: to configure exactly which modules, types, etc are generated. But it seems like there are many inconsistencies between what's generated rust-side vs. dart-side, and unfortunately the error logging also doesn't help much:

  • some INFO messages seem to actually be fatal errors for the parser, e.g. in the case of unit-structs
  • When errors are thrown and backtraces are printed to the console, occasionally there's no message printed, or theres a message that doesn't point to the exact qualified source location that is causing the issue (helpful for knowing what mods/methods to ignore)

As stated in the docs, having the ability to ignore individual structs/enums would be helpful to circumvent errors.

I realize that it's hard to cover all edge cases for parsing/code-gen, and altogether I think the automatic scanning / code-gen works very well for 90% of the cases - some improved error logging and further work on ignoring structs/enums would go a long way in making the feature "good enough" IMO :)

@fzyzcjy
Copy link
Owner

fzyzcjy commented Jul 19, 2024

Totally agree, that's partially why the "scan whole 3rd patry crate" feature is marked as "experimental"!

@xqZ7vR9p
Copy link

xqZ7vR9p commented Sep 7, 2024

Hey, @pattobrien. Did you get any bright ideas on how to parse the yrs lib?

@fzyzcjy fzyzcjy changed the title Error when running third-party scanner [experimental-feature] Error when running third-party scanner Sep 11, 2024
@Musta-Pollo
Copy link

Did someone get it working?

@pattobrien
Copy link
Author

Hey, @pattobrien. Did you get any bright ideas on how to parse the yrs lib?

Sorry for the delayed response! Unfortunately no, I wasn't able to get yrs parsed - it seemed like a rabbit hole of issue after issue, and since I'm a novice at Rust it wasn't going to be simple for me to debug.

Sorry I couldn't have been of more help!

@fzyzcjy
Copy link
Owner

fzyzcjy commented Oct 8, 2024

I guess yrs itself is too complex to be auto parsed by this experimental feature today. But if many people are interested in it, one way is to collaborate to manually handle the yrs, just like how people make a (semi-manual) binding from one language to another language.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting Waiting for responses, PR, further discussions, upstream release, etc bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants