fzyzcjy / flutter_rust_bridge

Flutter/Dart <-> Rust binding generator, feature-rich, but seamless and simple.
https://fzyzcjy.github.io/flutter_rust_bridge/
MIT License
3.64k stars 256 forks source link

[Bug] Unable to parse use statements when having `use a::b as c`? #360

Closed polypixeldev closed 1 year ago

polypixeldev commented 2 years ago

Describe the bug

I am working on a Flutter project that uses libp2p, and I came across this tool to bind rust-libp2p to the Flutter project. I followed the "Integrating with existing projects" instructions on the docs, using the cloned repository as the crate. I got through the instructions until the last step of running the tool. The logs are shown below. After some looking around, the issue appears to be the use statements in the Rust code.

Codegen logs with RUST_LOG=debug env variable

user@ubuntu:~/Documents/programs/project$ RUST_LOG=debug flutter_rust_bridge_codegen -r rust-libp2p/src/lib.rs -d lib/bridge_generated.dart -c ios/Runner/bridge_generated.h
[2022-02-19T19:56:10Z DEBUG flutter_rust_bridge_codegen::commands] execute command: bin=sh args=["-c", "dart pub global list"] current_dir=None cmd="sh" "-c" "dart pub global list"
[2022-02-19T19:56:10Z DEBUG flutter_rust_bridge_codegen::commands] command="sh" "-c" "dart pub global list" stdout=dartdoc 4.1.0
    ffigen 4.1.3
    protoc_plugin 20.0.0
     stderr=
[2022-02-19T19:56:10Z DEBUG flutter_rust_bridge_codegen::commands] execute command: bin=sh args=["-c", "test -x \"$(which cbindgen)\""] current_dir=None cmd="sh" "-c" "test -x \"$(which cbindgen)\""
[2022-02-19T19:56:10Z DEBUG flutter_rust_bridge_codegen::commands] command="sh" "-c" "test -x \"$(which cbindgen)\"" stdout= stderr=
[2022-02-19T19:56:10Z DEBUG flutter_rust_bridge_codegen::commands] 
[2022-02-19T19:56:10Z INFO  flutter_rust_bridge_codegen] Picked config: Opts { rust_input_path: "/home/user/Documents/programs/project/rust-libp2p/src/lib.rs", dart_output_path: "/home/user/Documents/programs/project/lib/bridge_generated.dart", dart_decl_output_path: None, c_output_path: "/home/user/Documents/programs/project/ios/Runner/bridge_generated.h", rust_crate_dir: "/home/user/Documents/programs/project/rust-libp2p", rust_output_path: "/home/user/Documents/programs/project/rust-libp2p/src/bridge_generated.rs", class_name: "Libp2P", dart_format_line_length: 80, skip_add_mod_to_lib: false, llvm_path: ["/opt/homebrew/opt/llvm", "/usr/local/opt/llvm", "/usr/lib/llvm-9/lib", "/usr/lib/llvm-10/lib", "/usr/lib/llvm-11/lib", "/usr/lib/llvm-12/lib", "/usr/lib/llvm-13/lib", "/usr/lib/", "/usr/lib64/"], llvm_compiler_opts: "", manifest_path: "/home/user/Documents/programs/project/rust-libp2p/Cargo.toml" }
[2022-02-19T19:56:10Z INFO  flutter_rust_bridge_codegen] Phase: Parse source code to AST
[2022-02-19T19:56:10Z INFO  flutter_rust_bridge_codegen] Phase: Parse AST to IR
[2022-02-19T19:56:11Z DEBUG flutter_rust_bridge_codegen::source_graph] Trying to parse "/home/user/Documents/programs/project/rust-libp2p/src/transport_ext.rs"
[2022-02-19T19:56:11Z DEBUG flutter_rust_bridge_codegen::source_graph] Trying to parse "/home/user/Documents/programs/project/rust-libp2p/src/bandwidth.rs"
[2022-02-19T19:56:11Z DEBUG flutter_rust_bridge_codegen::source_graph] WARNING: flatten_use_tree() found an import rename (use a::b as c). flatten_use_tree() will now abort.
[2022-02-19T19:56:11Z DEBUG flutter_rust_bridge_codegen::source_graph] WARNING: This happened while parsing Path(UsePath { ident: Ident(std), colon2_token: Colon2, tree: Group(UseGroup { brace_token: Brace, items: [Path(UsePath { ident: Ident(convert), colon2_token: Colon2, tree: Rename(UseRename { ident: Ident(TryFrom), as_token: As, rename: Ident(_) }) }), Comma, Name(UseName { ident: Ident(io) }), Comma, Path(UsePath { ident: Ident(pin), colon2_token: Colon2, tree: Name(UseName { ident: Ident(Pin) }) }), Comma, Path(UsePath { ident: Ident(sync), colon2_token: Colon2, tree: Group(UseGroup { brace_token: Brace, items: [Path(UsePath { ident: Ident(atomic), colon2_token: Colon2, tree: Name(UseName { ident: Ident(Ordering) }) }), Comma, Name(UseName { ident: Ident(Arc) })] }) }), Comma, Path(UsePath { ident: Ident(task), colon2_token: Colon2, tree: Group(UseGroup { brace_token: Brace, items: [Name(UseName { ident: Ident(Context) }), Comma, Name(UseName { ident: Ident(Poll) })] }) }), Comma] }) })
[2022-02-19T19:56:11Z DEBUG flutter_rust_bridge_codegen::source_graph] WARNING: This use statement will be ignored.
[2022-02-19T19:56:11Z DEBUG flutter_rust_bridge_codegen::source_graph] Trying to parse "/home/user/Documents/programs/project/rust-libp2p/src/simple.rs"
thread 'main' panicked at 'internal error: entered unreachable code', /home/user/.cargo/registry/src/github.com-1ecc6299db9ec823/flutter_rust_bridge_codegen-1.20.0/src/source_graph.rs:442:38
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

