Make new unchecked unsafe

[Toorero]

I was just reading your code and was wondering what's up with the Day::__new_unchecked function.

Wouldn't it make more sense to "really" expose it publicly and make it unsafe, so that the caller has to take care to uphold safety?

diff --git a/src/template/day.rs b/src/template/day.rs
--- a/src/template/day.rs
+++ b/src/template/day.rs
@@ -31,9 +31,12 @@ impl Day {
         Some(Self(day))
     }
 
-    // Not part of the public API
-    #[doc(hidden)]
-    pub const fn __new_unchecked(day: u8) -> Self {
+    /// Creates a new day without checking if it's in the valid range.
+    ///
+    /// # Safety
+    ///
+    /// Ensure that day is in range `1..=24`.
+    pub const unsafe fn new_unchecked(day: u8) -> Self {
         Self(day)
     }
 
@@ -146,7 +149,7 @@ macro_rules! day {
                 "`, expecting a value between 1 and 25"
             ),
         );
-        $crate::template::Day::__new_unchecked($day)
+        unsafe { $crate::template::Day::new_unchecked($day) }
     }};
 }

[fspoettel]

I'm not 100% sure anymore, maybe @tguichaoua remembers why this was added in #35?

[tguichaoua]

Back when it was added it was impossible to do comparison in a const function, so to create a Day in a const context I implemeted it with a day! macros that check if the value is a valid day then pass it to new_unchecked. Since it's now possible to do comparison in a const function, we can remove new_unchecked and make new const and day! may be changed to something like this (note that Option::except isn't const):

macro_rules! day {
    ($day:expr) => {
        const {
            let day: u8 = $day;
            match Day::new(day) {
                Some(day) => day,
                None => panic!("invalid day number `{day}`, expecting a value between 1 and 25"),
            }
        }
    };
}

Also unsafe wouldn't be suitable here as Day didn't hold any soundness invariant.

[Toorero]

Interesting, thanks for the input.