chore: less unsafe, more unsafe

raklaptudirm · Jul 29, 2024 · 7468976 · 7468976
1 parent 5ba18e9
commit 7468976
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 21 deletions.
diff --git a/games/src/interface/bitboard.rs b/games/src/interface/bitboard.rs
@@ -103,7 +103,7 @@ where
 
     /// pop_lsb pops the least significant Self::Square from the Self, i.e. it
     /// removes the lsb from the Self and returns its value.
-    fn pop_lsb(&mut self) -> Self::Square {
+    fn pop_lsb(&mut self) -> Option<Self::Square> {
         let lsb = self.lsb();
         let copy = <Self as Into<u64>>::into(*self);
         *self = <Self as From<u64>>::from(copy & (copy - 1));
@@ -113,21 +113,33 @@ where
 
     /// pop_msb pops the most significant Self::Square from the Self i.e. it
     /// removes the msb from the Self and returns its value.
-    fn pop_msb(&mut self) -> Self::Square {
+    fn pop_msb(&mut self) -> Option<Self::Square> {
         let msb = self.msb();
-        *self = Self::from(<Self as Into<u64>>::into(*self) ^ Self::from(msb).into());
+        if let Some(msb) = msb {
+            *self = Self::from(<Self as Into<u64>>::into(*self) ^ Self::from(msb).into());
+        }
 
         msb
     }
 
     /// get_lsb returns the least significant Self::Square from the Self.
-    fn lsb(self) -> Self::Square {
-        Self::Square::unsafe_from(self.into().trailing_zeros() as usize)
+    fn lsb(self) -> Option<Self::Square> {
+        let sq = self.into().trailing_zeros() as usize;
+        if sq < Self::Square::N {
+            Some(unsafe { Self::Square::unsafe_from(sq) })
+        } else {
+            None
+        }
     }
 
     /// get_msb returns the most significant Self::Square from the Self.
-    fn msb(self) -> Self::Square {
-        Self::Square::unsafe_from(63 - self.into().leading_zeros() as usize)
+    fn msb(self) -> Option<Self::Square> {
+        let sq = 63 - self.into().leading_zeros() as usize;
+        if sq < Self::Square::N {
+            Some(unsafe { Self::Square::unsafe_from(sq) })
+        } else {
+            None
+        }
     }
 
     fn singles(self) -> Self {

diff --git a/games/src/interface/mod.rs b/games/src/interface/mod.rs
@@ -22,8 +22,13 @@ pub trait RepresentableType<B: Into<usize>>:
     const N: usize;
 
     /// unsafe_from unsafely converts the given number into Self.
-    fn unsafe_from<T: Copy + Into<usize>>(number: T) -> Self {
+    /// # Safety
+    /// `unsafe_from` assumes that the target type can represent the provided
+    /// number, i.e. the number has a valid representation in the target type.
+    /// The function comes with a debug check for the same, and failure to
+    /// uphold this invariant will result in undefined behavior.
+    unsafe fn unsafe_from<T: Copy + Into<usize>>(number: T) -> Self {
         debug_assert!(number.into() < Self::N);
-        unsafe { std::mem::transmute_copy(&number) }
+        std::mem::transmute_copy(&number)
     }
 }
diff --git a/games/src/interface/piece.rs b/games/src/interface/piece.rs
@@ -16,7 +16,7 @@ where
 
     /// new creates a new ColoredPiece from the given Piece and Color.
     fn new(piece: Self::Piece, color: Self::Color) -> Self {
-        Self::unsafe_from(color.into() * Self::Piece::N as u8 + piece.into())
+        unsafe { Self::unsafe_from(color.into() * Self::Piece::N as u8 + piece.into()) }
     }
 
     /// piece returns the Piece part of the given ColoredPiece.

diff --git a/games/src/interface/square.rs b/games/src/interface/square.rs
@@ -9,30 +9,46 @@ where
     type Rank;
 
     fn new(file: Self::File, rank: Self::Rank) -> Self {
-        Self::unsafe_from(rank.into() * Self::File::N as u8 + file.into())
+        unsafe { Self::unsafe_from(rank.into() * Self::File::N as u8 + file.into()) }
     }
 
     fn file(self) -> Self::File {
-        Self::File::unsafe_from(self.into() % Self::File::N as u8)
+        unsafe { Self::File::unsafe_from(self.into() % Self::File::N as u8) }
     }
 
     fn rank(self) -> Self::Rank {
-        Self::Rank::unsafe_from(self.into() / Self::File::N as u8)
+        unsafe { Self::Rank::unsafe_from(self.into() / Self::File::N as u8) }
     }
 
-    fn north(self) -> Self {
-        Self::unsafe_from(self.into() + Self::File::N as u8)
+    fn north(self) -> Option<Self> {
+        if self.rank().into() as usize == Self::Rank::N - 1 {
+            None
+        } else {
+            Some(unsafe { Self::unsafe_from(self.into() + Self::File::N as u8) })
+        }
     }
 
-    fn south(self) -> Self {
-        Self::unsafe_from(self.into() - Self::File::N as u8)
+    fn south(self) -> Option<Self> {
+        if self.rank().into() as usize == 0 {
+            None
+        } else {
+            Some(unsafe { Self::unsafe_from(self.into() - Self::File::N as u8) })
+        }
     }
 
-    fn east(self) -> Self {
-        Self::unsafe_from(self.into() + 1)
+    fn east(self) -> Option<Self> {
+        if self.file().into() as usize == Self::File::N - 1 {
+            None
+        } else {
+            Some(unsafe { Self::unsafe_from(self.into() + 1) })
+        }
     }
 
-    fn west(self) -> Self {
-        Self::unsafe_from(self.into() - 1)
+    fn west(self) -> Option<Self> {
+        if self.file().into() as usize == 0 {
+            None
+        } else {
+            Some(unsafe { Self::unsafe_from(self.into() - 1) })
+        }
     }
 }