Don't need to assert for pos.is_ok() when position is constant

It's only necessary to do the checking at the end of every non-const
member (including the constructors and from_fen()) of class Position.
Once the post-condition of every modifier guarantees the class invariant,
we don't need to verify sanity of the position as preconditions for outside
callers such as movegen, search etc. For non-class types such as Move and
Square we still need to assert of course.

Suggested by Rein Halbersma.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

src/evaluate.cpp

@@ -275,7 +275,6 @@ Value do_evaluate(const Position& pos, Value& margin) {
   Value margins[2];
   Score score, mobilityWhite, mobilityBlack;
 
-  assert(pos.is_ok());
   assert(pos.thread() >= 0 && pos.thread() < MAX_THREADS);
   assert(!pos.in_check());
 

src/move.cpp

@@ -88,7 +88,6 @@ const string move_to_san(Position& pos, Move m) {
   if (m == MOVE_NULL)
       return "(null)";
 
-  assert(pos.is_ok());
   assert(move_is_ok(m));
 
   Bitboard attackers;

src/movegen.cpp

@@ -151,7 +151,6 @@ namespace {
 template<MoveType Type>
 MoveStack* generate(const Position& pos, MoveStack* mlist) {
 
-  assert(pos.is_ok());
   assert(!pos.in_check());
 
   Color us = pos.side_to_move();
@@ -202,7 +201,6 @@ template MoveStack* generate<MV_NON_EVASION>(const Position& pos, MoveStack* mli
 template<>
 MoveStack* generate<MV_NON_CAPTURE_CHECK>(const Position& pos, MoveStack* mlist) {
 
-  assert(pos.is_ok());
   assert(!pos.in_check());
 
   Bitboard b, dc;
@@ -243,7 +241,6 @@ MoveStack* generate<MV_NON_CAPTURE_CHECK>(const Position& pos, MoveStack* mlist)
 template<>
 MoveStack* generate<MV_EVASION>(const Position& pos, MoveStack* mlist) {
 
-  assert(pos.is_ok());
   assert(pos.in_check());
 
   Bitboard b, target;
@@ -315,8 +312,6 @@ MoveStack* generate<MV_EVASION>(const Position& pos, MoveStack* mlist) {
 template<>
 MoveStack* generate<MV_LEGAL>(const Position& pos, MoveStack* mlist) {
 
-  assert(pos.is_ok());
-
   MoveStack *last, *cur = mlist;
   Bitboard pinned = pos.pinned_pieces();
 

src/pawns.cpp

@@ -79,8 +79,6 @@ namespace {
 
 PawnInfo* PawnInfoTable::get_pawn_info(const Position& pos) const {
 
-  assert(pos.is_ok());
-
   Key key = pos.get_pawn_key();
   PawnInfo* pi = probe(key);
 

src/position.cpp

@@ -103,6 +103,8 @@ Position::Position(const Position& pos, int th) {
   detach(); // Always detach() in copy c'tor to avoid surprises
   threadID = th;
   nodes = 0;
+
+  assert(is_ok());
 }
 
 Position::Position(const string& fen, bool isChess960, int th) {
@@ -214,6 +216,8 @@ void Position::from_fen(const string& fenStr, bool isChess960) {
   st->value = compute_value();
   st->npMaterial[WHITE] = compute_non_pawn_material(WHITE);
   st->npMaterial[BLACK] = compute_non_pawn_material(BLACK);
+
+  assert(is_ok());
 }
 
 
@@ -492,7 +496,6 @@ bool Position::move_attacks_square(Move m, Square s) const {
 
 bool Position::pl_move_is_legal(Move m, Bitboard pinned) const {
 
-  assert(is_ok());
   assert(move_is_ok(m));
   assert(pinned == pinned_pieces());
 
@@ -559,8 +562,6 @@ bool Position::move_is_legal(const Move m) const {
 
 bool Position::move_is_pl(const Move m) const {
 
-  assert(is_ok());
-
   Color us = sideToMove;
   Color them = opposite_color(sideToMove);
   Square from = move_from(m);
@@ -683,7 +684,6 @@ bool Position::move_is_pl(const Move m) const {
 
 bool Position::move_gives_check(Move m, const CheckInfo& ci) const {
 
-  assert(is_ok());
   assert(move_is_ok(m));
   assert(ci.dcCandidates == discovered_check_candidates());
   assert(piece_color(piece_on(move_from(m))) == side_to_move());
@@ -794,6 +794,8 @@ void Position::do_setup_move(Move m) {
   // Our StateInfo newSt is about going out of scope so copy
   // its content before it disappears.
   detach();
+
+  assert(is_ok());
 }
 
 
@@ -809,7 +811,6 @@ void Position::do_move(Move m, StateInfo& newSt) {
 
 void Position::do_move(Move m, StateInfo& newSt, const CheckInfo& ci, bool moveIsCheck) {
 
-  assert(is_ok());
   assert(move_is_ok(m));
   assert(&newSt != st);
 
@@ -1181,7 +1182,6 @@ void Position::do_castle_move(Move m) {
 
 void Position::undo_move(Move m) {
 
-  assert(is_ok());
   assert(move_is_ok(m));
 
   sideToMove = opposite_color(sideToMove);
@@ -1355,7 +1355,6 @@ void Position::undo_castle_move(Move m) {
 
 void Position::do_null_move(StateInfo& backupSt) {
 
-  assert(is_ok());
   assert(!in_check());
 
   // Back up the information necessary to undo the null move to the supplied
@@ -1385,6 +1384,8 @@ void Position::do_null_move(StateInfo& backupSt) {
   st->rule50++;
   st->pliesFromNull = 0;
   st->value += (sideToMove == WHITE) ?  TempoValue : -TempoValue;
+
+  assert(is_ok());
 }
 
 
@@ -1392,7 +1393,6 @@ void Position::do_null_move(StateInfo& backupSt) {
 
 void Position::undo_null_move() {
 
-  assert(is_ok());
   assert(!in_check());
 
   // Restore information from the our backup StateInfo object
@@ -1407,6 +1407,8 @@ void Position::undo_null_move() {
   sideToMove = opposite_color(sideToMove);
   st->rule50--;
   st->gamePly--;
+
+  assert(is_ok());
 }
 
 
@@ -1757,8 +1759,6 @@ void Position::init() {
 
 void Position::flip() {
 
-  assert(is_ok());
-
   // Make a copy of current position before to start changing
   const Position pos(*this, threadID);
 

src/position.h

@@ -89,9 +89,10 @@ struct StateInfo {
 
 class Position {
 
-  // No default or copy c'tor allowed, default c'tor will not be generated
+  // No defaul, copy c'tor or assignment allowed, default c'tor will not be
-  // anyhow because of user-defined c'tors.
+  // generated anyhow because of user-defined c'tors.
   Position(const Position&);
+  Position& operator=(const Position&);
 
 public:
   Position(const Position& pos, int threadID);

src/uci.cpp

@@ -216,8 +216,6 @@ namespace {
     limits.time = time[pos.side_to_move()];
     limits.increment = inc[pos.side_to_move()];
 
-    assert(pos.is_ok());
-
     return think(pos, limits, searchMoves);
   }