Relax some type requirements

[Mingun]

The implementation of many traits is far from perfect, in a set of places there are restrictions on types that are not required at all for work. Overall, they look more or less random, added only to shut up the compiler. This is an attempt to relax some of these restrictions

[nical]

I would prefer requiring the Copy trait instead of Clone to keep euclid's code simple unless there is a real need to support non-copyable types (big nums and such). Are you in that situation?

Overall though I agree that some of the trait bounds sort of evolved into what they are now and could be simplified.

[Mingun]

Are you in that situation?

No, I just notice that when had working on another PRs. I have tried to relax the restrictions wherever I can without affecting the source code or affecting it to a minimal manner. In addition, several other methods can be made not requiring Copy. These are conversion methods -- they currently take a constant reference. If they will move the value instead, Copy boundary might be removed. Such change will also not break the user code, as it currently it could work only with the Copy types, and for that types calling the method implicitly copies self, so that the original variable remains available

[Mingun]

Tests failed because array deconstruction from 14ee25f with non-Copy types is possible only from 1.36.0: https://rust.godbolt.org/z/BKEe4L

There are two possible options:

• do not change that part of the code
• bump minimum compiler version to 1.36.0

Version 1.36.0 was released 2019-07-04, ie. 9 months ago. I think, that this is enough time to migrate, so bump version is preferred way to deal with that.

What do you think?

[bors-servo]

☔ The latest upstream changes (presumably #403) made this pull request unmergeable. Please resolve the merge conflicts.

[nical]

Tests failed because array deconstruction from 14ee25f with non-Copy types is possible only from 1.36.0: https://rust.godbolt.org/z/BKEe4L

We try to avoid bumping the required compiler version to avoid causing trouble for people stuck with frozen versions of the compiler or standard library which affects game devs that must port the standard library to some consoles and maybe some embeded enthusiasts (see dicussion rust-gamedev/wg#28).

We probably don't need to be extremely rigid about it though. Perhaps 1.36 can be considered old enough. Thougths @kvark @kyren ?

[kvark]

It would be less of an evil to at least bump the requirement in a new breaking version of euclid. Just releasing a patch that does that is a bit unfair to the users. adding @Lokathor who raised the issue in the WG.

[Mingun]

OK, I remove that change on next rebase. As workaround maybe raw pointers can be used, but I not sure that I do all right: https://rust.godbolt.org/z/QeTv8X

#![allow(dead_code)]
use std::fmt::Debug;

fn work3<T: Debug>(array: [T; 3]) {
  let p = array.as_ptr();
  unsafe {
    work(
      p.offset(0).read(),
      p.offset(1).read(),
      p.offset(2).read()
    )
  }
  std::mem::forget(array);
}
fn work<T: Debug>(x: T, y: T, z: T) {
  println!("{:?}, {:?}, {:?}", x, y, z)
}
pub fn main() {
  let array = ["Some".to_string(), "unsafe".to_string(), "magic".to_string()];
  work3(array);
}

[Lokathor]

A minimum version bump should be a breaking change for sure.

Stable debian is 1.34, so if you're trying to support "old" compilers and you don't need the stable alloc crate then I'd try to stick to 1.34 or earlier.

[kyren]

We probably don't need to be extremely rigid about it though. Perhaps 1.36 can be considered old enough. Thougths @kvark @kyren ?

I am not currently in the situation where I need to freeze the version of rust that we're using, and probably won't be for a while. It's very nice of you to consider that use case though, and that may indeed be desirable for me someday. For right now, though, selfishly, I'm 100% fine with compiler requirement bumps and would rather not impede progress.

Not thinking about just me, or thinking about me in the more distant future, I think requiring a compiler at least 6 months (or a year?) old and bumping it with breaking version numbers is a reasonable trade off.

[nical]

Thanks for the input!

Let's stick to our current compiler requirement since there isn't a strong enough motivation at this point (especially with debian being at rustc 1.34).

As workaround maybe raw pointers can be used

I would prefer to avoid anything complicated or clever unless we have a real motivating use case. We can take your patch without the part that breaks on euclid's currently supported compiler version.

[Mingun]

@weiznich recalled another way to deconstruct the array and it works in 1.31 for non-Copy types: https://rust.godbolt.org/z/TZTGxc

[Mingun]

Ready to merge

[nical]

Looks good overall. You changed the formatting of some code to something that is quite different from the rest of euclid an the official rust style. Please change it back to what it was and we should be good to go as far as I am concerned.

[nical]

Thanks @Mingun

@bors-servo r+

[bors-servo]

📌 Commit 51ffeda has been approved by nical

[bors-servo]

⌛ Testing commit 51ffeda with merge 74b0c29...

[bors-servo]

☀️ Test successful - checks-travis
Approved by: nical
Pushing 74b0c29 to master...