Fix move constructor and assignment for the Simplex tree

git-svn-id: svn+ssh://scm.gforge.inria.fr/svnroot/gudhi/branches/simplex_tree_fix_vincent@3831 636b058d-ea47-450e-bf9e-a15bfbe3eedb

Former-commit-id: ff520472b6a1b9f00d0688fbe7dc467af50e16fe

2 files changed, 94 insertions, 29 deletions

diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h
index 4759b352..8e0ddf75 100644
--- a/src/Simplex_tree/include/gudhi/Simplex_tree.h
+++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h
@@ -298,11 +298,11 @@ class Simplex_tree {
   /** \brief User-defined copy constructor reproduces the whole tree structure. */
   Simplex_tree(const Simplex_tree& simplex_source)
       : null_vertex_(simplex_source.null_vertex_),
-      root_(nullptr, null_vertex_ , simplex_source.root_.members_),
+      root_(nullptr, null_vertex_, simplex_source.root_.members_),
       filtration_vect_(),
       dimension_(simplex_source.dimension_) {
 #ifdef DEBUG_TRACES
-    std::cout << "copy constructor" << std::endl;
+    std::cout << "Simplex_tree copy constructor" << std::endl;
 #endif  // DEBUG_TRACES
     auto root_source = simplex_source.root_;
     rec_copy(&root_, &root_source);
@@ -323,17 +323,29 @@ class Simplex_tree {
     }
   }
 
-  /** \brief User-defined move constructor moves the whole tree structure. */
-  Simplex_tree(Simplex_tree && old)
-      : null_vertex_(std::move(old.null_vertex_)),
-      root_(std::move(old.root_)),
-      filtration_vect_(std::move(old.filtration_vect_)),
-      dimension_(std::move(old.dimension_)) {
+  /** \brief User-defined move constructor relocates the whole tree structure.
+   *  \exception std::invalid_argument In debug mode, if the simplex_source is invalid.
+   */
+  Simplex_tree(Simplex_tree && simplex_source)
+      : null_vertex_(std::move(simplex_source.null_vertex_)),
+      root_(std::move(simplex_source.root_)),
+      filtration_vect_(std::move(simplex_source.filtration_vect_)),
+      dimension_(std::move(simplex_source.dimension_)) {
 #ifdef DEBUG_TRACES
-    std::cout << "move constructor" << std::endl;
+    std::cout << "Simplex_tree move constructor" << std::endl;
 #endif  // DEBUG_TRACES
-    old.dimension_ = -1;
-    old.root_ = Siblings(nullptr, null_vertex_);
+    for (auto& map_el : root_.members()) {
+      if (map_el.second.children()->oncles() == &(simplex_source.root_))
+        // reset with the moved root_ pointer value
+        map_el.second.children()->oncles_ = &root_;
+      else
+        // if simplex is of dimension 0, oncles_ shall be nullptr
+        GUDHI_CHECK(map_el.second.children()->oncles_ == nullptr,
+                    std::invalid_argument("Simplex_tree move constructor from an invalid Simplex_tree"));
+    }
+    // just need to set dimension_ on source to make it available again
+    // (filtration_vect_ and members are already set from the move)
+    simplex_source.dimension_ = -1;
   }
 
   /** \brief Destructor; deallocates the whole tree structure. */
@@ -351,16 +363,24 @@ class Simplex_tree {
 #ifdef DEBUG_TRACES
     std::cout << "copy assignment" << std::endl;
 #endif  // DEBUG_TRACES
-    this->null_vertex_ = simplex_source.null_vertex_;
-    root_ = Siblings(nullptr, null_vertex_ , simplex_source.root_.members_);
-    this->filtration_vect_.clear();
-    this->dimension_ = simplex_source.dimension_;
+    null_vertex_ = simplex_source.null_vertex_;
+    filtration_vect_.clear();
+    dimension_ = simplex_source.dimension_;
     auto root_source = simplex_source.root_;
-    rec_copy(&(this->root_), &root_source);
+    // Here a copy will be done
+    root_ = Siblings(nullptr, null_vertex_);
+    root_.members() = Dictionary(boost::container::ordered_unique_range, root_source.members().begin(), root_source.members().end());
+    // Needs to reassign children
+    for (auto& map_el : root_.members()) {
+      map_el.second.assign_children(&root_);
+    }
+    rec_copy(&root_, &root_source);
     return *this;
   }
 
-  /** \brief User-defined move assignment reproduces the whole tree structure. */
+  /** \brief User-defined move assignment relocates the whole tree structure.
+   *  \exception std::invalid_argument In debug mode, if the simplex_source is invalid.
+   */
   Simplex_tree& operator=(Simplex_tree&& simplex_source)
   {
 #ifdef DEBUG_TRACES
@@ -372,6 +392,16 @@ class Simplex_tree {
       std::swap( root_, simplex_source.root_ );
       std::swap( filtration_vect_, simplex_source.filtration_vect_ );
       std::swap( dimension_, simplex_source.dimension_ );
+
+      for (auto& map_el : root_.members()) {
+        if (map_el.second.children()->oncles() == &(simplex_source.root_))
+          // reset with the moved root_ pointer value
+          map_el.second.children()->oncles_ = &root_;
+        else
+          // if simplex is of dimension 0, oncles_ shall be nullptr
+          GUDHI_CHECK(map_el.second.children()->oncles_ == nullptr,
+                      std::invalid_argument("Simplex_tree move constructor from an invalid Simplex_tree"));
+      }
     }
     return *this;
   }
diff --git a/src/Simplex_tree/test/simplex_tree_ctor_and_move_unit_test.cpp b/src/Simplex_tree/test/simplex_tree_ctor_and_move_unit_test.cpp
index 15eaf612..95e99afe 100644
--- a/src/Simplex_tree/test/simplex_tree_ctor_and_move_unit_test.cpp
+++ b/src/Simplex_tree/test/simplex_tree_ctor_and_move_unit_test.cpp
@@ -1,5 +1,6 @@
 #include <iostream>
 #include <vector>
+#include <string>
 
 #define BOOST_TEST_DYN_LINK
 #define BOOST_TEST_MODULE "simplex_tree_constructor_and_move"
@@ -14,38 +15,58 @@ using namespace Gudhi;
 
 typedef boost::mpl::list<Simplex_tree<>, Simplex_tree<Simplex_tree_options_fast_persistence>> list_of_tested_variants;
 
-template<class SimplicialComplex>
-SimplicialComplex move_it(SimplicialComplex sc) {
-  return sc;
-}
+template<typename Simplex_tree>
+void print_simplex_filtration(Simplex_tree& st, const std::string& msg) {
+  // Required before browsing through filtration values
+  st.initialize_filtration();
+
+  std::cout << "********************************************************************\n";
+  std::cout << "* " << msg << "\n";
+  std::cout << "* The complex contains " << st.num_simplices() << " simplices";
+  std::cout << "   - dimension " << st.dimension() << "\n";
+  std::cout << "* Iterator on Simplices in the filtration, with [filtration value]:\n";
+  for (auto f_simplex : st.filtration_simplex_range()) {
+    std::cout << "   "
+              << "[" << st.filtration(f_simplex) << "] ";
+    for (auto vertex : st.simplex_vertex_range(f_simplex)) std::cout << "(" << vertex << ")";
+    std::cout << std::endl;
+  }
 
+}
 
 BOOST_AUTO_TEST_CASE_TEMPLATE(simplex_copy_constructor, Simplex_tree, list_of_tested_variants) {
-  std::cout << "********************************************************************" << std::endl;
-  std::cout << "TEST OF COPY CONSTRUCTOR" << std::endl;
   Simplex_tree st;
 
   st.insert_simplex_and_subfaces({2, 1, 0}, 3.0);
   st.insert_simplex_and_subfaces({0, 1, 6, 7}, 4.0);
   st.insert_simplex_and_subfaces({3, 0}, 2.0);
   st.insert_simplex_and_subfaces({3, 4, 5}, 3.0);
+  st.insert_simplex_and_subfaces({8}, 1.0);
   /* Inserted simplex:        */
   /*    1   6                 */
   /*    o---o                 */
   /*   /X\7/                  */
-  /*  o---o---o---o           */
-  /*  2   0   3\X/4           */
+  /*  o---o---o---o   o       */
+  /*  2   0   3\X/4   8       */
   /*            o             */
   /*            5             */
   /*                          */
   /* In other words:          */
-  /*   A facet [2,1,0]        */
-  /*   An edge [0,3]          */
-  /*   A facet [3,4,5]        */
-  /*   A cell  [0,1,6,7]      */
+  /*   A facet  [2,1,0]       */
+  /*   An edge  [0,3]         */
+  /*   A facet  [3,4,5]       */
+  /*   A cell   [0,1,6,7]     */
+  /*   A vertex [8]           */
+
+  print_simplex_filtration(st, "Default Simplex_tree is initialized");
+
+  std::cout << "********************************************************************" << std::endl;
+  std::cout << "TEST OF COPY CONSTRUCTOR" << std::endl;
 
   Simplex_tree st1(st);
   Simplex_tree st2(st);
+  print_simplex_filtration(st1, "First copy constructor from the default Simplex_tree");
+  print_simplex_filtration(st2, "Second copy constructor from the default Simplex_tree");
   // Cross check
   BOOST_CHECK(st1 == st2);
   BOOST_CHECK(st == st2);
@@ -55,8 +76,11 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(simplex_copy_constructor, Simplex_tree, list_of_te
   std::cout << "TEST OF COPY ASSIGNMENT" << std::endl;
   Simplex_tree st3;
   st3 = st;
+  print_simplex_filtration(st3, "First copy assignment from the default Simplex_tree");
   Simplex_tree st4;
   st4 = st;
+  print_simplex_filtration(st4, "Second copy assignment from the default Simplex_tree");
+
   // Cross check
   BOOST_CHECK(st3 == st4);
   BOOST_CHECK(st == st4);
@@ -65,18 +89,29 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(simplex_copy_constructor, Simplex_tree, list_of_te
   std::cout << "********************************************************************" << std::endl;
   std::cout << "TEST OF MOVE CONSTRUCTOR" << std::endl;
   Simplex_tree st5(std::move(st1));
+  print_simplex_filtration(st5, "First move constructor from the default Simplex_tree");
+  print_simplex_filtration(st1, "First moved Simplex_tree shall be empty");
   Simplex_tree st6(std::move(st2));
+  print_simplex_filtration(st6, "Second move constructor from the default Simplex_tree");
+  print_simplex_filtration(st2, "Second moved Simplex_tree shall be empty");
 
   // Cross check
   BOOST_CHECK(st5 == st6);
   BOOST_CHECK(st == st6);
   BOOST_CHECK(st5 == st);
 
+  Simplex_tree empty_st;
+  BOOST_CHECK(st1 == st2);
+  BOOST_CHECK(empty_st == st2);
+  BOOST_CHECK(st1 == empty_st);
+
   std::cout << "********************************************************************" << std::endl;
   std::cout << "TEST OF MOVE ASSIGNMENT" << std::endl;
 
   // A swap is a copy ctor of a tmp value, then it uses move assignment
   std::swap(st3, st4);
+  print_simplex_filtration(st3, "First move assignment from the default Simplex_tree");
+  print_simplex_filtration(st4, "Second move assignment from the default Simplex_tree");
   BOOST_CHECK(st3 == st4);
   BOOST_CHECK(st == st4);
   BOOST_CHECK(st3 == st);