To Reproduce

  1. Create a new Flutter project
  2. Clone the rust-libp2p repository
  3. Follow the instructions in the docs for an existing project, using the cloned repository as the crate

Expected behavior

I expected the tool to generate the bindings

Generated binding code

No response

OS

Ubuntu 20.04

Version of flutter_rust_bridge_codegen

1.20.0

Flutter info

[✓] Flutter (Channel stable, 2.10.1, on Ubuntu 20.04.3 LTS 5.13.0-30-generic, locale en_US.UTF-8)
    • Flutter version 2.10.1 at /home/user/snap/flutter/common/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision db747aa133 (10 days ago), 2022-02-09 13:57:35 -0600
    • Engine revision ab46186b24
    • Dart version 2.16.1
    • DevTools version 2.9.2

[✓] Android toolchain - develop for Android devices (Android SDK version 32.0.0)
    • Android SDK at /home/user/Android/Sdk
    • Platform android-32, build-tools 32.0.0
    • Java binary at: /snap/android-studio/119/android-studio/jre/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.11+0-b60-7590822)
    • All Android licenses accepted.

[✗] Chrome - develop for the web (Cannot find Chrome executable at google-chrome)
    ! Cannot find Chrome. Try setting CHROME_EXECUTABLE to a Chrome executable.

[✓] Linux toolchain - develop for Linux desktop
    • clang version 6.0.0-1ubuntu2 (tags/RELEASE_600/final)
    • cmake version 3.10.2
    • ninja version 1.8.2
    • pkg-config version 0.29.1

[✓] Android Studio (version 2021.1)
    • Android Studio at /snap/android-studio/119/android-studio
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.11+0-b60-7590822)

[✓] VS Code
    • VS Code at /snap/code/current
    • Flutter extension version 3.34.0

[✓] Connected device (1 available)
    • Linux (desktop) • linux • linux-x64 • Ubuntu 20.04.3 LTS 5.13.0-30-generic

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.

Version of clang++

10.0.0-4ubuntu1

Version of cbindgen

0.20.0

Version of ffigen

4.1.3

Additional context

No response

welcome[bot] commented 2 years ago

Hi! Thanks for opening your first issue here! :smile:

Desdaemon commented 2 years ago

[2022-02-19T19:56:11Z DEBUG flutter_rust_bridge_codegen::source_graph] WARNING: flatten_use_tree() found an import rename (use a::b as c). flatten_use_tree() will now abort.

I think this might be the reason why, as import renames are not yet implemented.

polypixeldev commented 2 years ago

I think this might be the reason why, as import renames are not yet implemented.

Will they be implemented?

The line that throws the error seems to be https://github.com/fzyzcjy/flutter_rust_bridge/blob/4ceb7124e2c245a11985fa064c147988886c56c8/frb_codegen/src/source_graph.rs#L442 I don't know much about how the source graph works, but it seems that handling a match of UseTree::Group would fix the panicking.

Desdaemon commented 2 years ago

@Unoqwy I think the unreachable case mentioned here may actually cover something like this as well: use a::{*}, which I think is common enough in normal code. Maybe we should upgrade this to an unimplemented!?

@Poly-Pixel For now if you have imports that look like this, you probably can fix it by flattening the use tree like this:

// use foo::{*, bar};
use foo::*;
use foo::bar;
Unoqwy commented 2 years ago

@Unoqwy I think the unreachable case mentioned here may actually cover something like this as well: use a::{*}, which I think is common enough in normal code. Maybe we should upgrade this to an unimplemented!?

