Always init time_stamp to -2 (#8714)

It is not clear to me that this is the right thing to do:
- it is done for some classes
- but in some test, a comment mention that it should be uninitialized
(the test works if init to -1 but not to 0).
- Base_time_stamper is doing an init to -2

Alpha_wrap_3/include/CGAL/Alpha_wrap_3/internal/Alpha_wrap_triangulation_cell_base_3.h

@@ -97,7 +97,7 @@ template <typename Cb>
 class Cell_base_with_timestamp
   : public Cb
 {
-  std::size_t time_stamp_;
+  std::size_t time_stamp_ = std::size_t(-2);
 
 public:
   using Has_timestamp = CGAL::Tag_true;
@@ -112,7 +112,7 @@ public:
 public:
   template <typename... Args>
   Cell_base_with_timestamp(const Args&... args)
-    : Cb(args...), time_stamp_(-1)
+    : Cb(args...)
   { }
 
   Cell_base_with_timestamp(const Cell_base_with_timestamp& other)

Mesh_2/include/CGAL/Delaunay_mesh_face_base_2.h

@@ -78,7 +78,7 @@ public:
 
   void set_time_stamp(const std::size_t& ts) { time_stamp_ = ts; }
 
-  std::size_t time_stamp_;
+  std::size_t time_stamp_ = std::size_t(-2);
 };
 
 } // namespace CGAL

Mesh_2/include/CGAL/Delaunay_mesh_vertex_base_2.h

@@ -67,7 +67,7 @@ public:
 
   void set_time_stamp(const std::size_t& ts) { time_stamp_ = ts; }
 
-  std::size_t time_stamp_;
+  std::size_t time_stamp_ = std::size_t(-2);
 #endif // CGAL_MESH_2_DEBUG_REFINEMENT_POINTS
 };
 

Mesh_3/include/CGAL/Compact_mesh_cell_base_3.h

@@ -644,7 +644,7 @@ private:
 #ifdef CGAL_INTRUSIVE_LIST
   Cell_handle next_intrusive_ = {}, previous_intrusive_ = {};
 #endif
-  std::size_t time_stamp_;
+  std::size_t time_stamp_ = std::size_t(-2);
 
   std::array<Index, 4> surface_center_index_table_ = {};
   /// Stores visited facets (4 first bits)

Mesh_3/include/CGAL/Mesh_cell_base_3.h

@@ -273,7 +273,7 @@ private:
 #ifdef CGAL_INTRUSIVE_LIST
   Cell_handle next_intrusive_, previous_intrusive_;
 #endif
-  std::size_t time_stamp_;
+  std::size_t time_stamp_ = std::size_t(-2);
 
 };  // end class Mesh_cell_base_3
 

Mesh_3/include/CGAL/Mesh_polyhedron_3.h

@@ -41,10 +41,9 @@ private:
   typedef CGAL::HalfedgeDS_vertex_base<Refs, Tag, Point> Pdv_base;
 
   Set_of_indices indices;
-  std::size_t time_stamp_;
+  std::size_t time_stamp_ = std::size_t(-2);
-
 public:
-  int nb_of_feature_edges;
+  int nb_of_feature_edges = 0;
 
   bool is_corner() const {
     return nb_of_feature_edges > 2;
@@ -85,8 +84,8 @@ public:
     return indices;
   }
 
-  Mesh_polyhedron_vertex() : Pdv_base(), time_stamp_(-1), nb_of_feature_edges(0) {}
+  Mesh_polyhedron_vertex() = default;
-  Mesh_polyhedron_vertex(const Point& p) : Pdv_base(p), time_stamp_(-1), nb_of_feature_edges(0) {}
+  Mesh_polyhedron_vertex(const Point& p) : Pdv_base(p) {}
 };
 
 template <class Refs, class Tprev, class Tvertex, class Tface>
