Merge remote-tracking branch 'cgal/6.0.x-branch'

Conflicts:
	STL_Extension/include/CGAL/Compact_container.h
	STL_Extension/include/CGAL/Concurrent_compact_container.h

Mesh_3/include/CGAL/Compact_mesh_cell_base_3.h

@@ -21,12 +21,12 @@
 #include <CGAL/array.h>
 #include <CGAL/assertions.h>
 #include <CGAL/basic.h>
-#include <CGAL/TDS_3/internal/Dummy_tds_3.h>
-#include <CGAL/tags.h>
 #include <CGAL/Has_timestamp.h>
-
 #include <CGAL/Regular_triangulation_cell_base_3.h>
 #include <CGAL/SMDS_3/io_signature.h>
+#include <CGAL/tags.h>
+#include <CGAL/TDS_3/internal/Dummy_tds_3.h>
+#include <CGAL/Time_stamper.h>
 
 #ifdef CGAL_LINKED_WITH_TBB
 # include <atomic>
@@ -644,7 +644,7 @@ private:
 #ifdef CGAL_INTRUSIVE_LIST
   Cell_handle next_intrusive_ = {}, previous_intrusive_ = {};
 #endif
-  std::size_t time_stamp_ = std::size_t(-2);
+  std::size_t time_stamp_ = Time_stamper<void>::invalid_time_stamp;
 
   std::array<Index, 4> surface_center_index_table_ = {};
   /// Stores visited facets (4 first bits)

SMDS_3/include/CGAL/Compact_simplicial_mesh_cell_base_3.h

@@ -19,10 +19,10 @@
 #include <CGAL/assertions.h>
 #include <CGAL/basic.h>
 #include <CGAL/Has_timestamp.h>
+#include <CGAL/SMDS_3/io_signature.h>
 #include <CGAL/tags.h>
 #include <CGAL/TDS_3/internal/Dummy_tds_3.h>
-
+#include <CGAL/Time_stamper.h>
-#include <CGAL/SMDS_3/io_signature.h>
 
 namespace CGAL {
 
@@ -266,7 +266,7 @@ private:
   std::array<Cell_handle, 4> N;
   std::array<Vertex_handle, 4> V;
 
-  std::size_t time_stamp_ =  std::size_t(-2);
+  std::size_t time_stamp_ =  Time_stamper<void>::invalid_time_stamp;
 
   // 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

@@ -19,12 +19,12 @@
 #include <CGAL/array.h>
 #include <CGAL/assertions.h>
 #include <CGAL/basic.h>
-#include <CGAL/TDS_3/internal/Dummy_tds_3.h>
-#include <CGAL/tags.h>
 #include <CGAL/Has_timestamp.h>
-
-#include <CGAL/Triangulation_cell_base_3.h>
 #include <CGAL/SMDS_3/io_signature.h>
+#include <CGAL/tags.h>
+#include <CGAL/TDS_3/internal/Dummy_tds_3.h>
+#include <CGAL/Time_stamper.h>
+#include <CGAL/Triangulation_cell_base_3.h>
 
 #ifdef CGAL_LINKED_WITH_TBB
 # include <atomic>
@@ -189,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(-2);
+  std::size_t time_stamp_ = Time_stamper<void>::invalid_time_stamp;
 
   // The index of the cell of the input complex that contains me
   Subdomain_index subdomain_index_ = {};

STL_Extension/include/CGAL/Compact_container.h

@@ -218,6 +218,33 @@ namespace internal {
       e.increment_erase_counter();
     }
   };
+
+  template <typename T, typename Time_stamper>
+  struct Time_stamp_and_erase_counter_backup_and_restore_guard
+  {
+    T* const ptr;
+    const unsigned int ec;
+    std::size_t ts;
+
+    using EraseCounterStrategy =
+        internal::Erase_counter_strategy<internal::has_increment_erase_counter<T>::value>;
+
+    Time_stamp_and_erase_counter_backup_and_restore_guard(T* ptr)
+      : ptr(ptr), ec(EraseCounterStrategy::erase_counter(ptr))
+    {
+      if constexpr (Time_stamper::has_timestamp) {
+        ts = Time_stamper::time_stamp(ptr);
+      }
+    }
+
+    ~Time_stamp_and_erase_counter_backup_and_restore_guard() {
+      EraseCounterStrategy::restore_erase_counter(ptr, ec);
+      if constexpr (Time_stamper::has_timestamp) {
+        Time_stamper::restore_timestamp(ptr, ts);
+      }
+    }
+  };
+
 }
 
 template < class T,
