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.61k stars 254 forks source link

An Issue with methods of Rust struct and other V2.0 questions #1906

Open dbsxdbsx opened 2 weeks ago

dbsxdbsx commented 2 weeks ago

This is my 1st time using Frb v2.0(with dev. 32) in my own project since Frb v1.x.

  1. An Issue with methods of Rust struct:
    If the struct is not defined in the Api folder, then its methods can't be generated --- This reminds me of the heavy time I spent on the multi-block issue PR to solve this issue(the potential recursive issue, because one of the fields of the issued rust struct could be another rust struct/enum, which may contain its own public methods...). (Given the extensive time and effort I have already dedicated to this issue, I am hesitant to consider further contributions in this area, especially V2.0 is still not stable and under active development. My latest v1.x modification without this issue could be referred here, though it has other bugs)

Questions on V2.0:

  1. Within V2.0, for the tag rust_input in flutter_rust_bridge.yaml, how to specify more than one file respectively? (Seems no way at present) I know rust_input: rust/src/api/**/*.rs is supported, but how to specify more than one file respectively.
  2. Within V2.0, now all the API fns can be directly invoked without a prefix, But I still suggest generating Dart Classes accordingly as in V1.x to make it more distinguishable. For example, in multi-block cases, with prefixes like Api1. Api2. can make it more clear in Dart side, and it can also be used to distinguish the same name dart methods---indeed, currently, it can be done by manually adding the dart class, but a little verbose, since it has to be done everytime the rust side fns is changed.
  3. V2.0 introduces many macros, like #[frb(sync)],#[frb(ignore)], and compound usage case, I did find them in separated guidance file, but I think it is better to systematically introduce them in a specific page file in guidance in table style.
fzyzcjy commented 2 weeks ago

Hi!

If the struct is not defined in the Api folder, then its methods can't be generated

This looks like a bug, not a feature. Could you please create a minimal reproducible sample?

especially V2.0 is still not stable and under active development

