zbus-old Check if generic `impl<T: TryFrom<Value>> TryFrom<OwnedValue> for T` can work

In GitLab by @elmarco on Feb 11, 2021, 15:53

We implement a lot of conversion traits manually for Value and OwnedValue.

It seems reasonable to think that impl<T: TryFrom<Value>> TryFrom<OwnedValue> for T should work. Someone should give that a try and leave a comment if it can't be done.

Feb 11 '21 14:02 zeenix

In GitLab by @ids1024 on Feb 21, 2021, 21:23

This definitely seems desirable, but as far as I can tell it isn't possible:

   Compiling zvariant v2.5.0 (/home/ian/src/zbus/zvariant)
error[E0119]: conflicting implementations of trait `std::convert::TryFrom<owned_value::OwnedValue>`:
  --> zvariant/src/owned_value.rs:81:1
   |
81 | / impl<'a, T> TryFrom<OwnedValue> for T 
82 | | where
83 | |     T: TryFrom<Value<'a>, Error = crate::Error> {
84 | |
...  |
89 | |     }
90 | | }
   | |_^
   |
   = note: conflicting implementation in crate `core`:
           - impl<T, U> TryFrom<U> for T
             where U: Into<T>;

error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`owned_value::OwnedValue`)
  --> zvariant/src/owned_value.rs:81:10
   |
81 | impl<'a, T> TryFrom<OwnedValue> for T 
   |          ^ type parameter `T` must be covered by another type when it appears before the first local type (`owned_value::OwnedValue`)
   |
   = note: implementing a foreign trait is only possible if at least one of the types for which it is implemented is local, and no uncovered type parameters appear before that first local type
   = note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0119, E0210.
For more information about an error, try `rustc --explain E0119`.
error: could not compile `zvariant`

To learn more, run the command again with --verbose.

Feb 21 '21 20:02 zeenix

@ids1024 Thanks so much for trying it out though. These TryFrom impls conflicting with From (and TryInto with Into) is very annoying and still makes me question if 993d51d14b29b1223a8648df67bc8aad41fbf14d and 7036c13a689f5a44abd5288a5f8db242e439f54a were desirable changes. Having said that, perhaps we can imply something like !262 here?

Feb 22 '21 12:02 zeenix

In GitLab by @ids1024 on Feb 24, 2021, 16:29

Another related issue is that impl<T, U> TryFrom<U> for T where U: Into<T> results in an implementation of TryFrom with Error = Infallable, so it doesn't satisfy the V: TryFrom<Value<'v>, Error = crate::Error> bounds.

That's at least part of the cause for https://gitlab.freedesktop.org/dbus/zbus/-/issues/135.

Feb 24 '21 15:02 zeenix

In GitLab by @ids1024 on Mar 10, 2021, 01:11

I just noticed how OwnedValue is currently defined:

pub struct OwnedValue(Value<'static>);

Could this be changed to a type alias instead of a newtype, or would that not work for some reason?

type OwnedValue = Value<'static>;

It's possible to implement methods or even traits for a particular instance of a generic type. i.e. impl Value<'static> and impl SomeTrait for Value<'static>.

This could make some things much easier. Is there some issue with this I'm not considering?

Mar 10 '21 00:03 zeenix

Could this be changed to a type alias instead of a newtype, or would that not work for some reason?
type OwnedValue = Value<'static>;

Hmm.. interesting idea. I won't be against trying that.

This could make some things much easier. Is there some issue with this I'm not considering?

Strictly speaking this will be an API break but as long as the using code continues to build w/o any modifications, we're good.

Thanks to Jonas, I recently realized how for the related impl Into<ObjectPath> issue (!262), there is a way to treat From<T> for T as TryFrom<T> for T if there is a way to convert Infallible to zvariant::Error. I've a WIP branch that I really should finish up very soon. Perhaps that conversion also helps here? If it can, I'd prefer that route over potentially break the API.

Mar 10 '21 10:03 zeenix

In GitLab by @ids1024 on Mar 10, 2021, 15:36

I think this would only break code that depends on OwnedValue and Value<'static> being different types.

there is a way to treat From<T> for T as TryFrom<T> for T if there is a way to convert Infallible to zvariant::Error.

Great! Lets see how that works out. Though it may be desirable to make both changes. But I guess OwnedValue being a newtype may largely not be an issue if it can implement conversion types and Deref appropriately.

Mar 10 '21 14:03 zeenix

I think this would only break code that depends on OwnedValue and Value<'static> being different types.

Hmm.. is that practically possible? I think except for the special types (!Unpin for example), Rust doesn't have the notion of requiring something to be not of a specific type. I could be wrong though.

Mar 10 '21 15:03 zeenix

In GitLab by @ids1024 on Mar 10, 2021, 16:47

What comes to mind is something like this:

trait Foo;

impl Foo for OwnedValue {}
impl<'a> Foo for Value<'a> {}

These will conflict and fail to compile if the types are the same. But I assume code using zbus/zvariant isn't likely do be doing things like that.

Mar 10 '21 15:03 zeenix

What comes to mind is something like this:

Right, that's true.

But I assume code using zbus/zvariant isn't likely do be doing things like that.

Yeah, me too. If they are, I wouldn't feel too bad about breaking this and the fix is trivial. But anyway, let's see if my Fallible suggestion can be implemented, first.

Mar 10 '21 16:03 zeenix

I've a WIP branch that I really should finish up very soon. Perhaps that conversion also helps here? If it can, I'd prefer that route over potentially break the API.

@ids1024 Just fyi that work got merged already (even released) through !274.

Mar 15 '21 11:03 zeenix

Hmm.. I tried:

impl<'a, T, E> TryFrom<OwnedValue> for T
where
    T: TryFrom<Value<'a>, Error = E>,
    E: Into<crate::Error>,

And I get unconstrained type parameter for E error. So we'll probably need to try the non-ideal way. :(

Mar 15 '21 21:03 zeenix

In GitLab by @ids1024 on Mar 15, 2021, 23:01

It seems that should be:

impl<'a, T> TryFrom<OwnedValue> for T
where
    T: TryFrom<Value<'a>>,
    T::Error: Into<crate::Error>

But that fails with "type parameter T must be covered by another type when it appears before the first local type" like I was getting earlier.

This sort of blanket implementation isn't allowed by the orphan rule.

Mar 15 '21 22:03 zeenix

But I assume code using zbus/zvariant isn't likely do be doing things like that.

Yeah, me too. If they are, I wouldn't feel too bad about breaking this and the fix is trivial. But anyway, let's see if my Fallible suggestion can be implemented, first.

@ids1024 Btw, that work was merged a while ago (f7853dda59f8864eab70729d2ca24ab0b1a666cb). Maybe that helps. If it doesn't, I'm also planning an API break (zvariant 3.0) soon so we can easily convert OwnedValue to a type alias like you suggested.

Jul 11 '21 19:07 zeenix

Although we don't know for sure if this will require an API break but I'm adding the tag anyway since it's likely.

Jul 11 '21 19:07 zeenix

In GitLab by @ids1024 on Aug 25, 2021, 23:26

mentioned in commit ids1024/zbus@680a66d55242c8261f28d20496a7b8e5181554f3

Aug 25 '21 21:08 zeenix