Would make sense, yeah. You might have me mistaken for @SecondFlight though, they were the one to implement this.

Desdaemon commented 2 years ago

My bad, I'll page @SecondFlight.

SecondFlight commented 2 years ago

Hm... I can't tell exactly what's going on, except that I left a comment saying what would cause that, and I think the comment is wrong.

use parsing isn't actually being used by anything (the code that uses source_graph.rs is pretty simplistic), so the smart thing to do here might be to just disable that part of source_graph.rs until it's needed.

polypixeldev commented 2 years ago

It seems like there are two issues here: the use a::{*} not being implemented and the use a::b as c not being implemented. Should this be separated into two issues?

I have disabled the line that panics with the use a::{*} syntax, and I am getting a different error: thread 'main' panicked at called Option::unwrap() on a None value', /home/user/.cargo/registry/src/github.com-1ecc6299db9ec823/flutter_rust_bridge_codegen-1.20.0/src/source_graph.rs:340:41. I'm not sure exactly what causes this, although it could be the import renames.

fzyzcjy commented 2 years ago

364 is merged.

SecondFlight commented 2 years ago

@Poly-Pixel That should fix the issue. I haven't had a chance to try it so please let me know if you're still seeing issues with it.

polypixeldev commented 2 years ago

@SecondFlight It seems to have fixed the warnings about import renames and the program no longer panicks due to the import groups, but I am now getting the error that I mentioned in my last comment. Here are the logs:

user@ubuntu:~/Documents/programs/project$ RUST_LOG=debug flutter_rust_bridge_codegen -r rust-libp2p/src/lib.rs -d lib/bridge_generated.dart
[2022-02-21T01:12:36Z DEBUG flutter_rust_bridge_codegen::commands] execute command: bin=sh args=["-c", "dart pub global list"] current_dir=None cmd="sh" "-c" "dart pub global list"
[2022-02-21T01:12:37Z DEBUG flutter_rust_bridge_codegen::commands] command="sh" "-c" "dart pub global list" stdout=dartdoc 4.1.0
    ffigen 4.1.3
    protoc_plugin 20.0.0
     stderr=
[2022-02-21T01:12:37Z DEBUG flutter_rust_bridge_codegen::commands] execute command: bin=sh args=["-c", "test -x \"$(which cbindgen)\""] current_dir=None cmd="sh" "-c" "test -x \"$(which cbindgen)\""
[2022-02-21T01:12:37Z DEBUG flutter_rust_bridge_codegen::commands] command="sh" "-c" "test -x \"$(which cbindgen)\"" stdout= stderr=
[2022-02-21T01:12:37Z DEBUG flutter_rust_bridge_codegen::commands] 
[2022-02-21T01:12:37Z INFO  flutter_rust_bridge_codegen] Picked config: Opts { rust_input_path: "/home/user/Documents/programs/project/rust-libp2p/src/lib.rs", dart_output_path: "/home/user/Documents/programs/project/lib/bridge_generated.dart", dart_decl_output_path: None, c_output_path: ["/tmp/.tmpBi04xk.h"], rust_crate_dir: "/home/user/Documents/programs/project/rust-libp2p", rust_output_path: "/home/user/Documents/programs/project/rust-libp2p/src/bridge_generated.rs", class_name: "Libp2P", dart_format_line_length: 80, skip_add_mod_to_lib: false, llvm_path: ["/opt/homebrew/opt/llvm", "/usr/local/opt/llvm", "/usr/lib/llvm-9/lib", "/usr/lib/llvm-10/lib", "/usr/lib/llvm-11/lib", "/usr/lib/llvm-12/lib", "/usr/lib/llvm-13/lib", "/usr/lib/", "/usr/lib64/"], llvm_compiler_opts: "", manifest_path: "/home/user/Documents/programs/project/rust-libp2p/Cargo.toml", dart_root: Some("/home/user/Documents/programs/project"), build_runner: true }
[2022-02-21T01:12:37Z INFO  flutter_rust_bridge_codegen] Phase: Parse source code to AST
[2022-02-21T01:12:37Z INFO  flutter_rust_bridge_codegen] Phase: Parse AST to IR
[2022-02-21T01:12:38Z DEBUG flutter_rust_bridge_codegen::source_graph] Trying to parse "/home/user/Documents/programs/project/rust-libp2p/src/transport_ext.rs"
[2022-02-21T01:12:38Z DEBUG flutter_rust_bridge_codegen::source_graph] Trying to parse "/home/user/Documents/programs/project/rust-libp2p/src/bandwidth.rs"
[2022-02-21T01:12:38Z DEBUG flutter_rust_bridge_codegen::source_graph] Trying to parse "/home/user/Documents/programs/project/rust-libp2p/src/simple.rs"
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /home/user/.cargo/registry/src/github.com-1ecc6299db9ec823/flutter_rust_bridge_codegen-1.21.0/src/source_graph.rs:339:41
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
SecondFlight commented 2 years ago

