Fix bugs in BoardVisitor & GroupVisitor. Add Tests. (#234)

Author: tommadams

cc/position.h

@@ -53,8 +53,9 @@ class BoardVisitor {
   // Starts a new visit around the board.
   void Begin() {
     MG_DCHECK(Done());
-    if (epoch_++ == 0) {
+    if (++epoch_ == 0) {
       memset(visited_.data(), 0, sizeof(visited_));
+      epoch_ = 1;
     }
   }
 
@@ -83,7 +84,10 @@ class BoardVisitor {
  private:
   inline_vector<Coord, kN * kN> stack_;
   std::array<uint8_t, kN * kN> visited_;
-  uint8_t epoch_ = 0;
+
+  // Initializing to 0xff means the visited_ array will get initialized on the
+  // first call to Begin().
+  uint8_t epoch_ = 0xff;
 };
 
 // GroupVisitor simply keeps track of which groups have been visited since the
@@ -94,8 +98,9 @@ class GroupVisitor {
   GroupVisitor() = default;
 
   void Begin() {
-    if (epoch_++ == 0) {
+    if (++epoch_ == 0) {
       memset(visited_.data(), 0, sizeof(visited_));
+      epoch_ = 1;
     }
   }
 
@@ -108,8 +113,11 @@ class GroupVisitor {
   }
 
  private:
-  uint8_t epoch_ = 0;
   std::array<uint8_t, Group::kMaxNumGroups> visited_;
+
+  // Initializing to 0xff means the visited_ array will get initialized on the
+  // first call to Begin().
+  uint8_t epoch_ = 0xff;
 };
 
 // Position represents a single board position.

cc/position_test.cc

@@ -27,6 +27,35 @@
 namespace minigo {
 namespace {
 
+TEST(BoardVisitorTest, TestEpochRollover) {
+  for (int j = 0; j <= 256; ++j) {
+    BoardVisitor bv;
+    for (int i = 0; i < j; ++i) {
+      bv.Begin();
+    }
+    for (int i = 0; i < kN * kN; ++i) {
+      auto c = Coord(i);
+      ASSERT_TRUE(bv.Visit(c));
+      ASSERT_EQ(c, bv.Next());
+      ASSERT_FALSE(bv.Visit(c));
+    }
+  }
+}
+
+TEST(GroupVisitorTest, TestEpochRollover) {
+  for (int j = 0; j <= 256; ++j) {
+    GroupVisitor gv;
+    for (int i = 0; i < j; ++i) {
+      gv.Begin();
+    }
+    for (int i = 0; i < Group::kMaxNumGroups; ++i) {
+      auto g = GroupId(i);
+      ASSERT_TRUE(gv.Visit(g));
+      ASSERT_FALSE(gv.Visit(g));
+    }
+  }
+}
+
 TEST(PositionTest, TestIsKoish) {
   auto board = TestablePosition(R"(
       .X.O.O.O.