Impl from collections for RpcValue
#6
Conversation
Add blanket implementations of traits: - From<Vec<T>> - From<BTreeMap<String, T>> - From<BTreeMap<i32, T>> for RpcValue where T: Into<RpcValue> This allows to perform uniform conversion to RpcValue for structs whose fields are either of a type convertible to RpcValue or a collection of such type. There is no specialization for T equals to `RpcValue` due to limitations of Rust, so the conversion is unnecessarily costly as it iterates through the collection instead of just moving the inner data. The compiler in release mode is able to generate optimized code of `From<Vec<RpcValue>>`, but unfortunately not for `BTreeMap`.
Replace new() and sequence of insert() with more idiomatic initialization from slice of tuples collected into a map.
If enabled, provides special `impl From<Collection<RpcValue>> for RpcValue` for List, Map and IMap, so it just moves the inner data without iterating the collection. Requires nightly to enable `min_specialization` compiler feature.
j4r0u53k
changed the title
RpcValue
j4r0u53k
force-pushed
the
impl-from-collections-for-rpcvalue
branch
from
6f76f31 to
e58df3b
Compare
| @@ -15,5 +15,12 @@ chrono = "0.4.38" | |||
| hex = "0.4.3" | |||
| clap = { version = "4.5.7", features = ["derive"] } | |||
|
|
|||
| [features] | |||
|
|
|||
| # Enables feature `min_specialization` of nightly compiler and provides | |||
There was a problem hiding this comment.
I which production project can we use unstable rust?
There was a problem hiding this comment.
It's entirely up to consumers of the lib whether they want to enable this feature or not. By default it's disabled.
The purpose of this feature is only to provide optimalization for converting a collection of RpcValues into RpcValue, where a user code decides to handle all the conversions in the uniform way - call .into() on any convertible data without special treatment of Collection<RpcValue> case.
There was a problem hiding this comment.
We have only one consumer currently - us. So this code is effectively dead. We can keep it until min_specialization will be stabilized, hopefully there will be some external consumers this time. My concern was just to know, how can be this feature utilized now.
There was a problem hiding this comment.
Whether or not there are currently any external consumers shouldn't be the concern here. Anybody can start a project at any time utilizing the feature if they consider it meaningful.
The min_specialization has been around for a while and it's highly demanded. I don't think it will get removed. However, it might not get stabilized for another couple of years, which would mean we should either give up on this feature completely, or at least leave it there as an option, disabled by default, which is what I propose.
There was a problem hiding this comment.
Well, we can keep it and revisit it when specialization will be stabilized even if it is far from finished rust-lang/rust#31844.
There was a problem hiding this comment.
I still wonder, why specialization generates necessity of macro used on lines rpcvalue.rs:208 - rpcvalue.rs:222, which wasn't needed before. My naive understanding is, that just special cases should be introduced, but the generic impl should remain the same.
There was a problem hiding this comment.
The cfg macros inside the impls are necessary because of the default keyword/marker in the definition of the generic implementations of from methods, which can appear only if the compiler feature is enabled.
No description provided.