Thanks for testing it. I'll dig into this a bit more.

SecondFlight commented 2 years ago

It looks like the issue is an invalid assumption I made.

If a file has a mod statement, I look for a folder matching the statement which has a mod.rs file. If I can't find a matching folder, I look for a file matching the mod statement. If I can't find either, the code breaks.

rust-libp2p breaks this assumption. Its tutorials module doesn't have a matching tutorials.rs or tutorials/mod.rs. In this case it's fine to ignore since the tutorials module only has doc comments. However, I'm worried this may point to a bigger issue with source_graph.rs. I can just ignore cases that don't match my assumptions, but I don't want to cause code breaks in cases that aren't just doc comments.

Anyone know why this configuration is allowed? Specifically:

Cargo.toml
lib.rs
tutorials
├─ file1.rs
└─ file2.rs
// lib.rs
mod tutorials;
SecondFlight commented 2 years ago

@Poly-Pixel I dug into your use-case a bit, and I don't think flutter_rust_bridge supports what you're trying here.

The scope of flutter_rust_bridge is currently fairly narrow. It takes the functions exposed in a single Rust file and allows them to be called from Flutter. You can use types from anywhere within the same crate, but your functions currently must all be defined in one file.

However, you currently can't use imported types (see #249). Since rust-libp2p is structured as a monorepo, flutter_rust_bridge won't currently work on it. This, combined with flutter_rust_bridge being unable to see functions outside of your API file (rust-libp2p/src/lib.rs in this case), means that flutter_rust_bridge won't be able to automatically create a Flutter API from rust-libp2p.

That doesn't mean you can't use flutter_rust_bridge in your project, but you may have to get a bit creative. You can use any crate in your Rust API code as long as types from that crate are not used in the return value or arguments of your API function, as doing this exposes them to flutter_rust_bridge. Edit: #352 provides a manual workaround - thanks to @fzyzcjy for pointing this out.

There are examples included in this repo that show the general layout that flutter_rust_bridge expects.

fzyzcjy commented 2 years ago

Possibly related: https://github.com/fzyzcjy/flutter_rust_bridge/pull/352 by @Unoqwy

SecondFlight commented 2 years ago

Ah yeah, I haven't added that one to my mental toolbox just yet 😄

fzyzcjy commented 2 years ago

@Poly-Pixel In addition, you are welcomed to make a PR and I am looking forward to merging it!

As for how to implement it, @SecondFlight has made quite a bit of research and is an expert in this question. Discussions in #294 and #319 may also be helpful. Other code generators such as https://github.com/eqrion/cbindgen/issues/50 may also be helpful as a reference.

My naive thought is that, in order to understand types in external crates, @SecondFlight's source_graph.rs can be reused to parse each and every single crate; the code that needs to be implemented is mainly about searching and finding those crate source codes.

SecondFlight commented 2 years ago

@fzyzcjy That's certainly part of it, and not a hard one to solve. I believe cargo metadata can be used to map out dependencies and sub-dependencies and find their resolved on-disk locations, and each can be resolved to a Crate from source_graph.rs.

Our intermediate representation is currently simplistic. All structs are put into a single HashMap and are looked up by their identifiers, and the same is done for enums. This works for most cases but it doesn't understand scope, and it obviously doesn't scale.

The easy solution is to still use the simple map idea, but to make separate maps for each crate in the dependency tree. I believe this is what cbindgen does and it'll probably work in most cases. There are some edge cases like pub use that would be nice to handle here as well. In cases where the simple map doesn't work, it would be fine as long as 1) it doesn't cause a panic and 2) the types that caused the failure case aren't referenced in API function signatures.

The "correct" solution is to write code that understands scope, but this is a much harder problem to solve and would require a more significant overhaul of portions of the bindgen code.

SecondFlight commented 2 years ago

With all that said, it still only half-solves OP's original use-case. Their use-case requires solving bindgen in the general case, where you provide a Rust crate and bindgen outputs a Flutter API. That means a lot more than just understanding extern imports.

polypixeldev commented 2 years ago

Thank you for the responses. I am in the process of working on a PR for this. I am a little new to Rust though; just hit my first stack overflow in Rust while working on this.

fzyzcjy commented 2 years ago

@SecondFlight Great insights!

@Poly-Pixel Looking forward to your PR!

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

polypixeldev commented 2 years ago

I am still working out the PR, but I have been busy with other things recently. Sorry that it is taking so long.

fzyzcjy commented 2 years ago

Take your time :)

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] commented 1 year ago

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new issue.