@@ -95,7 +94,7 @@ public CGAL::HalfedgeDS_halfedge_base<Refs,Tprev,Tvertex,Tface>
 {
 private:
   bool feature_edge;
-  std::size_t time_stamp_;
+  std::size_t time_stamp_ = std::size_t(-2);
 
 public:
 
@@ -143,7 +142,7 @@ public CGAL::HalfedgeDS_face_base<Refs,T_,Pln_>
 {
 private:
   Patch_id_ patch_id_;
-  std::size_t time_stamp_;
+  std::size_t time_stamp_ = std::size_t(-2);
 
 public:
 

Mesh_3/include/CGAL/Mesh_vertex_base_3.h

@@ -250,7 +250,7 @@ private:
   Vertex_handle next_intrusive_;
   Vertex_handle previous_intrusive_;
 #endif
-  std::size_t time_stamp_;
+  std::size_t time_stamp_ = std::size_t(-2);
 public:
 
   friend std::istream& operator>>(std::istream &is, Mesh_vertex_3& v)

SMDS_3/include/CGAL/Compact_simplicial_mesh_cell_base_3.h

@@ -47,20 +47,7 @@ public:
 
 public:
   // Constructors
-  Compact_simplicial_mesh_cell_3()
+  Compact_simplicial_mesh_cell_3() = default;
-    : time_stamp_(std::size_t(-1))
-  {}
-
-  Compact_simplicial_mesh_cell_3(const Compact_simplicial_mesh_cell_3& rhs)
-    : N(rhs.N)
-    , V(rhs.V)
-    , time_stamp_(rhs.time_stamp_)
-    , subdomain_index_(rhs.subdomain_index_)
-  {
-    for(int i=0; i <4; i++){
-      surface_index_table_[i] = rhs.surface_index_table_[i];
-    }
-  }
 
   Compact_simplicial_mesh_cell_3(Vertex_handle v0,
                                  Vertex_handle v1,
@@ -279,7 +266,7 @@ private:
   std::array<Cell_handle, 4> N;
   std::array<Vertex_handle, 4> V;
 
-  std::size_t time_stamp_;
+  std::size_t time_stamp_ =  std::size_t(-2);
 
   // The index of the cell of the input complex that contains me
   Subdomain_index subdomain_index_ = {};

SMDS_3/include/CGAL/Simplicial_mesh_cell_base_3.h

@@ -96,9 +96,7 @@ public:
   };
 
 public:
-  Simplicial_mesh_cell_base_3()
+  Simplicial_mesh_cell_base_3() = default;
-    : time_stamp_(std::size_t(-1))
-  {}
 
   Simplicial_mesh_cell_base_3(const Simplicial_mesh_cell_base_3& rhs)
     : Cb(static_cast<const Cb&>(rhs)),
@@ -191,7 +189,7 @@ private:
   /// Stores surface_index for each facet of the cell
   std::array<Surface_patch_index, 4> surface_index_table_ = {};
 
-  std::size_t time_stamp_;
+  std::size_t time_stamp_ = std::size_t(-2);
 
   // The index of the cell of the input complex that contains me
   Subdomain_index subdomain_index_ = {};

SMDS_3/include/CGAL/Simplicial_mesh_vertex_base_3.h

@@ -130,7 +130,6 @@ public:
     , index_()
     , dimension_(-1)
     , cache_validity(false)
-    , time_stamp_(std::size_t(-1))
   {}
 
   // Default copy constructor and assignment operator are ok
@@ -218,7 +217,7 @@ private:
   // that contains me. Negative values are a marker for special vertices.
   short dimension_;
   bool cache_validity;
-  std::size_t time_stamp_;
+  std::size_t time_stamp_ = std::size_t(-2);
 
 public:
   friend std::istream& operator>>(std::istream& is,

STL_Extension/include/CGAL/Compact_container.h

@@ -220,6 +220,10 @@ class Compact_container
                              CGAL_INCREMENT_COMPACT_CONTAINER_BLOCK_SIZE>
           >::type                                   Increment_policy;
   typedef TimeStamper_                              Ts;
+
+  template <typename U> using EraseCounterStrategy =
+      internal::Erase_counter_strategy<internal::has_increment_erase_counter<U>::value>;
+
   typedef Compact_container <T, Al, Ip, Ts>         Self;
   typedef Compact_container_traits <T>              Traits;
 public:
@@ -354,7 +358,7 @@ public:
     return all_items[block_number].first[index_in_block];
   }
 
-  friend void swap(Compact_container& a, Compact_container b) {
+  friend void swap(Compact_container& a, Compact_container b) noexcept {
     a.swap(b);
   }
 
@@ -390,22 +394,16 @@ public:
   // (just forward the arguments to the constructor, to optimize a copy).
   template < typename... Args >
   iterator
-  emplace(const Args&... args)
+  emplace(Args&&... args)
   {
     if (free_list == nullptr)
       allocate_new_block();
 
     pointer ret = free_list;
     free_list = clean_pointee(ret);
-    std::size_t ts;
+    const auto ts = Time_stamper::time_stamp(ret);
-    CGAL_USE(ts);
+    new (ret) value_type(std::forward<Args>(args)...);
-    if constexpr (Time_stamper::has_timestamp) {
+    Time_stamper::restore_timestamp(ret, ts);
-      ts = ret->time_stamp();
-    }
-    new (ret) value_type(args...);
-    if constexpr (Time_stamper::has_timestamp) {
-      ret->set_time_stamp(ts);
-    }
     Time_stamper::set_time_stamp(ret, time_stamp);
     CGAL_assertion(type(ret) == USED);
     ++size_;
@@ -414,24 +412,7 @@ public:
 
   iterator insert(const T &t)
   {
-    if (free_list == nullptr)
+    return emplace(t);
-      allocate_new_block();
-
-    pointer ret = free_list;
-    free_list = clean_pointee(ret);
-    std::size_t ts;
-    CGAL_USE(ts);
-    if constexpr (Time_stamper::has_timestamp) {
-      ts = ret->time_stamp();
-    }
-    std::allocator_traits<allocator_type>::construct(alloc, ret, t);
-    if constexpr (Time_stamper::has_timestamp) {
-      ret->set_time_stamp(ts);
-    }
-    Time_stamper::set_time_stamp(ret, time_stamp);
-    CGAL_assertion(type(ret) == USED);
-    ++size_;
-    return iterator(ret, 0);
   }
 
   template < class InputIterator >
@@ -450,22 +431,14 @@ public:
 
   void erase(iterator x)
   {
-    typedef internal::Erase_counter_strategy<
+    auto ptr = &*x;
-      internal::has_increment_erase_counter<T>::value> EraseCounterStrategy;
+    CGAL_precondition(type(ptr) == USED);
+    EraseCounterStrategy<T>::increment_erase_counter(*x);
+    const auto ts = Time_stamper::time_stamp(ptr);
+    std::allocator_traits<allocator_type>::destroy(alloc, ptr);
+    Time_stamper::restore_timestamp(ptr, ts);
 
-    CGAL_precondition(type(&*x) == USED);
+    put_on_free_list(ptr);
-    EraseCounterStrategy::increment_erase_counter(*x);
-    std::size_t ts;
-    CGAL_USE(ts);
-    if constexpr (Time_stamper::has_timestamp) {
-      ts = x->time_stamp();
-    }
-    std::allocator_traits<allocator_type>::destroy(alloc, &*x);
-    if constexpr (Time_stamper::has_timestamp) {
-      x->set_time_stamp(ts);
-    }
-
-    put_on_free_list(&*x);
     --size_;
   }
 
@@ -598,25 +571,7 @@ public:
     while ( capacity_<n )
     { // Pb because the order of free list is no more the order of
       // allocate_new_block();
-      pointer new_block = alloc.allocate(block_size + 2);
+      push_back_new_block();
-      all_items.push_back(std::make_pair(new_block, block_size + 2));
-      capacity_ += block_size;
-      // We insert this new block at the end.
-      if (last_item == nullptr) // First time
-      {
-        first_item = new_block;
-        last_item  = new_block + block_size + 1;
-        set_type(first_item, nullptr, START_END);
-      }
-      else
-      {
-        set_type(last_item, new_block, BLOCK_BOUNDARY);
-        set_type(new_block, last_item, BLOCK_BOUNDARY);
-        last_item = new_block + block_size + 1;
-      }
-      set_type(last_item, nullptr, START_END);
-      // Increase the block_size for the next time.
-      Increment_policy::increase_size(*this);
     }
 
     // Now we put all the new elements on freelist, starting from the last block
@@ -627,16 +582,18 @@ public:
     do
     {
       --curblock; // We are sure we have at least create a new block
-      pointer new_block = all_items[curblock].first;
+      auto [new_block, block_size] = all_items[curblock];
-      for (size_type i = all_items[curblock].second-2; i >= 1; --i)
+      put_block_on_free_list(new_block, block_size - 2);
-        put_on_free_list(new_block + i);
     }
     while ( curblock>lastblock );
   }
 
 private:
 
-  void allocate_new_block();
+std::pair<pointer, size_type> push_back_new_block();
+void put_block_on_free_list(pointer new_block, size_type block_size);
+
+void allocate_new_block();
 
   void put_on_free_list(pointer x)
   {
@@ -697,7 +654,7 @@ public:
   static bool is_begin_or_end(const_pointer ptr)
   { return type(ptr)==START_END; }
 
-  void swap(Self &c)
+  void swap(Self &c) noexcept
   {
     std::swap(alloc, c.alloc);
     std::swap(capacity_, c.capacity_);
@@ -803,23 +760,13 @@ void Compact_container<T, Allocator, Increment_policy, TimeStamper>::clear()
 }
 
 template < class T, class Allocator, class Increment_policy, class TimeStamper >
-void Compact_container<T, Allocator, Increment_policy, TimeStamper>::allocate_new_block()
+auto Compact_container<T, Allocator, Increment_policy, TimeStamper>::push_back_new_block()
+    -> std::pair<pointer, size_type>
 {
-  typedef internal::Erase_counter_strategy<
-    internal::has_increment_erase_counter<T>::value> EraseCounterStrategy;
-
   pointer new_block = alloc.allocate(block_size + 2);
+  std::pair<pointer, size_type> result{new_block, block_size};
   all_items.push_back(std::make_pair(new_block, block_size + 2));
   capacity_ += block_size;
-  // We don't touch the first and the last one.
-  // We mark them free in reverse order, so that the insertion order
-  // will correspond to the iterator order...
-  for (size_type i = block_size; i >= 1; --i)
-  {
-    EraseCounterStrategy::set_erase_counter(*(new_block + i), 0);
-    Time_stamper::initialize_time_stamp(new_block + i);
-    put_on_free_list(new_block + i);
-  }
   // We insert this new block at the end.
   if (last_item == nullptr) // First time
   {
@@ -836,8 +783,33 @@ void Compact_container<T, Allocator, Increment_policy, TimeStamper>::allocate_ne
   set_type(last_item, nullptr, START_END);
   // Increase the block_size for the next time.
   Increment_policy::increase_size(*this);
+  return result;
 }
 
+template < class T, class Allocator, class Increment_policy, class TimeStamper >
+void Compact_container<T, Allocator, Increment_policy, TimeStamper>::
+put_block_on_free_list(pointer new_block, size_type block_size)
+{
+  // The block actually has a size==block_size+2.
+  // We don't touch the first and the last one.
+  // We mark them free in reverse order, so that the insertion order
+  // will correspond to the iterator order...
+  for (size_type i = block_size; i >= 1; --i)
+  {
+    EraseCounterStrategy<T>::set_erase_counter(*(new_block + i), 0);
+    Time_stamper::initialize_time_stamp(new_block + i);
+    put_on_free_list(new_block + i);
+  }
+}
+
+template < class T, class Allocator, class Increment_policy, class TimeStamper >
+void Compact_container<T, Allocator, Increment_policy, TimeStamper>::allocate_new_block()
+{
+  auto [new_block, block_size] = push_back_new_block();
+  put_block_on_free_list(new_block, block_size);
+}
+
+
 template < class T, class Allocator, class Increment_policy, class TimeStamper >
 inline
 bool operator==(const Compact_container<T, Allocator, Increment_policy, TimeStamper> &lhs,

STL_Extension/include/CGAL/Concurrent_compact_container.h

@@ -202,6 +202,9 @@ class Concurrent_compact_container
   typedef Concurrent_compact_container <T, Al>                      Self;
   typedef Concurrent_compact_container_traits <T>                   Traits;
 
+  template <typename U> using EraseCounterStrategy =
+      internal::Erase_counter_strategy<internal::has_increment_erase_counter<U>::value>;
+
 public:
   typedef CGAL::Time_stamper_impl<T>                Time_stamper;
   typedef Time_stamper                              Time_stamper_impl; // backward compatibility
@@ -344,28 +347,21 @@ public:
   // (just forward the arguments to the constructor, to optimize a copy).
   template < typename... Args >
   iterator
-  emplace(const Args&... args)
+  emplace(Args&&... args)
   {
-    typedef CCC_internal::Erase_counter_strategy<
-      CCC_internal::has_increment_erase_counter<T>::value> EraseCounterStrategy;
     FreeList * fl = get_free_list();
     pointer ret = init_insert(fl);
-    auto erase_counter = EraseCounterStrategy::erase_counter(*ret);;
+    auto erase_counter = EraseCounterStrategy<T>::erase_counter(*ret);
-    new (ret) value_type(args...);
+    const auto ts = Time_stamper::time_stamp(ret);
-    EraseCounterStrategy::set_erase_counter(*ret, erase_counter);
+    new (ret) value_type(std::forward<Args>(args)...);
+    Time_stamper::restore_timestamp(ret, ts);
+    EraseCounterStrategy<T>::set_erase_counter(*ret, erase_counter);
     return finalize_insert(ret, fl);
   }
 
   iterator insert(const T &t)
   {
-    typedef CCC_internal::Erase_counter_strategy<
+    return emplace(t);
-      CCC_internal::has_increment_erase_counter<T>::value> EraseCounterStrategy;
-    FreeList * fl = get_free_list();
-    pointer ret = init_insert(fl);
-    auto erase_counter = EraseCounterStrategy::erase_counter(*ret);;
-    std::allocator_traits<allocator_type>::construct(m_alloc, ret, t);
-    EraseCounterStrategy::set_erase_counter(*ret, erase_counter);
-    return finalize_insert(ret, fl);
   }
 
   template < class InputIterator >
@@ -385,13 +381,13 @@ public:
 private:
   void erase(iterator x, FreeList * fl)
   {
-    typedef CCC_internal::Erase_counter_strategy<
-      CCC_internal::has_increment_erase_counter<T>::value> EraseCounterStrategy;
-
     CGAL_precondition(type(x) == USED);
-    EraseCounterStrategy::increment_erase_counter(*x);
+    EraseCounterStrategy<T>::increment_erase_counter(*x);
 
+    auto ptr = &*x;
+    const auto ts = Time_stamper::time_stamp(ptr);
     std::allocator_traits<allocator_type>::destroy(m_alloc, &*x);
+    Time_stamper::restore_timestamp(ptr, ts);
 
     put_on_free_list(&*x, fl);
   }
@@ -778,9 +774,6 @@ template < class T, class Allocator >
 void Concurrent_compact_container<T, Allocator>::
   allocate_new_block(FreeList * fl)
 {
-  typedef CCC_internal::Erase_counter_strategy<
-    CCC_internal::has_increment_erase_counter<T>::value> EraseCounterStrategy;
-
   size_type old_block_size;
   pointer new_block;
 
@@ -818,7 +811,7 @@ void Concurrent_compact_container<T, Allocator>::
   // will correspond to the iterator order...
   for (size_type i = old_block_size; i >= 1; --i)
   {
-    EraseCounterStrategy::set_erase_counter(*(new_block + i), 0);
+    EraseCounterStrategy<T>::set_erase_counter(*(new_block + i), 0);
     Time_stamper::initialize_time_stamp(new_block + i);
     put_on_free_list(new_block + i, fl);
   }

STL_Extension/include/CGAL/Time_stamper.h

@@ -30,13 +30,17 @@ struct Time_stamper
 {
   static constexpr bool has_timestamp = true;
 
+  static bool is_valid(const T* pt) {
+    return pt != nullptr && pt->time_stamp() != std::size_t(-2);
+  }
+
   static void initialize_time_stamp(T* pt) {
     pt->set_time_stamp(std::size_t(-1));
   }
 
   template <typename time_stamp_t>
   static void set_time_stamp(T* pt, time_stamp_t& time_stamp_) {
-    CGAL_assertion(pt->time_stamp() != std::size_t(-2));
+    CGAL_assertion(is_valid(pt));
     if(pt->time_stamp() == std::size_t(-1)) {
       const std::size_t new_ts = time_stamp_++;
       pt->set_time_stamp(new_ts);
@@ -64,13 +68,18 @@ struct Time_stamper
 
   static std::size_t time_stamp(const T* pt)
   {
-    CGAL_assertion(pt == nullptr || pt->time_stamp() != std::size_t(-2));
+    CGAL_assertion(is_valid(pt));
     if(pt == nullptr){
       return std::size_t(-1);
     }
     return pt->time_stamp();
   }
 
+  static void restore_timestamp(T* pt, std::size_t ts)
+  {
+    pt->set_time_stamp(ts);
+  }
+
   static auto display_id(const T* pt, int offset = 0)
   {
     if(pt == nullptr)
@@ -80,7 +89,7 @@ struct Time_stamper
   }
 
   static std::size_t hash_value(const T* p) {
-    CGAL_assertion(p == nullptr || p->time_stamp() != std::size_t(-2));
+    CGAL_assertion(nullptr== p || is_valid(p));
     if(nullptr == p)
       return std::size_t(-1);
     else
@@ -116,6 +125,10 @@ public:
     return 0;
   }
 
+  static void restore_timestamp(T*, std::size_t)
+  {
+  }
+
   static auto display_id(const T* pt, int)
   {
     return static_cast<const void*>(pt);

STL_Extension/test/STL_Extension/test_Compact_container.cpp

@@ -18,7 +18,6 @@ template <typename Has_timestamp_ = CGAL::Tag_true>
 struct Node_1
 : public CGAL::Compact_container_base
 {
-  Node_1() {} // // it is important `time_stamp_` is not initialized
   bool operator==(const Node_1 &) const { return true; }
   bool operator!=(const Node_1 &) const { return false; }
   bool operator< (const Node_1 &) const { return false; }
@@ -49,7 +48,7 @@ struct Node_1
   }
   ///@}
   int m_erase_counter;
-  std::size_t time_stamp_;
+  std::size_t time_stamp_ = std::size_t(-2);
 };
 
 class Node_2

STL_Extension/test/STL_Extension/test_Concurrent_compact_container.cpp

@@ -32,7 +32,6 @@ int main()
 struct Node_1
 : public CGAL::Compact_container_base
 {
-  Node_1() {}
   bool operator==(const Node_1 &) const { return true; }
   bool operator!=(const Node_1 &) const { return false; }
   bool operator< (const Node_1 &) const { return false; }
@@ -45,7 +44,7 @@ struct Node_1
   void set_time_stamp(const std::size_t& ts) {
     time_stamp_ = ts;
   }
-  std::size_t time_stamp_;
+  std::size_t time_stamp_ = std::size_t(-2);
 };
 
 class Node_2

STL_Extension/test/STL_Extension/test_namespaces.cpp

@@ -13,7 +13,7 @@
 
 int main()
 {
-  CGAL::cpp0x::array<int, 3> arr;
+  CGAL::cpp0x::array<int, 3> arr{1, 2, 3};
   std::array<int, 3> arr2;
 
   CGAL::cpp0x::tuple<double, int> tuple;