Concerns about this crate

[Speak2Erase]

I'm currently looking around for a chess AI written in rust. I found Pleco, and it seems like a neat project, it's only one of a handful of crates I've seen that actually implement a chess AI.

However- I have a lot of concerns about this crate. For starters, the history. @chase-manning made a new version of the crate (Tanton, see #140), but then shortly after seems to be the owner of Pleco. All they've done is various updates to CI and fixing warnings, which is fair enough- but nothing really substantial.

Then, there's the code. Oh boy, there's a lot of undefined behavior and unsafe code. It's honestly quite daunting to look over it all- so specifically, I'll focus on src/movepick/mod.rs, as it's generally pretty simple. Let's look at the struct definition for MovePicker:

pub struct MovePicker {
    pick: Pick,
    board: *const Board,
    moves: ScoringMoveList,
    depth: i16,
    ttm: BitMove,
    killers: [BitMove; 2],
    cm: BitMove,
    recapture_sq: SQ,
    threshold: i32,
    main_hist: *const ButterflyHistory,
    capture_hist: *const CapturePieceToHistory,
    cont_hist: *const [*const PieceToHistory; 4],
    cur_ptr: *mut ScoringMove,
    end_ptr: *mut ScoringMove,
    end_bad_captures: *mut ScoringMove,
}

That's.. a lot of pointers. I can't comment on why they're there, but I can speculate. In MovePicker::new, there are a bunch of references passed as arguments:

    pub fn main_search(
        board: &Board,
        depth: i16,
        main_hist: &ButterflyHistory,
        // there are a lot more
    ) -> Self

..and all of them get stored as a pointer. If I had to guess, whoever wrote the code for MovePicker did not want to deal with lifetimes. Or is a C programmer who uses Rust as if it were C. At the least this function should be unsafe. What happens if someone passes a Board that goes out of scope? Then there's this (also in new):

        MovePicker {
            pick,
            board: &*board,
            // more fields
            recapture_sq: unsafe { mem::MaybeUninit::uninit().assume_init() },
            threshold: unsafe { mem::MaybeUninit::uninit().assume_init() },
            /// more fields
        }

Those mem::MaybeUninit::uninit().assume_init()? Insta UB. There's a reason the compiler doesn't let you use uninitialized values- when you create a value on the stack and don't initialize it, the function call will be at the deepest the stack has ever been at best, or point to some random garbage that used to be there. That's assuming that rustc will behave like a C compiler or not put variables into registers. In any case, these can create invalid values (imagine a bool that is not true or false) which can cause a lot of problems. Further down in the code, there's a similar thing with mem::zeroed!

No comments about why these may be safe by the way! All of this kind of code is repeated throughout the codebase- it really calls into question the mindset of the person who wrote it.