niklasf / shakmaty

A Rust library for chess and chess variant rules and operations
https://docs.rs/shakmaty
GNU General Public License v3.0
210 stars 41 forks source link

feat!: `const`-ify all the things! #62

Closed BastiDood closed 2 years ago

BastiDood commented 2 years ago

... well, okay, maybe not all the things. Methods that take in &mut self are not yet eligible for const-ification. This PR does manage to const-ify many things, though. 🥳

const-ified Methods

For most methods, adding a const declaration was trivial since the compiler already supported the operations within. The rest required some extra massaging around const traits—or lack thereof.

In particular, many Bitboard operations may actually be const-ified if it weren't for non-const traits such as From, Into, Add, Sub, BitOr, BitAnd, etc. To resolve this, I've added *_const variants for these operations in the impl Bitboard itself.

The const-ification cascaded upwards to all dependent structs from that point forward. It was actually fascinating to see how most of the tests immediately finish.

impl Bitboard {
    // ... and more!

    pub fn is_disjoint<T: Into<Self>>(self, other: T) -> Self { ... }
    pub const fn is_disjoint_const(self, other: Self) -> Self { ... }

    pub fn is_subset<T: Into<Self>>(self, other: T) -> Self { ... }
    pub const fn is_subset_const(self, other: Self) -> Self { ... }

    pub fn is_superset<T: Into<Self>>(self, other: T) -> Self { ... }
    pub const fn is_superset_const(self, other: Self) -> Self { ... }

    // ... and more!
}

MSRV Bump

All the const-ification required one MSRV bump from 1.60 to 1.61. The specific feature for which const stabilization is required is the pointer::offset method.

Clippy: Prefer Self Alias

While I was fixing up the methods, I also decided to use the Self alias in various methods wherever possible (instead of explicitly naming the concrete Self). This should lessen unnecessary repetition of struct names.

niklasf commented 2 years ago

Thank you! Just want to let you know that it will be a few days before I get to review this.

niklasf commented 2 years ago

Looks great! const has come a long way. It's getting pretty close to being able to generate the static tables from build.rs at compile time.

(As a matter of taste, I made most Self aliases more explicit, again, at least where it's not significantly more verbose. Gives a bit of orientation. I'll reconsider if the clippy lint becomes warn by default.)

BastiDood commented 2 years ago

As a matter of taste, I made most Self aliases more explicit, again, at least where it's not significantly more verbose. Gives a bit of orientation. I'll reconsider if the clippy lint becomes warn by default.

Sure! I have no qualms against this. Thanks for the merge!