Repository: chronotope/chrono (language: rust)

Issue:
0.4.37 semver-incompatibly removes trait bounds on `DateTime`
Hello,

The following code compile in 0.4.35 but breaks in 0.4.37:

```rust
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 https://github.com/chronotope/chrono/pull/1492


Discussion / hints:
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.
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.
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.
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. 
Reverting the change to `Copy` is easy, shall I just make a PR?
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:

```rust
pub struct DateTime<Tz: TimeZone> {
    datetime: NaiveDateTime,
    offset: Tz::Offset,
}
```

the `Tz: Copy` shouldn't be necessary.
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`.
> The manual Copy implementation that was deleted in https://github.com/chronotope/chrono/pull/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.
Sorry, I'm not using the right term. Meant 'correct'.

Fix patch:
diff --git a/CITATION.cff b/CITATION.cff
index 0e44adfac8..96cbf16cdd 100644
--- a/CITATION.cff
+++ b/CITATION.cff
@@ -3,8 +3,8 @@ cff-version: 1.2.0
 message: Please cite this crate using these information.
 
 # Version information.
-date-released: 2024-03-27
-version: 0.4.37
+date-released: 2024-04-15
+version: 0.4.38
 
 # Project information.
 abstract: Date and time library for Rust
diff --git a/Cargo.toml b/Cargo.toml
index 4a555d21ac..e84c7eb8a7 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "chrono"
-version = "0.4.37"
+version = "0.4.38"
 description = "Date and time library for Rust"
 homepage = "https://github.com/chronotope/chrono"
 documentation = "https://docs.rs/chrono/"
diff --git a/src/datetime/mod.rs b/src/datetime/mod.rs
index 366bbe6ad7..28cc64ab79 100644
--- a/src/datetime/mod.rs
+++ b/src/datetime/mod.rs
@@ -46,7 +46,7 @@ mod tests;
 /// There are some constructors implemented here (the `from_*` methods), but
 /// the general-purpose constructors are all via the methods on the
 /// [`TimeZone`](./offset/trait.TimeZone.html) implementations.
-#[derive(Copy, Clone)]
+#[derive(Clone)]
 #[cfg_attr(
     any(feature = "rkyv", feature = "rkyv-16", feature = "rkyv-32", feature = "rkyv-64"),
     derive(Archive, Deserialize, Serialize),
@@ -1408,6 +1408,15 @@ impl<Tz: TimeZone> Timelike for DateTime<Tz> {
     }
 }
 
+// We don't store a field with the `Tz` type, so it doesn't need to influence whether `DateTime` can
+// be `Copy`. Implement it manually if the two types we do have are `Copy`.
+impl<Tz: TimeZone> Copy for DateTime<Tz>
+where
+    <Tz as TimeZone>::Offset: Copy,
+    NaiveDateTime: Copy,
+{
+}
+
 impl<Tz: TimeZone, Tz2: TimeZone> PartialEq<DateTime<Tz2>> for DateTime<Tz> {
     fn eq(&self, other: &DateTime<Tz2>) -> bool {
         self.datetime == other.datetime


Test patch:
diff --git a/src/datetime/tests.rs b/src/datetime/tests.rs
index 126e1d34d1..c890863d8b 100644
--- a/src/datetime/tests.rs
+++ b/src/datetime/tests.rs
@@ -1,8 +1,8 @@
 use super::DateTime;
 use crate::naive::{NaiveDate, NaiveTime};
-use crate::offset::{FixedOffset, TimeZone, Utc};
 #[cfg(feature = "clock")]
-use crate::offset::{Local, Offset};
+use crate::offset::Local;
+use crate::offset::{FixedOffset, Offset, TimeZone, Utc};
 use crate::{Datelike, Days, MappedLocalTime, Months, NaiveDateTime, TimeDelta, Timelike, Weekday};
 
 #[derive(Clone)]
@@ -1318,9 +1318,44 @@ fn test_datetime_format_with_local() {
 
 #[test]
 fn test_datetime_is_send_and_copy() {
+    #[derive(Clone)]
+    struct Tz {
+        _not_send: *const i32,
+    }
+    impl TimeZone for Tz {
+        type Offset = Off;
+
+        fn from_offset(_: &Self::Offset) -> Self {
+            unimplemented!()
+        }
+        fn offset_from_local_date(&self, _: &NaiveDate) -> crate::MappedLocalTime<Self::Offset> {
+            unimplemented!()
+        }
+        fn offset_from_local_datetime(
+            &self,
+            _: &NaiveDateTime,
+        ) -> crate::MappedLocalTime<Self::Offset> {
+            unimplemented!()
+        }
+        fn offset_from_utc_date(&self, _: &NaiveDate) -> Self::Offset {
+            unimplemented!()
+        }
+        fn offset_from_utc_datetime(&self, _: &NaiveDateTime) -> Self::Offset {
+            unimplemented!()
+        }
+    }
+
+    #[derive(Copy, Clone, Debug)]
+    struct Off(());
+    impl Offset for Off {
+        fn fix(&self) -> FixedOffset {
+            unimplemented!()
+        }
+    }
+
     fn _assert_send_copy<T: Send + Copy>() {}
-    // UTC is known to be `Send + Copy`.
-    _assert_send_copy::<DateTime<Utc>>();
+    // `DateTime` is `Send + Copy` if the offset is.
+    _assert_send_copy::<DateTime<Tz>>();
 }
 
 #[test]