Fix serde deserialisation

[mulimoen]

Fixes #231 by first copying from a Shadow of the same structure. This fails on invalid structures, and ensures only valid structures can be deserialized.

This unfortunately breaks backwards compatability as CsVecBase does not have a Deref restriction, unlike CsMatBase. We should consider merging this when we are preparing for 0.10.

[vbarrielle]

Looking good, though as you say we probably want to accumulate breaking changes for a 0.10

Maybe a test exercising the failure case of deserialization could be added (eg deserializing invalid indices).

[mulimoen]

I am working on adding some tests, but this makes inference fail in strange places iff the serde feature is activated, serde_json added to Cargo.toml and the below test is added to src/sparse/serde_traits.rs.

#[test]
fn valid_vector() {
    let vec = CsVecI::new(5, vec![0_u16, 3, 4], vec![1_u8, 5, 6]);
    // Commenting this line and stuff works again...
    let json = serde_json::to_string(&vec).unwrap();
}

This complains with

error[E0283]: type annotations needed for `ArrayBase<OwnedRepr<A>, Dim<[usize; 2]>>`
   --> src/sparse/binop.rs:503:31
    |
503 |         let expected_output = Array::eye(3);
    |             ---------------   ^^^^^^^^^^ cannot infer type for type parameter `A`
    |             |
    |             consider giving `expected_output` the explicit type `ArrayBase<OwnedRepr<A>, Dim<[usize; 2]>>`, where the type parameter `A` is specified
    |
    = note: cannot satisfy `_: Clone`
    = note: required by `ndarray::impl_constructors::<impl ArrayBase<S, Dim<[usize; 2]>>>::eye`

error[E0283]: type annotations needed for `ArrayBase<OwnedRepr<A>, Dim<[usize; 2]>>`
  --> src/sparse/to_dense.rs:48:19
   |
48 |         let res = Array::eye(3);
   |             ---   ^^^^^^^^^^ cannot infer type for type parameter `A`
   |             |
   |             consider giving `res` the explicit type `ArrayBase<OwnedRepr<A>, Dim<[usize; 2]>>`, where the type parameter `A` is specified
   |
   = note: cannot satisfy `_: Clone`
   = note: required by `ndarray::impl_constructors::<impl ArrayBase<S, Dim<[usize; 2]>>>::eye`

error[E0282]: type annotations needed
   --> src/sparse/triplet.rs:572:9
    |
572 |         assert_eq!(m.indices(), &[]);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type
    |
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0283`.
error: could not compile `sprs`

It is really strange as these errors do not happen in locations affected by the feature flag.

[vbarrielle]

That's very strange indeed. I'm wondering if that's a compiler bug...

[vbarrielle]

Well, the issue arises for all the compiler versions I've tried (1.42.0, 1.45.1, 1.46.0) so this probably is not a compiler bug. But I do not understand.

[ia0]

I managed to reduce the problem to the following, which shows that the problem is not in this crate, but either in the serde_json crate or in the compiler itself:

pub fn foo() {
    // Inference at MARK succeeds when commenting the following line.
    let json = serde_json::to_string(&()).unwrap();
}

pub fn bar() {
    let a: Vec<usize> = Vec::new();
    let b = Vec::new(); // MARK
    assert_eq!(a, b);
}

The problem is visible in the playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=c136cb6f7399b53905e1fab44506eb97

[mulimoen]

@ia0 That is a great minimal reproduction! It seems we also hit this issue when switching from serde_json to serde_yaml, but no error for toml.

[mulimoen]

Might be a variant of https://github.com/serde-rs/json/issues/651

[ia0]

Indeed, that looks exactly like the same issue.

[vbarrielle]

Thanks @ia0 for the minimal reproduction. Seeing the related issues, it looks like there's no alternative to annotating the types explicitly.

[mulimoen]

There are no problems when using this crate in another binary, even when using serde-json. I'll add a test through another workspace crate instead