@@ -415,20 +442,24 @@ public:
 
     pointer ret = free_list;
     free_list = clean_pointee(ret);
-    const auto ts = Time_stamper::time_stamp(ret);
+
-    const auto ec = EraseCounterStrategy<T>::erase_counter(ret);
+    {
-    new (ret) value_type(std::forward<Args>(args)...);
+      internal::Time_stamp_and_erase_counter_backup_and_restore_guard<T, Time_stamper>
-    Time_stamper::restore_timestamp(ret, ts);
+          guard(ret);
+
+      new (ret) value_type(std::forward<Args>(args)...);
+    } // this scope is important: the destructor of the guard has to be called
+      // before the time_stamp is set, otherwise it will be set to the wrong value.
     Time_stamper::set_time_stamp(ret, time_stamp);
-    EraseCounterStrategy<T>::restore_erase_counter(ret, ec);
     CGAL_assertion(type(ret) == USED);
     ++size_;
     return iterator(ret, 0);
   }
 
-  iterator insert(const T &t)
+  template <typename U>
+  iterator insert(U&&u)
   {
-    return emplace(t);
+    return emplace(std::forward<U>(u));
   }
 
   template < class InputIterator >
@@ -450,11 +481,13 @@ public:
     auto ptr = &*x;
     CGAL_precondition(type(ptr) == USED);
     EraseCounterStrategy<T>::increment_erase_counter(*x);
-    const auto ts = Time_stamper::time_stamp(ptr);
+
-    const auto ec = EraseCounterStrategy<T>::erase_counter(*x);
+    {
-    std::allocator_traits<allocator_type>::destroy(alloc, ptr);
+      internal::Time_stamp_and_erase_counter_backup_and_restore_guard<T, Time_stamper>
-    Time_stamper::restore_timestamp(ptr, ts);
+          guard(ptr);
-    EraseCounterStrategy<T>::restore_erase_counter(ptr, ec);
+
+      std::allocator_traits<allocator_type>::destroy(alloc, ptr);
+    }
 
     put_on_free_list(ptr);
     --size_;
@@ -531,7 +564,7 @@ public:
       res += s-2;
     }
 
-    return (size_type)-1; // cit does not belong to this compact container
+    return static_cast<size_type>(-1); // cit does not belong to this compact container
   }
 
   // Returns whether the iterator "cit" is in the range [begin(), end()].
@@ -547,21 +580,18 @@ public:
     if (cit == end())
       return true;
 
-    const_pointer c = &*cit;
+    const_pointer ptr = &*cit;
 
-    for (typename All_items::const_iterator it = all_items.begin(), itend = all_items.end();
+    for (const auto [chunk_ptr, size] : all_items) {
-         it != itend; ++it) {
-      const_pointer p = it->first;
-      size_type s = it->second;
 
       // Are we in the address range of this block (excluding first and last
       // elements) ?
-      if (c <= p || (p+s-1) <= c)
+      if (ptr <= chunk_ptr || (chunk_ptr+size-1) <= ptr)
         continue;
 
-      CGAL_assertion_msg( (c-p)+p == c, "wrong alignment of iterator");
+      CGAL_assertion_msg( (ptr-chunk_ptr)+chunk_ptr == ptr, "wrong alignment of iterator");
 
-      return type(c) == USED;
+      return type(ptr) == USED;
     }
     return false;
   }
@@ -571,7 +601,8 @@ public:
     return cit != end() && owns(cit);
   }
 
