yewstack / implicit-clone

Immutable types and ImplicitClone trait similar to Copy
20 stars 10 forks source link

How to use the library? #6

Closed guitarino closed 1 year ago

guitarino commented 2 years ago

Hello,

I'm trying to figure out how I can use the library correctly. Here's what I'm trying to do:

use implicit_clone::ImplicitClone;
use std::{cell::RefCell, rc::Rc};

#[derive(Clone)]
struct A {
    num: i32,
}

impl ImplicitClone for A {}

struct B<T>(Rc<RefCell<T>>);

impl<T> Clone for B<T> {
    fn clone(&self) -> Self {
        Self(self.0.clone())
    }
}

impl<T> ImplicitClone for B<T> {}

fn do_stuff(val: B<A>) {
    val.0.borrow_mut().num = 200;
}

fn do_stuff_2<F: Fn()>(closure: F) {
    closure();
}

fn main() {
    let a = B(Rc::new(RefCell::new(A { num: 100 })));
    do_stuff(a);
    do_stuff_2(|| a.0.borrow_mut().num = 200);
    println!("{}", a.0.borrow().num);
}

This results in a compiler error:

error[E0382]: borrow of moved value: `a`
  --> src/main.rs:32:16
   |
30 |     let a = B(Rc::new(RefCell::new(A { num: 100 })));
   |         - move occurs because `a` has type `B<A>`, which does not implement the `Copy` trait
31 |     do_stuff(a);
   |              - value moved here
32 |     do_stuff_2(|| a.0.borrow_mut().num = 200);
   |                ^^ --- borrow occurs due to use in closure
   |                |
   |                value borrowed here after move

When I change this to explicit clones:

    do_stuff(a.clone());
    do_stuff_2(|| a.clone().0.borrow_mut().num = 200);

It works as expected and prints out 200. Am I doing something wrong?

Using version implicit-clone = "0.3.1" and rust edition = "2021".

Thanks for help!

ColonelThirtyTwo commented 1 year ago

I'm not sure why this library describes ImplicitClone as "Behaves exactly like Copy but calls the Clone implementation instead", because it's untrue and impossible to do in Rust - it's impossible to overwrite the behavior of assignment. It may work like that in the context of Yew's html macro and its relatives, but that's macro magic covering things up and not an intrinsic property of Rust or the trait.

The trait is implemented as pub trait ImplicitClone: Clone {}; ie nothing additional to Clone.

Beyond that, the library provides some structures that are basically just Rc wrappers over std types. It's not very well written - as I pointed out in other issues, IString is missing PartialOrd and Ord. For some reason, it uses include! rather than Rust's module system.

cecton commented 1 year ago

Hello @guitarino @ColonelThirtyTwo @erindru @szagi3891

Yes you are misunderstanding the usage of this library. It is not meant to be used directly like what you are trying to achieve here. As @ColonelThirtyTwo pointed out, it is impossible to override the behavior of an assignment and you can't go over the borrow checker of Rust anyway. This library is designed to be used for other libraries to allow marking types as "cheap to clone" (or "implicitly cloneable").

So what does it do concretely speaking? Well, it has been made with Yew in mind where properties of components are passed through children using a trait called IntoPropValue. In other words, it's up to the user of implicit-clone (Yew in this case) to implement the kinda copy-ish behavior.

Feel free to re-open the issue if you have more questions.

It's not very well written - as I pointed out in other issues

How rude!!! May I remind you this project is in 0.x version for a reason? (PR are welcome btw :wink: )

ColonelThirtyTwo commented 1 year ago

@cecton I'm sorry, but as it is, using include! instead of mod makes this crate look very amateurish. It's also missing a lot of functionality, mostly in terms of implementing basic traits. As I pointed out, IString doesn't implement PartialOrd or Ord. Neither IArray nor IMap implement Extend. Nothing implements Borrow or AsRef.

The only reason I'm commenting is because yew makes me use IString. I think yew would be better off using something like smol_str or an interner like lasso. For everything else, I'm either using im_rc or just Rc with make_mut.

cecton commented 1 year ago

"amateurish"? Stop being awful? Please? You keep escalating and I keep enduring your attacks.

using include! instead of mod makes this crate look very amateurish.

The code is actually shared between 2 different modules (unsync.rs and sync.rs). Thus the include.

IString doesn't implement PartialOrd or Ord. Neither IArray nor IMap implement Extend. Nothing implements Borrow or AsRef.

Sorry I might repeat myself but... this is 0.x version. Yes! Lots of things are not perfect. Feel free to make PR to improve the quality of this crate. I'm not a super human I don't make perfect crates but I do believe that people, as a team, can create great products because everybody has a different point of view and one see flaws that the other doesn't.

So if you do are sincere about quality, why not make a PR? That way you can improve the quality of this project. Or why not implement one of the proposal you said (using smol_str, lasso, ...). Just do what you say and actually do something.

I think yew would be better off using something like smol_str or an interner like lasso. For everything else, I'm either using im_rc or just Rc with make_mut.

To be honest I would be really happy to have one less OSS project to maintain. Please go for it :pray:

guitarino commented 1 year ago

@cecton thank you for clarifying and for doing this work. I hope that people like @ColonelThirtyTwo are not discouraging. If they wanted to contribute positively, they would've already made a PR rather than complaining. Keep up the good work and ignore people who feel entitled to have code implemented for them the way they want

kirillsemyonkin commented 1 year ago

using include! instead of mod makes this crate look very amateurish

To be fair, include! does not work 100% at least with VSCode (I believe included files are not linted properly, as the IDE asks to add the .rs file somewhere using mod declaration). It is discouraged by the community, in favor of mod declarations (it is what Rust tried to fix when comparing it to C/C++, after all), although the include macro is in the Rust language for some reason. A bit less troublesome variant (as it would lint more-or-less properly, even if compiler on errors keeps saying "it is macro's fault, try nightly macro backtrace, bla bla bla") to me seems to be wrapping those whole 3 files in macros, that would accept either Rc or Arc.

The included files are quite large and would be a big trouble if we had to maintain two versions of sizable pieces of code, bugs/mere disappointments resulting from API mismatches would be jumping out quite often whenever we tried to add new features to them. If you, @ColonelThirtyTwo, is able to think of even better solution you are welcome to create a PR.