From 7468976db544ec4672acc26fffc1ec78244d1077 Mon Sep 17 00:00:00 2001 From: Rak Laptudirm Date: Tue, 30 Jul 2024 02:01:51 +0530 Subject: [PATCH] chore: less unsafe, more unsafe --- games/src/interface/bitboard.rs | 26 ++++++++++++++++------ games/src/interface/mod.rs | 9 ++++++-- games/src/interface/piece.rs | 2 +- games/src/interface/square.rs | 38 +++++++++++++++++++++++---------- 4 files changed, 54 insertions(+), 21 deletions(-) diff --git a/games/src/interface/bitboard.rs b/games/src/interface/bitboard.rs index 70072f2..504a33e 100644 --- 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 { let lsb = self.lsb(); let copy = >::into(*self); *self = >::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 { let msb = self.msb(); - *self = Self::from(>::into(*self) ^ Self::from(msb).into()); + if let Some(msb) = msb { + *self = Self::from(>::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 { + 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 { + 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 index 0f954b2..8364613 100644 --- a/games/src/interface/mod.rs +++ b/games/src/interface/mod.rs @@ -22,8 +22,13 @@ pub trait RepresentableType>: const N: usize; /// unsafe_from unsafely converts the given number into Self. - fn unsafe_from>(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>(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 index d1330bf..b4bbe14 100644 --- 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 index 10a1f1e..3cfbacc 100644 --- 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 { + 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 { + 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 { + 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 { + if self.file().into() as usize == 0 { + None + } else { + Some(unsafe { Self::unsafe_from(self.into() - 1) }) + } } }