Indeed it is almost stable, just put a .dev to allow rare breaking changes. The codebase will not have major overhaul. But I totally understand your feeling when a huge PR cannot be merged though we both tried our best! :(

Within V2.0, for the tag rust_input in flutter_rust_bridge.yaml, how to specify more than one file respectively?

Could you please elaborate a bit about your scenario? The rust/src/api/**/*.rs syntax supports whole folders directly.

I still suggest generating Dart Classes accordingly as in V1.x to make it more distinguishable.

I am not very sure, but it seems that Dart's coding convention does not add the prefix. When users want the prefix, they can import 'some_file.dart' as some_prefix; some_prefix.SomeClass(). However, the proposal looks like a useful feature under some cases!

but I think it is better to systematically introduce them in a specific page file in guidance in table style.

Totally agree! I will do that in the next batch

dbsxdbsx commented 2 weeks ago

For the method issue, let me just use my real example(by the way, I didn't find the official multi-block example=_=): the rust API is defined like this(the folder rust is at the root level): image in mod.rs, it is like this:

pub mod api_1;
pub mod api_2;

the lib.rs:

mod frb_generated; /* AUTO INJECTED BY flutter_rust_bridge. This line may not be accurate, and you can change it according to your needs. */
pub mod api;
mod database;
mod media;

use ctor::ctor;

#[ctor]
/// 这个函数会在库首次加载时被调用
fn called_on_load() {
    crate::init_logger("./logs/");
    log::info!("日志系统已初始化");
}
...

I guess using #[ctor] instead of #Frb[(init)], which has no side effect here, right?

, and in api_1.rs, the key 2 fns with customized struct MediaDoc is like this:

use crate::media::{CustomMediaCategory, MediaDoc};

#[tokio::main]
pub async fn init_media_docs(
    cms_list: CmsList,
    onlive_file_path: String,
    number: usize, // 所属分类要获取的总数,若category为None,则`number`代表所有总数
    target_media_type: Option<CustomMediaType>,
    media_name_to_search: Option<String>,
) -> Vec<MediaDoc> {
    if let Some(target_media_type) = target_media_type {
        if target_media_type.main_type == "直播" {
            return media::init_all_onlive_media_docs(&onlive_file_path, number).await;
        }
        media::init_all_cms_media_docs(cms_list, number, target_media_type)
            .await
            .unwrap_or_default()
    } else {
        return media::init_media_docs_by_name(cms_list, number, &media_name_to_search.unwrap())
            .await
            .unwrap_or_default();
    }
}

#[tokio::main]
pub async fn update_media_doc_detail(media_doc: MediaDoc) -> MediaDoc {
    media::update_media_doc_detail(media_doc)
        .await
        .unwrap_or_default()
}

again, the path tree is like this: image

and the struct is defind like this:

#[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct MediaDoc {
    pub names: Vec<String>,
  ...
}

impl MediaDoc {
    pub fn is_year_valid(&self) -> bool {
        self.year.chars().all(|c| c.is_ascii_digit()) && self.year.len() == 4
    }
    #[frb(sync)]
    pub fn get_tag_text(&self) -> String {
...
}

Could you please elaborate a bit about your scenario? The rust/src/api/*/.rs syntax supports whole folders directly.

What I want is like before, specific to each file, like rust_input: rust/src/api/api_1.rs rust/src/api/api_2.rs. Anyway, this issue is trifle, I am ok with the folder basis mechanism.

When users want the prefix, they can import 'some_file.dart' as some_prefix; some_prefix.SomeClass(). However, the proposal looks like a useful feature in some cases!

Yes! I can import like import 'package:xiao_dong_tv/src/rust/api/api_1.dart' as api_1;, with this line, it is totally solved, and I guess there is no need for this feature.

By the way, I wonder what config flags dump_all, local, and full_dep exactly mean? I saw their explanation in pub(crate) struct GenerateCommandArgsPrimary {...}, but I am still at a loss, don't know when need to use them.

fzyzcjy commented 2 weeks ago

by the way, I didn't find the official multi-block example

Because the example shows a folder where there can have many files, so in v2 there is no speciality between single-block and multi-block.

let me just use my real example

Oh, IIRC we do not scan impl blocks outside the API folder? Then it seems that we should scan it.

it is totally solved, and I guess there is no need for this feature.

Happy to see it is solved!

By the way, I wonder what config flags dump_all, local, and full_dep exactly mean?

dump_all and local is for debugging (not for end users) - I probably should make the comments clearer. full_dep is because, by default I do not require users to install llvm (which ffigen requires). If you need some features that are missing without full_dep (and llvm), you can enable it; but normally this can be left as default.

dbsxdbsx commented 2 weeks ago

Thanks, I have no other questions. Feel free to close it when needed.

fzyzcjy commented 2 weeks ago

You are welcome. No worries, maybe we can leave it open and I will work on some of them a bit later.

dbsxdbsx commented 3 days ago

Hopefully, this issue could be fixed ASAP, because I realize it can't be even worked around: For example, for the target struct named MediaDoc which both need fields and methods to be called in Dart, if putting the struct to the api_1.rs or api_2.rs, then it would raise the issue:

Error: Will generate duplicated class names (["MediaDoc"]). This is often because the type is auto inferred as both opaque and non-opaque. Try to 
add `#[frb(opaque)]` or `#[frb(non_opaque)]` to the struct, or change code that uses it.

By tagging with #[frb(non_opaque)], it has the same error msg. By tagging #[frb(opaque)], there is no error, but there is no field that can be called in Dart.

By the way, I also tried to put the struct in api/mod.rs(which I know is discouraged), then it had the same issue as putting the struct in the original non api folder.

In a word, I just can't find a workaround for dealing with a rust struct in which both fields and methods need to be called in Dart and used at least in one of the API files.

fzyzcjy commented 3 days ago

Hi, could you please provide a minimal reproducible sample? With your description I am not sure why it is both inferred as opaque and non-opaque... It should be reasonable to be inferred as non-opaque.

dbsxdbsx commented 3 days ago

Hi, could you please provide a minimal reproducible sample? With your description I am not sure why it is both inferred as opaque and non-opaque... It should be reasonable to be inferred as non-opaque.

Yes, exactly, I just found that not all struct would be failed, but with this one:

pub struct MyTestStruct {
    pub name: String,
    pub age: u32,
}
impl MyTestStruct {
    // comment out this function would make everything generated ok.
    // but with `pub` and `&mut self`, it would cause error.
    pub fn test_mut(&mut self) {
        self.name = "test_mut".to_string();
    }
}

IIRC, &mut self is supported due to some version of Frb v2.0.

fzyzcjy commented 3 days ago

I see: &mut self means that, it must be opaque. This is because non-opaque types are like "plain old java object (POJO)" (if you are familiar with that concept), and the object is created each and every time a function is called. Therefore, it is not possible to have a &mut, since it is not possible to modify the one on Dart side even if we do self.name = ....

By tagging #[frb(opaque)], there is no error, but there is no field that can be called in Dart.

I am thinking about another approach: What if we automatically generate getters and setters for opaque types? Like,

class MyTestStruct { // opaque type
  String get name => ..auto call rust..;
  set name(String val) => ..auto call rust..;
}
dbsxdbsx commented 3 days ago

I am thinking about another approach: What if we automatically generate getters and setters for opaque types? Like,

I think it could be a good idea. And I guess it means that the field needs to be called in Dart as methods like that in C#.

fzyzcjy commented 3 days ago

And I guess it means that the field needs to be called in Dart as methods like that in C#.

Not very sure, could you please elaborate a bit? In Dart we do have getters and setters, thus users can write myObject.myField and it indeed calls a real function String get myField => ...your implementation...

dbsxdbsx commented 3 days ago

Oh, so I misunderstood your idea. I am ok with both myObject.myField and myObject.myField()--the latter style is an encouraged way to interact with the field in a struct/class in some languages, like C# or even Rust.

I just realized there is a workaround at present--- Write Get/Set methods in Rust for all fields needed to be called in Dart, though it is quite tedious when there are many fields that need to be called in Dart.

fzyzcjy commented 3 days ago

the latter style is an encouraged way to interact with the field in a struct/class in some languages, like C# or even Rust.

I think so (Rust only supports the latter IIRC). On the other hand, it seems that Dart/Flutter encourages the former way, so I guess we can follow Dart's convension.

I just realized there is a workaround at present--- Write Get/Set methods in Rust for all fields needed to be called in Dart, though it is quite tedious when there are many fields that need to be called in Dart.

Totally agree it is tedious!

Talking about accessors, one big problem is that, it is good for things like String and int, but what if a field is of some complex type, or even some opaque type?

pub struct A {
  pub field b: B,
}

#[frb(opaque)]
pub struct B { }

Consider what is generated under the hood:

// impossible, since we do not support returning a reference (which will be troublesome for lifetime management)
fn a_field_b(a: &A) -> &B { a.b }

// is it ok? especially, sometimes it is not possible?
fn a_field_b(a: &A) -> B { a.b.clone() }

// seems also not good, because then the non-opaque `A` is consumed.
fn a_field_b(a: A) -> B { a.b }
dbsxdbsx commented 3 days ago

Talking about accessors, one big problem is that, it is good for things like String and int, but what if a field is of some complex type, or even some opaque type?

That is partly what I am concerned as I said at the very 1st post:

the potential recursive issue, because one of the fields of the issued rust struct could be another rust struct/enum, which may contain its own public methods...

And indeed as you said, this recursive issue could occur within params of methods of a rust struct/enum, but not only within fields. In addition, what makes the design more complex is that Frb v2.0 is designed to support &mut and & param.

Consider what is generated under the hood:

From a user's perspective, as long as a method or function is valid in Rust, it should be supported for use in Dart. Frankly, designing this integration is beyond my capability and likely requires a global project context consideration to ensure it is constructive and easy to maintain for future Frb development.