Primitives as trait

[vmx]

As issue #19 outlines, it might make sense to use a different internal type for integer values. This commit makes all primitive types generic, so that you can choose your own.

Though you're discouraged from doing so. Usually using the IPLD type should be enough and leads to the best interoperability.

Thanks @Stebalien for the idea of this implementation.

Fixes #19.

[vmx]

The current version doesn't quite work as it's implementing the Serde traits for Ipld (aka IpldGeneric<DefaultPrimitives>) and not IpldGeneric<P>. I've looked into implementing it for that instead.

One problem are the visitors. Let's look at the integer one at

rust-ipld-core/src/serde/de.rs

Line 269 in 4ef5d73

Self::Integer(i128) => visitor.visit_i128(i128),

The problem here is that one would need to decide based on the generic (trait), which visitor to call. It doesn't make sense to call visit_i128() if your integer value is i64. A fix for that would be to put that definition also into the trait. That would then gain a function with a signature along the lines of:

fn visit_integer<V>(visitor: V, value: :Self::Integer);

The problem is the visitor, which is of type serde::de::Visitor<'de>. It needs the 'de lifetime. This means that this lifetime would need to be bubbled up even into the Ipld type. That's not desirable, as you only need this lifetime in case you use Serde, for the non-Serde case you wouldn't.

Unless I've missed something, I don't think that's a route we should follow.

[Stebalien]

'de doesn't need to be on the root type, just in the type constraints in the Deserializer implementation. E.g.:

impl<'de, P> de::Deserializer<'de> for IpldGeneric<P> {
    where
        P: Primitives,
        P::Integer: SomeTrait<'de>,
...

We can also define a special SerdePrimitives<'de> trait to make the constraints easier. The real question is how much these constraints will pollute everything else.

[vmx]

Thanks @Stebalien for the comment, it made me trying it again. I've pushed my WIP. This is all still kind of hacky, I'm still unsure if that's the way to go. Nonetheless, the deserializer seems to compile, the serializer not. That's where I'm stuck. I don't know how to workaround that PhatomData error:

$ cargo build --features serde
   Compiling ipld-core v0.4.1 (/home/vmx/src/pl/rust/ipld-core)
error[E0277]: the trait bound `fn(PhantomData<_>) -> serde::ser::Serializer<_> {serde::ser::Serializer::<_>}: serde::Serializer` is not satisfied
   --> src/serde/ser.rs:83:21
    |
83  |     value.serialize(Serializer)
    |           --------- ^^^^^^^^^^ the trait `serde::Serializer` is not implemented for fn item `fn(PhantomData<_>) -> serde::ser::Serializer<_> {serde::ser::Serializer::<_>}`
    |           |
    |           required by a bound introduced by this call
    |
    = help: the following other types implement trait `serde::Serializer`:
              &'a mut Formatter<'b>
              FlatMapSerializer<'a, M>
              serde::__private::ser::content::ContentSerializer<E>
              serde::ser::Serializer<P>
note: required by a bound in `serde::Serialize::serialize`
   --> /home/vmx/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.197/src/ser/mod.rs:251:12
    |
249 |     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
    |        --------- required by a bound in this associated function
250 |     where
251 |         S: Serializer;
    |            ^^^^^^^^^^ required by this bound in `Serialize::serialize`

If anyone has any ideas, any help would be appreciated.

[Stebalien]

Hm. I agree this is getting a bit ugly...

In terms of the error, Serializer both a type and a constructor... so rust is complaining about the constructor for the Serializer type. You need Serializer(PhantomData) (took way too long for me to realize this...)

[vmx]

You need Serializer(PhantomData) (took way too long for me to realize this...)

Thanks so much, this is what I didn't get, which makes sense now.

I'm getting closer to getting things compiled. Though there's one issue for strings and bytes. I've pushed a non-working version to show the problem. I somehow need bounds to get from &P::String to `&str.

I've tried something like for<'a> &'a str: From<&'a P::String>, which the compiler suggested, but that doesn't seem right.