fzyzcjy / flutter_rust_bridge

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

Implement PartialEq and Eq for RustAutoOpaque? #2322

Closed patmuk closed 1 month ago

patmuk commented 1 month ago

Is your feature request related to a problem? Please describe. In https://github.com/fzyzcjy/flutter_rust_bridge/issues/2272#issuecomment-2345056097 you stated that PartEq is not straight-forward to implement due to the inner RWLock, which doesn't implement it.

However, having PartEq implemented for RustAutoOpaque would be helpful for assert_eq in unit tests. Surely a getter to the inner field could be written - but doing so I wonder is this shouldn't be baked into frb?

In short, wouldn't some like the following work?

impl PartialEq for RustAutoOpaque {
    fn eq(&self, other: &Self) -> bool {
        *self.blocking_read().data == *self.blocking_read().data
    }
}

Describe the solution you'd like

#[derive(PartEq)]
pub struct MyStruct {
   pub myField: RustAutoOpaque<String>
}

Describe alternatives you've considered

Implementing it on your own:

impl PartialEq for MyStruct {
    fn eq(&self, other: &Self) -> bool {
        *self.myField.blocking_read() == *other.myField.blocking_read()
    }
}

Additional context

I probably overlook something here - since RWLock isn't PartEq already. Is this too dangerous to produce a deadlock? I need it for unit tests only, so any locks would be released before the assert ... but maybe one could trigger blocking_read() without noticing in the normal program flow?

fzyzcjy commented 1 month ago

Yes I guess it is dangerous to have deadlock, or to have a eq magically hang for 10 seconds (waiting for another writer to release lock)... And even if we only intend for unit tests, other users may accidentially use it for real scenarios, and find this surprising behavior.

Given that Rust's RwLock itself is not Eq/PartialEq, I guess we can follow Rust itself's idea.

patmuk commented 1 month ago

Yes, I agree. I am doing the blocking_read directly in the unit test now. But thanks for discussing this :)

github-actions[bot] commented 1 month 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.