Better document how Position c'tor works

Renamed a bit the functions to be more clear what
we actually are doing when we craete a Position object
and explained how StateInfo works.

No functional change.

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

diff --git a/src/position.cpp b/src/position.cpp
index 1350ddf97a5923e33c3f677bdef398df79154ea3..2b6e2b4c07a5e84640d839cd6aca37b77abf9f6d 100644 (file)
--- a/src/position.cpp
+++ b/src/position.cpp
@@ -76,15 +76,52 @@ CheckInfo::CheckInfo(const Position& pos) {
   checkSq[KING] = EmptyBoardBB;
 }
 
   checkSq[KING] = EmptyBoardBB;
 }
 

+
+/// Position c'tors. Here we always create a slower but safer copy of
+/// the original position or the FEN string, we want the new born Position
+/// object do not depend on any external data. Instead if we know what we
+/// are doing and we need speed we can create a position with default
+/// c'tor Position() and then use just fast_copy().
+
+Position::Position() {}
+

 Position::Position(const Position& pos) {
 Position::Position(const Position& pos) {

-  copy(pos);
+
+  fast_copy(pos);
+  detach(); // Always detach() in copy c'tor to avoid surprises

 }
 
 Position::Position(const string& fen) {
 }
 
 Position::Position(const string& fen) {

+

   from_fen(fen);
 }
 
 
   from_fen(fen);
 }
 
 

+/// Position::fast_copy() creates a partial copy of the given position,
+/// only data that changes with a do_move() / undo_move() cycle is copied,
+/// in particular for stateInfo are copied only the pointers, so that the
+/// actual data remains stored in the parent Position. This is not a problem
+/// if the parent Position is known not to be destroyed while we are still alive,
+/// as is the common case, see detach() otherwise.
+
+void Position::fast_copy(const Position& pos) {
+
+  memcpy(this, &pos, sizeof(Position));
+}
+
+
+/// Position::detach() copies the content of the current state and castling
+/// masks inside the position itself. This is needed when the st pointee could
+/// become stale, as example because the caller is about to going out of scope.
+
+void Position::detach() {
+
+  startState = *st;
+  st = &startState;
+  st->previous = NULL; // as a safe guard
+}
+
+

 /// Position::from_fen() initializes the position object with the given FEN
 /// string. This function is not very robust - make sure that input FENs are
 /// correct (this is assumed to be the responsibility of the GUI).
 /// Position::from_fen() initializes the position object with the given FEN
 /// string. This function is not very robust - make sure that input FENs are
 /// correct (this is assumed to be the responsibility of the GUI).


@@ -345,15 +382,6 @@ void Position::print(Move m) const {
 }
 
 
 }
 
 

-/// Position::copy() creates a copy of the input position.
-
-void Position::copy(const Position& pos) {
-
-  memcpy(this, &pos, sizeof(Position));
-  saveState(); // detach and copy state info
-}
-
-

 /// Position:hidden_checkers<>() returns a bitboard of all pinned (against the
 /// king) pieces for the given color and for the given pinner type. Or, when
 /// template parameter FindPinned is false, the pieces of the given color
 /// Position:hidden_checkers<>() returns a bitboard of all pinned (against the
 /// king) pieces for the given color and for the given pinner type. Or, when
 /// template parameter FindPinned is false, the pieces of the given color


@@ -1443,19 +1471,6 @@ int Position::see(Square from, Square to) const {
 }
 
 
 }
 
 

-/// Position::saveState() copies the content of the current state
-/// inside startState and makes st point to it. This is needed
-/// when the st pointee could become stale, as example because
-/// the caller is about to going out of scope.
-
-void Position::saveState() {
-
-  startState = *st;
-  st = &startState;
-  st->previous = NULL; // as a safe guard
-}
-
-

 /// Position::clear() erases the position object to a pristine state, with an
 /// empty board, white to move, and no castling rights.
 
 /// Position::clear() erases the position object to a pristine state, with an
 /// empty board, white to move, and no castling rights.
 

diff --git a/src/position.h b/src/position.h
index a1d53f042c8cb111124746c0b68919bb6285ddb5..e7dc3a802fca35ae821517815650dfb692db7eef 100644 (file)
--- a/src/position.h
+++ b/src/position.h
@@ -146,7 +146,7 @@ public:
   };
 
   // Constructors
   };
 
   // Constructors

-  Position() {}
+  Position();

   Position(const Position& pos);
   Position(const std::string& fen);
 
   Position(const Position& pos);
   Position(const std::string& fen);
 


@@ -156,7 +156,7 @@ public:
   void print(Move m = MOVE_NONE) const;
 
   // Copying
   void print(Move m = MOVE_NONE) const;
 
   // Copying

-  void copy(const Position& pos);
+  void fast_copy(const Position& pos);

   void flipped_copy(const Position& pos);
 
   // The piece on a given square
   void flipped_copy(const Position& pos);
 
   // The piece on a given square


@@ -234,7 +234,7 @@ public:
   bool square_is_weak(Square s, Color c) const;
 
   // Doing and undoing moves
   bool square_is_weak(Square s, Color c) const;
 
   // Doing and undoing moves

-  void saveState();
+  void detach();

   void do_move(Move m, StateInfo& st);
   void do_move(Move m, StateInfo& st, const CheckInfo& ci, bool moveIsCheck);
   void undo_move(Move m);
   void do_move(Move m, StateInfo& st);
   void do_move(Move m, StateInfo& st, const CheckInfo& ci, bool moveIsCheck);
   void undo_move(Move m);

diff --git a/src/search.cpp b/src/search.cpp
index f205774124063dca9e3fed76471d2e0083d89115..31de0b01e4979e5f5fd23adff739e000b7d4dcd3 100644 (file)
--- a/src/search.cpp
+++ b/src/search.cpp
@@ -2981,7 +2981,7 @@ namespace {
     splitPoint->mp = mp;
     splitPoint->moves = *moves;
     splitPoint->cpus = 1;
     splitPoint->mp = mp;
     splitPoint->moves = *moves;
     splitPoint->cpus = 1;

-    splitPoint->pos.copy(p);
+    splitPoint->pos.fast_copy(p);

     splitPoint->parentSstack = sstck;
     for (i = 0; i < ActiveThreads; i++)
         splitPoint->slaves[i] = 0;
     splitPoint->parentSstack = sstck;
     for (i = 0; i < ActiveThreads; i++)
         splitPoint->slaves[i] = 0;

diff --git a/src/uci.cpp b/src/uci.cpp
index f7ba2067ee509dbb3c5e735b49fe731712152faf..21bbb88472d4ce55b1fb5e56eba372a333e074c7 100644 (file)
--- a/src/uci.cpp
+++ b/src/uci.cpp
@@ -54,7 +54,7 @@ namespace {
   // The root position. This is set up when the user (or in practice, the GUI)
   // sends the "position" UCI command. The root position is sent to the think()
   // function when the program receives the "go" command.
   // The root position. This is set up when the user (or in practice, the GUI)
   // sends the "position" UCI command. The root position is sent to the think()
   // function when the program receives the "go" command.

-  Position RootPosition;
+  Position RootPosition(StartPosition);

 
   // Local functions
   bool handle_command(const string& command);
 
   // Local functions
   bool handle_command(const string& command);


@@ -210,7 +210,7 @@ namespace {
             }
             // Our StateInfo st is about going out of scope so copy
             // its content inside RootPosition before they disappear.
             }
             // Our StateInfo st is about going out of scope so copy
             // its content inside RootPosition before they disappear.

-            RootPosition.saveState();
+            RootPosition.detach();

         }
     }
   }
         }
     }
   }