decorum
decorum copied to clipboard
Add `Proxy::from_ref` and `Proxy::from_mut`
These methods allow user to cast references to f32/f64 into the appropriate Proxy
type.
Thanks for the PR! I really like this addition. The changes are consistent with the existing code, but I think we may want to approach this a bit differently and I have a few thoughts below.
I recently started experimenting with some changes to construction and conversions of proxy types and primitives on the convert
branch. I'd like to migrate away from using standard infallible traits like From
and Into
for conversions that panic, as this violates the documented contract for those traits. Instead, I think proxy types should be more explicit about panics at points of conversion. Similarly, I don't think AsRef
and AsMut
should be implemented to cast primitives to proxy types, as the panic violates the same contract of those traits (well, except for Total
, where the cast is infallible).
Though a bit less convenient, I'd suggest APIs more like the following to integrate with these upcoming changes (mutable conversions not shown):
impl<T, P> Proxy<T, P>
where
T: Float + Primitive,
P: Constraint<T>,
{
// Be sure to document that this is an `O(N)` operation!
pub fn try_from_slice<'a>(slice: &'a [T]) -> Result<&'a [Self], P::Error> { ... }
}
impl<T> Total<T>
where
T: Float + Primitive,
{
// This is a trivial cast. `O(1)`, unlike `try_from_slice`.
pub fn from_slice<'a>(slice: &'a [T]) -> &'a [Self] { ... }
}
impl<'a, T> From<&'a T> for &'a Total<T> {
// This is a trivial cast.
fn from(inner: &'a T) -> Self { ... }
}
// Use a macro to implement this for the remaining proxy and primitive types.
impl<'a> TryFrom<&'a f64> for &'a Finite<f64> {
type Error = ConstraintViolation;
fn try_from(inner: &'a f64) -> Result<Self, Self::Error> { ... }
}
This provides both fallible reference conversions for non-Total
proxies and an infallible conversion for Total
. We could also provide slice casts too! Unfortunately, the From
and TryFrom
trait implementations interact poorly, so TryFrom
must be implemented for the combinations of remaining proxy types and primitive types. This can be done easily enough in this macro, which is already doing the same for by-value conversions.
Finally, I think Constraint::filter_map
poses a bit of a problem here. It's designed so that constraints can do two things: report violations and map values (mapping all -0
to +0
, for example). Decorum has never actually used the latter capability though and values are never mapped into something different. We should remove this first, as it conflicts with these kinds of casts. I think we can simplify Constraint
like this:
pub trait Constraint<T>: Member<RealSet>
where
T: Float + Primitive,
{
fn check(inner: &T) -> Result<(), ConstraintViolation>;
}
I know this is a lot! Sorry for the wall of text. If you're still willing to work on this that'd be awesome! I'd slightly prefer to wait for the convert
branch to land (should happen very soon) but we don't have to. Please let me know what you'd prefer to do and thanks again for opening this. I hadn't considered these casts and I think they'll be useful!
I've just landed fc809c9 from the convert
branch with the changes I described above. I've also landed eae7b5e, which changes Constraint::filter_map
to Constraint::check
. I've updated the links in my previous comment. I think we should be good to go on this plan. :tada:
It's been some time, but I've landed b536468, which implements conversions from references and slices as described in my previous comment. Since that change effectively implements the features from the original PR changes, I'm going to close this. Thanks!