EmbarkStudios / mirror-mirror

🪞 Powerful reflection library for Rust
Apache License 2.0
79 stars 2 forks source link

Remove unused `into_any` / `into_reflect` trait methods #74

Closed bnjbvr closed 1 year ago

bnjbvr commented 1 year ago

Consider these PR's commits as questions :grin: These methods seem unused both in the crate itself, and in our main use case. Why are they here? Do you have use cases for them in a near future? If not, I'd suggest adding them back only when needs be, following the YAGNI principle.

davidpdrsn commented 1 year ago

I see your point but since they don't really add much complexity I think they're worth having for completeness.

bnjbvr commented 1 year ago

Ok, I see that I can't do: let b: Box<dyn Reflect> = ...; let b2: Box<dyn Any> = b; because trait upcasting is still unstable. But into_reflect can be achieved easily that way: Box::new(i32) as Box<dyn Reflect>; what's the benefit of augmenting the binary size / API surface with a specific function to do it?

davidpdrsn commented 1 year ago

It enables doing something like this

fn foo(a: Box<dyn Reflect>) {
    let s: Box<String> = a.into_any().downcast::<String>().unwrap();
    let s: String = *s;
}

Which I don't think you can do another way without cloning if you didn't have this method 🤔

But I don't feel strongly about this so think we can just remove it. Its not like we're going 1.0 any time soon so we can just bring these methods back if we need them.