Introduce isMoveLegal(board, move)

selftest/sofcheck/intf.h

@@ -64,7 +64,7 @@ inline Board boardFromFen(const char *fen) {
 
 inline MovePersistence tryMakeMove(Board &board, const Move &move) {
   auto p = SoFCore::moveMake(board, move);
-  if (!isMoveLegal(board)) {
+  if (!wasMoveLegal(board)) {
     SoFCore::moveUnmake(board, move, p);
     return std::nullopt;
   }

src/bot_api/clients/uci.cpp

@@ -555,16 +555,13 @@ PollResult UciServerConnector::processUciPosition(std::istream &tokens) {
   vector<Move> moves;
   while (tokens >> token) {
     const Move move = moveParse(token.c_str(), dstBoard);
-    if (!move.isWellFormed(dstBoard.side) || !isMoveValid(dstBoard, move)) {
-      logError(UCI_SERVER) << "Move \"" << token << "\" is invalid";
+    if (!move.isWellFormed(dstBoard.side) || !isMoveValid(dstBoard, move) ||
+        !isMoveLegal(dstBoard, move)) {
+      logError(UCI_SERVER) << "Move \"" << token << "\" is illegal";
       return PollResult::NoData;
     }
     moves.push_back(move);
     moveMake(dstBoard, move);
-    if (!isMoveLegal(dstBoard)) {
-      logError(UCI_SERVER) << "Move \"" << token << "\" is not legal";
-      return PollResult::NoData;
-    }
   }
 
   // Finally, after everything is parsed, just call client API

src/core/bitboard.h

@@ -22,6 +22,12 @@
 
 namespace SoFCore {
 
+// Empty bitboard
+constexpr bitboard_t BB_EMPTY = 0;
+
+// Full bitboard
+constexpr bitboard_t BB_FULL = 0xffffffffffffffff;
+
 // Bitboards for diagonals of type I. So, the bit `c` is set in the bitboard `BB_DIAG1[d]` iff
 // `coordDiag1(c) == d`
 constexpr bitboard_t BB_DIAG1[15] = {

src/core/board.cpp

@@ -424,7 +424,7 @@ ValidateResult Board::validate() {
   }
 
   // Check for `OpponentKingAttacked`
-  if (!isMoveLegal(*this)) {
+  if (!wasMoveLegal(*this)) {
     return ValidateResult::OpponentKingAttacked;
   }
 

src/core/movegen.cpp

@@ -76,7 +76,7 @@ bool isCheck(const Board &b) {
   return isCellAttacked(b, b.kingPos(c), invert(c));
 }
 
-bool isMoveLegal(const Board &b) {
+bool wasMoveLegal(const Board &b) {
   const Color c = b.side;
   return !isCellAttacked(b, b.kingPos(invert(c)), c);
 }
@@ -484,6 +484,59 @@ inline static bool isMoveValidImpl(const Board &b, const Move move) {
   return false;
 }
 
+template <Color C>
+inline static bool isAttackedMaskedImpl(const Board &b, const coord_t kingPos,
+                                        const bitboard_t bbAll, const bitboard_t bbOursMask) {
+  // Here, we use black attack map for white, as we need to trace the attack from destination piece,
+  // not from the source one
+  constexpr auto *pawnAttacks =
+      (C == Color::White) ? Private::BLACK_PAWN_ATTACKS : Private::WHITE_PAWN_ATTACKS;
+
+  // Check near attacks
+  if (((b.bbPieces[makeCell(C, Piece::Pawn)] & pawnAttacks[kingPos]) |
+       (b.bbPieces[makeCell(C, Piece::King)] & Private::KING_ATTACKS[kingPos]) |
+       (b.bbPieces[makeCell(C, Piece::Knight)] & Private::KNIGHT_ATTACKS[kingPos])) &
+      bbOursMask) {
+    return true;
+  }
+
+  // Check far attacks
+  return (Private::bishopAttackBitboard(bbAll, kingPos) & bbDiagPieces<C>(b) & bbOursMask) ||
+         (Private::rookAttackBitboard(bbAll, kingPos) & bbLinePieces<C>(b) & bbOursMask);
+}
+
+template <Color C>
+inline static bool isMoveLegalImpl(const Board &b, const Move move) {
+  if (SOF_UNLIKELY(move.kind == MoveKind::Null)) {
+    return !isCheck(b);
+  }
+
+  const coord_t src = move.src;
+  const coord_t dst = move.dst;
+  const bitboard_t bbSrc = coordToBitboard(src);
+  const bitboard_t bbDst = coordToBitboard(dst);
+  const cell_t srcCell = b.cells[src];
+
+  if (srcCell == makeCell(C, Piece::King)) {
+    return !isAttackedMaskedImpl<invert(C)>(b, dst, b.bbAll ^ bbSrc, BB_FULL);
+  }
+
+  const coord_t king = b.kingPos(C);
+  bitboard_t bbAll = (b.bbAll ^ bbSrc) | bbDst;
+  bitboard_t bbOursMask = ~bbDst;
+  if (move.kind == MoveKind::Enpassant) {
+    const bitboard_t bbTmp = advancePawnForward(invert(C), bbDst);
+    bbAll ^= bbTmp;
+    bbOursMask ^= bbTmp;
+  }
+  return !isAttackedMaskedImpl<invert(C)>(b, king, bbAll, bbOursMask);
+}
+
+bool isMoveLegal(const Board &b, Move move) {
+  return (b.side == Color::White) ? isMoveLegalImpl<Color::White>(b, move)
+                                  : isMoveLegalImpl<Color::Black>(b, move);
+}
+
 bool isMoveValid(const Board &b, const Move move) {
   return (b.side == Color::White) ? isMoveValidImpl<Color::White>(b, move)
                                   : isMoveValidImpl<Color::Black>(b, move);

src/core/movegen.h

@@ -29,7 +29,7 @@ namespace SoFCore {
 // Checks if the cell is attacked by any of the pieces of color `C`
 //
 // For optimization purposes, enpassant captures are not considered by this function, as the primary
-// purpose of this function is to check for king attacks (e. g. in `isMoveLegal()`)
+// purpose of this function is to check for king attacks (e. g. in `wasMoveLegal()`)
 template <Color C>
 bool isCellAttacked(const Board &b, coord_t coord);
 
@@ -41,7 +41,7 @@ inline bool isCellAttacked(const Board &b, const coord_t coord, const Color c) {
 // Returns the set of pieces of color `C` which attack the given cell
 //
 // For optimization purposes, enpassant captures are not considered by this function, as the primary
-// purpose of this function is to check for king attacks (e. g. in `isMoveLegal()`)
+// purpose of this function is to check for king attacks (e. g. in `wasMoveLegal()`)
 template <Color C>
 bitboard_t cellAttackers(const Board &b, coord_t coord);
 
@@ -55,7 +55,7 @@ inline bitboard_t cellAttackers(const Board &b, const coord_t coord, const Color
 //
 // So, the typical use of this function is to make a pseudo-legal move, then check whether it is
 // legal, and then unmake it if it's illegal.
-bool isMoveLegal(const Board &b);
+bool wasMoveLegal(const Board &b);
 
 // Returns `true` is the king of the moving side is currenly under check
 bool isCheck(const Board &b);
@@ -75,7 +75,7 @@ class MoveGen {
   // are valid according to the rules of chess. The moves are written into `list`, and the return
   // value indicates the number of moves generated.
   //
-  // To generate only legal moves you can use `isMoveLegal()` function
+  // To generate only legal moves you can use `isMoveLegal()` or `wasMoveLegal()` function
   size_t genAllMoves(Move *list) const;
   size_t genSimpleMoves(Move *list) const;
   size_t genSimpleMovesNoPromote(Move *list) const;
@@ -119,9 +119,18 @@ constexpr size_t BUFSZ_SIMPLE_PROMOTES = 32;
 // behavior is undefined. Null moves are considered invalid by this function, as they cannot be
 // returned by `genAllMoves()`.
 //
-// To check for legality you can use `isMoveLegal()` function
+// To check for legality you can use `isMoveLegal()` or `wasMoveLegal()` function
 bool isMoveValid(const Board &b, Move move);
 
+// Returns `true` if the move `move` is legal
+//
+// The move must be pseudo-legal (i.e. `move.isWellFormed(b.side) && isMoveValid(b, move)` must
+// return `true`), otherwise the behavior is undefined.
+//
+// As a special exception, null moves are considered legal by this function, but only if there is no
+// check.
+bool isMoveLegal(const Board &b, Move move);
+
 // Returns `true` if the move is capture
 inline constexpr bool isMoveCapture(const Board &b, const Move move) {
   return b.cells[move.dst] != EMPTY_CELL || move.kind == MoveKind::Enpassant;

src/core/test/selftest.cpp

@@ -78,15 +78,9 @@ static size_t filterLegalMoves(Board b, Move *moves, size_t origSize, const char
     if (!move.isWellFormed(b.side)) {
       panic("Move \"" + moveToStr(moves[i]) + "\" of type " + type + " is not well-formed");
     }
-    const Board saved = b;
-    MovePersistence p = moveMake(b, move);
-    if (isMoveLegal(b)) {
+    if (isMoveLegal(b, move)) {
       moves[newSize++] = move;
     }
-    moveUnmake(b, move, p);
-    if (!boardsBitCompare(b, saved)) {
-      panic("Board becomes different after making and unmaking move \"" + moveToStr(move) + "\"");
-    }
   }
   return newSize;
 }
@@ -147,10 +141,12 @@ void runSelfTest(Board b) {
   Move movesSeparate[1500];
   const size_t simpleCnt = genOnlyLegal(&MoveGen::genSimpleMovesNoPromote, gen, movesSeparate,
                                         "SIMPLE_NO_PROMOTE", BUFSZ_MOVES);
-  const size_t promoteCnt = genOnlyLegal(&MoveGen::genSimplePromotes, gen, movesSeparate + simpleCnt,
-                                         "PROMOTE", BUFSZ_SIMPLE_PROMOTES);
-  const size_t captureCnt = genOnlyLegal(
-      &MoveGen::genCaptures, gen, movesSeparate + simpleCnt + promoteCnt, "CAPTURE", BUFSZ_CAPTURES);
+  const size_t promoteCnt =
+      genOnlyLegal(&MoveGen::genSimplePromotes, gen, movesSeparate + simpleCnt, "PROMOTE",
+                   BUFSZ_SIMPLE_PROMOTES);
+  const size_t captureCnt =
+      genOnlyLegal(&MoveGen::genCaptures, gen, movesSeparate + simpleCnt + promoteCnt, "CAPTURE",
+                   BUFSZ_CAPTURES);
   if (simpleCnt + promoteCnt + captureCnt != moveCnt) {
     panic(
         "Moves generated by genAllMoves() differ from genSimpleMovesNoPromote() + "
@@ -192,7 +188,7 @@ void runSelfTest(Board b) {
   }
 
   // Check that a well-formed move is generated by `genAllMoves()` iff isMoveValid returns true
-  std::vector<Move> validMoves;
+  std::vector<Move> pseudoLegalMoves;
   const MoveKind kinds[] = {MoveKind::Null,
                             MoveKind::Simple,
                             MoveKind::PawnDoubleMove,
@@ -211,23 +207,33 @@ void runSelfTest(Board b) {
           continue;
         }
         if (isMoveValid(b, move)) {
-          validMoves.push_back(move);
+          pseudoLegalMoves.push_back(move);
         }
       }
     }
   }
-  validMoves.resize(filterLegalMoves(b, validMoves.data(), validMoves.size(), "VALID"));
-  std::sort(validMoves.begin(), validMoves.end(), cmpMoves);
-  if (!std::equal(validMoves.begin(), validMoves.end(), moves, moves + moveCnt)) {
+
+  auto legalMoves = pseudoLegalMoves;
+  legalMoves.resize(filterLegalMoves(b, legalMoves.data(), legalMoves.size(), "VALID"));
+  std::sort(legalMoves.begin(), legalMoves.end(), cmpMoves);
+  if (!std::equal(legalMoves.begin(), legalMoves.end(), moves, moves + moveCnt)) {
     panic("Valid move list and generated move list mismatch");
   }
 
-  // Check that board is valid after making moves
-  for (size_t i = 0; i < moveCnt; ++i) {
-    const Move move = moves[i];
+  // Check that `wasMoveLegal()` and `isMoveLegal()` are identical. Also check that board is valids
+  // after making moves.
+  for (const auto &move : pseudoLegalMoves) {
+    const bool isLegal1 = isMoveLegal(b, move);
     const Board saved = b;
     const MovePersistence p = moveMake(b, move);
-    testBoardValid(b);
+    const bool isLegal2 = wasMoveLegal(b);
+    if (isLegal1 != isLegal2) {
+      panic("Functions isMoveLegal() and wasMoveLegal() yield different result on move \"" +
+            moveToStr(move) + "\"");
+    }
+    if (isLegal2) {
+      testBoardValid(b);
+    }
     moveUnmake(b, move, p);
     if (!boardsBitCompare(b, saved)) {
       panic("Board becomes different after making and unmaking move \"" + moveToStr(move) + "\"");

src/gameset/reader.cpp

@@ -190,14 +190,12 @@ auto GameReader::tryReadCommand() -> std::optional<CommandResult> {
     for (const auto &srcMove : SoFUtil::split(bodyBegin)) {
       const Move move =
           SoFCore::moveParse(srcMove.data(), srcMove.data() + srcMove.size(), *lastBoard_);
-      if (!move.isWellFormed(lastBoard_->side) || !isMoveValid(*lastBoard_, move)) {
-        return error("Move #" + std::to_string(moves.size() + 1) + " is invalid");
+      if (!move.isWellFormed(lastBoard_->side) || !isMoveValid(*lastBoard_, move) ||
+          !isMoveLegal(*lastBoard_, move)) {
+        return error("Move #" + std::to_string(moves.size() + 1) + " is illegal");
       }
       moves.push_back(move);
       moveMake(*lastBoard_, move);
-      if (!isMoveLegal(*lastBoard_)) {
-        return error("Move #" + std::to_string(moves.size() + 1) + " is illegal");
-      }
       if (canCaptureBoards()) {
         capturedBoards_.push_back(*lastBoard_);
       }

src/search/private/job.cpp

@@ -342,7 +342,7 @@ score_t Searcher::quiescenseSearch(score_t alpha, const score_t beta, const Eval
     }
     DIAGNOSTIC(dgnMoves.add(move);)
     MoveMakeGuard guard(board_, move, tag);
-    if (!isMoveLegal(board_)) {
+    if (!wasMoveLegal(board_)) {
       continue;
     }
     const score_t score = -quiescenseSearch(-beta, -alpha, guard.tag());
@@ -512,7 +512,7 @@ score_t Searcher::doSearch(int32_t depth, const size_t idepth, score_t alpha, co
     DIAGNOSTIC(dgnMoves.add(move);)
     MoveMakeGuard guard(board_, move, tag);
     tt_.prefetch(board_.hash);
-    if (!isMoveLegal(board_)) {
+    if (!wasMoveLegal(board_)) {
       continue;
     }
     if constexpr (Node != NodeKind::Root) {

src/search/private/job_runner.cpp

@@ -91,13 +91,9 @@ static Move pickRandomMove(Board board) {
   SoFUtil::randomShuffle(moves, moves + count);
   for (size_t i = 0; i < count; ++i) {
     const Move move = moves[i];
-    const SoFCore::MovePersistence persistence = moveMake(board, move);
-    if (!isMoveLegal(board)) {
-      moveUnmake(board, move, persistence);
-      continue;
+    if (isMoveLegal(board, move)) {
+      return move;
     }
-    moveUnmake(board, move, persistence);
-    return move;
   }
   return Move::null();
 }