0.4.37 semver-incompatibly removes trait bounds on DateTime

[little-dude]

Hello,

The following code compile in 0.4.35 but breaks in 0.4.37:

use chrono::TimeZone;
use chrono::DateTime;

#[derive(Clone, Debug)]
pub struct MyDateTime<T: TimeZone>(DateTime<T>);

impl<T: TimeZone> Copy for MyDateTime<T> where T::Offset: Copy {}

With 0.4.37:

error[E0204]: the trait `Copy` cannot be implemented for this type
 --> src/main.rs:7:28
  |
5 | pub struct MyDateTime<T: TimeZone>(chrono::DateTime<T>);
  |                                    ------------------- this field does not implement `Copy`
6 |
7 | impl<T: TimeZone> Copy for MyDateTime<T> where T::Offset: Copy {}
  |                            ^^^^^^^^^^^^^
  |
note: the `Copy` impl for `DateTime<T>` requires that `T: Copy`
 --> src/main.rs:5:36
  |
5 | pub struct MyDateTime<T: TimeZone>(chrono::DateTime<T>);
  |                                    ^^^^^^^^^^^^^^^^^^^
help: consider further restricting this bound
  |
7 | impl<T: TimeZone + Copy> Copy for MyDateTime<T> where T::Offset: Copy {}

This is probably due to #1492

[djc]

Sorry for the regression! We'll look into this. I don't think we're going to just revert the changes for #1492, but let's see what we can do.

[little-dude]

No worries, it's not a big deal for me. I just thought opening an issue might be good if other people run into this.

[djc]

Right -- I think I'm inclined to let this one go unless there are multiple reporters for whom it turns out to be a larger problem. As I understand it, it mostly requires people add bounds that they should have been adding anyway.

[pitdicker]

The manual Copy implementation that was deleted in #1492 was sound, it was only the comment that was incorrect. We had it because the type of Tz doesn't matter as it isn't a field of the DateTime struct.

For the Send implementation I'm not 100% sure it was sound.

[pitdicker]

Reverting the change to Copy is easy, shall I just make a PR?

[little-dude]

Yeah I agree that the previous Copy impl was better than the derived one. Wdyt @djc ?

As I understand it, it mostly requires people add bounds that they should have been adding anyway.

Yes, it requires having Tz: Copy, Tz::Offset: Copy, but since the DateTime struct looks like this:

pub struct DateTime<Tz: TimeZone> {
    datetime: NaiveDateTime,
    offset: Tz::Offset,
}

the Tz: Copy shouldn't be necessary.

[djc]

Alright, sounds good.

I think the Send impl was also sound but it was easy to accidentally make it unsound if we changed the layout of NaiveDateTime.

[little-dude]

The manual Copy implementation that was deleted in #1492 was sound, it was only the comment that was incorrect.

OOC how could a Copy impl be unsound? It isn't an unsafe trait, so I assumed it couldn't be unsound.

[pitdicker]

Sorry, I'm not using the right term. Meant 'correct'.

[little-dude]

Thanks for the quick response on this @pitdicker and @djc !