distil / diffus

Apache License 2.0
48 stars 6 forks source link

Strange error suggestions when deriving Diffus #104

Open taladar opened 2 years ago

taladar commented 2 years ago

I am trying to derive Diffus on the following data type

#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Diffus)]
  struct LDAPEntry {
      dn: String,
      attrs: HashMap<String, Vec<String>>,
      bin_attrs: HashMap<String, Vec<Vec<u8>>>,
  }

which gives me some strange suggestions in the error it produces (on Rust 1.57.0)

error[E0277]: the trait bound `std::string::String: Same` is not satisfied
  --> src/bin/sync_ldap_subtree.rs:58:56
   |
58 | #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Diffus)]
   |                                                        ^^^^^^ the trait `Same` is not implemented for `std::string::String`
   |
   = note: required because of the requirements on the impl of `Diffable<'_>` for `Vec<std::string::String>`
   = note: 1 redundant requirement hidden
   = note: required because of the requirements on the impl of `Diffable<'diffus_a>` for `HashMap<std::string::String, Vec<std::string::String>>`
   = note: required because it appears within the type `diffus::edit::Edit<'diffus_a, HashMap<std::string::String, Vec<std::string::String>>>`
   = note: only the last field of a struct may have a dynamically sized type
   = help: change the field's type to have a statically known size
   = note: this error originates in the derive macro `Diffus` (in Nightly builds, run with -Z macro-backtrace for more info)
help: borrowed types always have a statically known size
   |
58 | #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, &Diffus)]
   |                                                        +
help: the `Box` type always has a statically known size and allocates its contents in the heap
   |
58 | #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Box<Diffus>)]
   |                                                        ++++      +

error[E0277]: the trait bound `Vec<u8>: Same` is not satisfied
  --> src/bin/sync_ldap_subtree.rs:58:56
   |
58 | #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Diffus)]
   |                                                        ^^^^^^ the trait `Same` is not implemented for `Vec<u8>`
   |
   = note: required because of the requirements on the impl of `Diffable<'_>` for `Vec<Vec<u8>>`
   = note: 1 redundant requirement hidden
   = note: required because of the requirements on the impl of `Diffable<'diffus_a>` for `HashMap<std::string::String, Vec<Vec<u8>>>`
note: required by a bound in `diffus::edit::Edit`
  --> /home/taladar/.cargo/registry/src/github.com-1ecc6299db9ec823/diffus-0.10.0/src/edit/mod.rs:11:22
   |
11 | pub enum Edit<'a, T: Diffable<'a> + ?Sized> {
   |                      ^^^^^^^^^^^^ required by this bound in `diffus::edit::Edit`
   = note: this error originates in the derive macro `Diffus` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.

I am reasonably sure that neither &Diffus nor Box make sense in the derive macro parameters?

Changing it to &Diffus experimentally yields

error: expected unsuffixed literal or identifier, found `&`
  --> src/bin/sync_ldap_subtree.rs:58:56
   |
58 | #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, &Diffus)]
   |                                                        ^
bergmark commented 2 years ago
   = note: required because of the requirements on the impl of `Diffable<'_>` for `Vec<std::string::String>`

I don't really know this lib but looking at the trait impls we have

impl<'a, T: Same + Diffable<'a> + 'a> Diffable<'a> for Vec<T>

so T = String needs to implement Same and Diffable. Same for String is not implemented.

Not sure if there is a reason for not implementing it or if it can just be added to https://github.com/distil/diffus/blob/master/diffus/src/same.rs#L25

taladar commented 2 years ago

Indeed, Same for Vec\<u8> is probably also pretty useful in general but my ticket was mostly about the strange suggestions in the derive macro roughly in the middle of the compiler output.

bergmark commented 2 years ago

Do you know if anything can be done about it in this lib? I guessed that this was just rustc trying its darndest to help out

taladar commented 2 years ago

I am not familiar enough with the writing of macros to know what exactly causes it but presumably the suggestion is propagated from some type in the generated code to that place. Something like cargo-expand might be useful to get some more information on the cause and possibly minimize the example to report it to the compiler issue tracker if necessary.

Jim-Holmstroem commented 2 years ago

Great find @taladar !

Thought about it for a few minutes I don't see any reason for not having it as a default implementation.

str does already have a default Same using Eq, so String should too to be "least surprising"

If you need an alternative "sameness" you'll have to wrap it in it's own type.

Added a test and fixed it: https://github.com/distil/diffus/pull/105