-
+  // wrong spelling, kept for backward compatibility
+  //   cspell:disable-next-line
   CGAL_DEPRECATED bool owns_dereferencable(const_iterator cit) const
   {
     return owns_dereferenceable(cit);
@@ -639,32 +670,40 @@ void allocate_new_block();
 
   static char * clean_pointer(char * p)
   {
-    return reinterpret_cast<char*>(reinterpret_cast<std::ptrdiff_t>(p) &
+    auto ptr = reinterpret_cast<std::ptrdiff_t>(p);
-                                   ~ (std::ptrdiff_t) START_END);
+    auto mask = static_cast<std::ptrdiff_t>(START_END);
+    return reinterpret_cast<char*>(ptr & ~mask);
   }
 
   // Returns the pointee, cleaned up from the squatted bits.
-  static pointer clean_pointee(const_pointer ptr)
+  static pointer clean_pointee(const_pointer ptr) {
-  {
+    void* raw_ptr = Traits::pointer(*ptr);
-    return (pointer) clean_pointer((char *) Traits::pointer(*ptr));
+    char* cleaned_ptr = clean_pointer(reinterpret_cast<char*>(raw_ptr));
+    return reinterpret_cast<pointer>(cleaned_ptr);
   }
 
   // Get the type of the pointee.
   static Type type(const_pointer ptr)
   {
-    char * p = (char *) Traits::pointer(*ptr);
+    char* p = reinterpret_cast<char*>(Traits::pointer(*ptr));
-    return (Type) (reinterpret_cast<std::ptrdiff_t>(p) -
+    // Compute the difference between the pointer and its cleaned version to extract the last 2 bits.
-                   reinterpret_cast<std::ptrdiff_t>(clean_pointer(p)));
+    std::ptrdiff_t diff = reinterpret_cast<std::ptrdiff_t>(p) -
+                          reinterpret_cast<std::ptrdiff_t>(clean_pointer(p));
+    return static_cast<Type>(diff);
   }
 
   // Sets the pointer part and the type of the pointee.
-  static void set_type(pointer ptr, void * p, Type t)
+  static void set_type(pointer ptr, void * p, Type type)
   {
     // This out of range compare is always true and causes lots of
     // unnecessary warnings.
     // CGAL_precondition(0 <= t && t < 4);
-    Traits::set_pointer(*ptr, reinterpret_cast<void *>
+
-      (reinterpret_cast<std::ptrdiff_t>(clean_pointer((char *) p)) + (int) t));
+    // Clean the pointer to remove any existing type bits, then set the new type bits.
+    char* p_cleaned = clean_pointer(reinterpret_cast<char*>(p));
+    std::ptrdiff_t p_value = reinterpret_cast<std::ptrdiff_t>(p_cleaned);
+    std::ptrdiff_t new_ptr = p_value + static_cast<std::ptrdiff_t>(type);
+    Traits::set_pointer(*ptr, reinterpret_cast<void*>(new_ptr));
   }
 
 public:

STL_Extension/include/CGAL/Concurrent_compact_container.h

@@ -365,19 +365,18 @@ public:
   {
     FreeList * fl = get_free_list();
     pointer ret = init_insert(fl);
-    auto erase_counter = EraseCounterStrategy<T>::erase_counter(*ret);
+    {
-    const auto ts = Time_stamper::time_stamp(ret);
+      internal::Time_stamp_and_erase_counter_backup_and_restore_guard<T, Time_stamper>
-    const auto ec = EraseCounterStrategy<T>::erase_counter(ret);
+          guard(ret);
-    new (ret) value_type(std::forward<Args>(args)...);
+      new (ret) value_type(std::forward<Args>(args)...);
-    Time_stamper::restore_timestamp(ret, ts);
+    }
-    EraseCounterStrategy<T>::set_erase_counter(*ret, erase_counter);
-    EraseCounterStrategy<T>::restore_erase_counter(ret, ec);
     return finalize_insert(ret, fl);
   }
 
-  iterator insert(const T &t)
+  template <typename U>
+  iterator insert(U&& u)
   {
-    return emplace(t);
+    return emplace(std::forward<U>(u));
   }
 
   template < class InputIterator >
@@ -397,17 +396,18 @@ public:
 private:
   void erase(iterator x, FreeList * fl)
   {
+    auto ptr = &*x;
     CGAL_precondition(type(x) == USED);
     EraseCounterStrategy<T>::increment_erase_counter(*x);
+    {
+      internal::Time_stamp_and_erase_counter_backup_and_restore_guard<T, Time_stamper>
+          guard(ptr);
 
-    auto ptr = &*x;
+      std::allocator_traits<allocator_type>::destroy(m_alloc, ptr);
-    const auto ts = Time_stamper::time_stamp(ptr);
+    }
-    const auto ec = EraseCounterStrategy<T>::erase_counter(*x);
-    std::allocator_traits<allocator_type>::destroy(m_alloc, &*x);
-    Time_stamper::restore_timestamp(ptr, ts);
-    EraseCounterStrategy<T>::restore_erase_counter(ptr, ec);
 
-    put_on_free_list(&*x, fl);
+
+    put_on_free_list(ptr, fl);
   }
 public:
 

STL_Extension/include/CGAL/Time_stamper.h

@@ -29,19 +29,21 @@ template <typename T>
 struct Time_stamper
 {
   static constexpr bool has_timestamp = true;
+  static constexpr std::size_t invalid_time_stamp = static_cast<std::size_t>(-2);
+  static constexpr std::size_t not_yet_used_time_stamp = static_cast<std::size_t>(-1);
 
   static bool is_valid(const T* pt) {
-    return pt != nullptr && pt->time_stamp() != std::size_t(-2);
+    return pt != nullptr && pt->time_stamp() != invalid_time_stamp;
   }
 
   static void initialize_time_stamp(T* pt) {
-    pt->set_time_stamp(std::size_t(-1));
+    pt->set_time_stamp(not_yet_used_time_stamp);
   }
 
   template <typename time_stamp_t>
   static void set_time_stamp(T* pt, time_stamp_t& time_stamp_) {
     CGAL_assertion(is_valid(pt));
-    if(pt->time_stamp() == std::size_t(-1)) {
+    if(pt->time_stamp() == not_yet_used_time_stamp) {
       const std::size_t new_ts = time_stamp_++;
       pt->set_time_stamp(new_ts);
     }
@@ -70,7 +72,7 @@ struct Time_stamper
   {
     CGAL_assertion(is_valid(pt));
     if(pt == nullptr){
-      return std::size_t(-1);
+      return not_yet_used_time_stamp;
     }
     return pt->time_stamp();
   }
@@ -91,7 +93,7 @@ struct Time_stamper
   static std::size_t hash_value(const T* p) {
     CGAL_assertion(nullptr== p || is_valid(p));
     if(nullptr == p)
-      return std::size_t(-1);
+      return not_yet_used_time_stamp;
     else
       return p->time_stamp();
   }

TDS_3/examples/TDS_3/tds.cpp

@@ -39,7 +39,7 @@ int main()
   // cell incident to PV[0]
   Cell_handle c = PV[0]->cell();
   int ind;
-  bool check = c->has_vertex( PV[0], ind );
+  [[maybe_unused]] bool check = c->has_vertex( PV[0], ind );
   assert( check );
   // PV[0] is the vertex of index ind in c
 

TDS_3/test/TDS_3/include/CGAL/_test_cls_tds_3.h

@@ -14,6 +14,7 @@
 #include <cassert>
 #include <iostream>
 #include <fstream>
+#include <set>
 
 #include "_test_cls_tds_vertex.h"
 #include "_test_cls_tds_cell.h"
@@ -164,8 +165,8 @@ _test_cls_tds_3( const Tds &)
   assert(tds3.number_of_vertices()==2);
 
   // Containers
-  Vertex_range & vertex_c = tds3.vertices();
+  [[maybe_unused]] Vertex_range & vertex_c = tds3.vertices();
-  Cell_range & cell_c = tds3.cells();
+  [[maybe_unused]] Cell_range & cell_c = tds3.cells();
 
   assert(vertex_c.size() == 2);
   assert(cell_c.size() == 2);

TDS_3/test/TDS_3/include/CGAL/_test_cls_tds_cell.h

@@ -11,6 +11,7 @@
 // Author(s)     : Rebufat Francois (Francois.Rebufat@sophia.inria.fr)
 
 #include <cassert>
+#include <iostream>
 
 template <class Tds>
 void
@@ -20,14 +21,14 @@ _test_cell_tds_3(const Tds &)
   typedef typename Tds::Cell_handle              Cell_handle;
 
   std::cout << "   Cells Tds Constructors " << std::endl;
-  int ind;
+  [[maybe_unused]] int ind;
   Tds tds;
   tds.set_dimension(3);
   Vertex_handle v0= tds.create_vertex();
   Vertex_handle v1= tds.create_vertex();
   Vertex_handle v2= tds.create_vertex();
   Vertex_handle v3= tds.create_vertex();
-  Cell_handle c1 = tds.create_cell(v0, v1, v2, v3);
+  [[maybe_unused]] Cell_handle c1 = tds.create_cell(v0, v1, v2, v3);
   assert(c1->has_vertex(v0));
   assert(c1->has_vertex(v1));
   assert(c1->has_vertex(v2));