Cmap add test gdamiand (#8737)

## Summary of Changes

Solve a bug for CMap and GMap when we iterate on an empty map with an
iterator that needs to mark cells, and when using index version.

## Release Management

* Affected package(s): CMap GMap

Combinatorial_map/include/CGAL/Cell_iterators.h

@@ -86,7 +86,8 @@ namespace CGAL {
                               Tag_true>::value);
       CGAL_assertion(amap.is_whole_map_unmarked(mcell_mark_number));
 
-      mark_cell<Map,i,dim>(amap, adart, mcell_mark_number);
+      if(this->cont())
+      { mark_cell<Map,i,dim>(amap, adart, mcell_mark_number); }
     }
 
     /// Destructor.
@@ -126,7 +127,8 @@ namespace CGAL {
     {
       unmark_treated_darts();
       Ite::rewind();
-      mark_cell<Map,i,dim>(*this->mmap, (*this), mcell_mark_number);
+      if(this->cont())
+      { mark_cell<Map,i,dim>(*this->mmap, (*this), mcell_mark_number); }
     }
 
     /// Prefix ++ operator.
@@ -199,7 +201,8 @@ namespace CGAL {
       static_assert(std::is_same<typename Ite::Basic_iterator,
                               Tag_true>::value);
       CGAL_assertion(amap.is_whole_map_unmarked(mmark_number));
-      mark_cell<Map,i,dim>(amap, adart, mmark_number);
+      if(this->cont())
+      { mark_cell<Map,i,dim>(amap, adart, mmark_number); }
     }
 
     /// Destructor.
@@ -234,7 +237,8 @@ namespace CGAL {
     {
       unmark_treated_darts();
       Ite::rewind();
-      mark_cell<Map,i,dim>(*this->mmap, (*this), mmark_number);
+      if(this->cont())
+      { mark_cell<Map,i,dim>(*this->mmap, (*this), mmark_number); }
     }
 
     /// Postfix ++ operator.
@@ -253,7 +257,7 @@ namespace CGAL {
              this->mmap->is_marked((*this), mmark_number));
 
       if (this->cont())
-        mark_cell<Map,i,dim>(*this->mmap, (*this), mmark_number);
+      { mark_cell<Map,i,dim>(*this->mmap, (*this), mmark_number); }
       return *this;
     }
 
@@ -306,7 +310,8 @@ namespace CGAL {
       static_assert(std::is_same<typename Base::Basic_iterator,
                               Tag_true>::value);
       CGAL_assertion(amap.is_whole_map_unmarked(mmark_number));
-      mark_cell<Map,i,dim>(amap, (*this), mmark_number);
+      if(this->cont())
+      { mark_cell<Map,i,dim>(amap, (*this), mmark_number); }
     }
 
    /// Constructor with a dart in parameter (for end iterator).
@@ -314,8 +319,8 @@ namespace CGAL {
       Base(amap, adart),
       mmark_number(amap.get_new_mark())
     {
-      if (adart!=this->mmap->null_descriptor)
-        mark_cell<Map,i,dim>(amap, (*this), mmark_number);
+      if (this->cont())
+      { mark_cell<Map,i,dim>(amap, (*this), mmark_number); }
     }
 
     /// Destructor.
@@ -350,7 +355,8 @@ namespace CGAL {
     {
       unmark_treated_darts();
       Base::rewind();
-      mark_cell<Map,i,dim>(*this->mmap, (*this), mmark_number);
+      if(this->cont())
+      { mark_cell<Map,i,dim>(*this->mmap, (*this), mmark_number); }
     }
 
     /// Postfix ++ operator.
@@ -369,7 +375,7 @@ namespace CGAL {
              this->mmap->is_marked((*this), mmark_number));
 
       if (this->cont())
-        mark_cell<Map,i,dim>(*this->mmap, (*this), mmark_number);
+      { mark_cell<Map,i,dim>(*this->mmap, (*this), mmark_number); }
       return *this;
     }
 

Combinatorial_map/include/CGAL/Compact_container_with_index.h

@@ -610,16 +610,16 @@ public:
   }
 
   iterator begin() { return empty()?end():iterator(this, 0, 0); }
-  iterator end()   { return iterator(this, upper_bound()); }
+  iterator end()   { return iterator(this, null_descriptor); }
 
   const_iterator begin() const { return empty()?end():const_iterator(this, 0, 0); }
-  const_iterator end()   const { return const_iterator(this, upper_bound()); }
+  const_iterator end()   const { return const_iterator(this, null_descriptor); }
 
-  reverse_iterator rbegin() { return reverse_iterator(end()); }
+  reverse_iterator rbegin() { return reverse_iterator(iterator(this, upper_bound())); }
   reverse_iterator rend()   { return reverse_iterator(begin()); }
 
   const_reverse_iterator
-  rbegin() const { return const_reverse_iterator(end()); }
+  rbegin() const { return const_reverse_iterator(iterator(this, upper_bound())); }
   const_reverse_iterator
   rend()   const { return const_reverse_iterator(begin()); }
 
@@ -878,7 +878,8 @@ namespace internal {
     CC_iterator_with_index(const iterator &it): m_ptr_to_cc(it.m_ptr_to_cc),
       m_index(it.m_index)
     {
-      CGAL_assertion(m_index<=m_ptr_to_cc->upper_bound());
+       CGAL_assertion(m_index<=m_ptr_to_cc->upper_bound() ||
+                     m_index==DSC::null_descriptor);
     }
 
     // Same for assignment operator
@@ -928,14 +929,16 @@ namespace internal {
       // It's either pointing to end(), or valid.
       CGAL_assertion_msg(m_ptr_to_cc!=nullptr,
          "Incrementing a singular iterator or an empty container iterator ?");
-      CGAL_assertion_msg(m_index<m_ptr_to_cc->upper_bound(),
-         "Incrementing end() ?");
+      CGAL_assertion_msg(m_index<m_ptr_to_cc->upper_bound() &&
+                             m_index!=DSC::null_descriptor,
+                         "Incrementing end() ?");
 
       // If it's not end(), then it's valid, we can do ++.
       do
       { ++m_index; }
       while(m_index<m_ptr_to_cc->upper_bound() &&
             (!m_ptr_to_cc->is_used(m_index)));
+      if(m_index==m_ptr_to_cc->upper_bound()) { m_index=DSC::null_descriptor; }
     }
 
     void decrement()
@@ -969,7 +972,7 @@ namespace internal {
 
     pointer   operator->() const { return &((*m_ptr_to_cc)[m_index]); }
 
-    bool is_end() const { return m_index>=m_ptr_to_cc->upper_bound(); }
+    bool is_end() const { return m_index==DSC::null_descriptor; }
 
     // Can itself be used for bit-squatting.
     size_type for_compact_container() const

Combinatorial_map/test/Combinatorial_map/CMakeLists.txt

@@ -12,6 +12,8 @@ set(hfiles Combinatorial_map_2_test.h Combinatorial_map_3_test.h
 # create a target per cppfile
 create_single_source_cgal_program(Combinatorial_map_test.cpp ${hfiles})
 create_single_source_cgal_program(Combinatorial_map_copy_test.cpp ${hfiles})
+create_single_source_cgal_program(cmap_test_split_attribute.cpp)
+create_single_source_cgal_program(Combinatorial_map_empty_it_test.cpp)
 
 # Same targets, defining USE_COMPACT_CONTAINER_WITH_INDEX to test index version
 add_executable(Combinatorial_map_test_index  Combinatorial_map_test.cpp ${hfiles})
@@ -24,7 +26,10 @@ target_compile_definitions(Combinatorial_map_copy_test_index PRIVATE USE_COMPACT
 target_link_libraries(Combinatorial_map_copy_test_index PRIVATE CGAL::CGAL CGAL::Data)
 cgal_add_compilation_test(Combinatorial_map_copy_test_index)
 
-create_single_source_cgal_program(cmap_test_split_attribute.cpp)
+add_executable(Combinatorial_map_empty_it_test_index Combinatorial_map_empty_it_test.cpp)
+target_compile_definitions(Combinatorial_map_empty_it_test_index PRIVATE USE_COMPACT_CONTAINER_WITH_INDEX)
+target_link_libraries(Combinatorial_map_empty_it_test_index PRIVATE CGAL::CGAL CGAL::Data)
+cgal_add_compilation_test(Combinatorial_map_empty_it_test_index)
 
 # Link with OpenMesh if possible
 find_package(OpenMesh QUIET)

Combinatorial_map/test/Combinatorial_map/Combinatorial_map_empty_it_test.cpp

@@ -0,0 +1,53 @@
+#include <CGAL/Combinatorial_map.h>
+
+struct Map_3_dart_items_3: public CGAL::Generic_map_min_items
+{
+#ifdef USE_COMPACT_CONTAINER_WITH_INDEX
+  typedef CGAL::Tag_true Use_index;
+#endif
+};
+
+using Map3=CGAL::Combinatorial_map<3, Map_3_dart_items_3>;
+Map3 m;
+
+template<typename Range>
+bool test_empty_it(const Range& r, const std::string& txt)
+{
+  bool res=true;
+  if(r.size()!=0)
+  {
+    std::cout<<"[ERROR "<<txt<<"] non zero size with range: "<<r.size()<<std::endl;
+    res=false;
+  }
+  std::size_t nb=0;
+  for(auto it=r.begin(), itend=r.end(); it!=itend; ++it)
+  { ++nb; }
+  if(nb!=0)
+  {
+    std::cout<<"[ERROR "<<txt<<"] non zero size with it: "<<nb<<std::endl;
+    res=false;
+  }
+
+  return res;
+}
+int main()
+{
+  bool res=true;
+
+  res=res && test_empty_it<Map3::Dart_range>(m.darts(), "Dart_range");
+  res=res && test_empty_it<Map3::Dart_const_range>
+    (const_cast<const Map3&>(m).darts(), "Dart_const_range");
+
+  res=res && test_empty_it<Map3::One_dart_per_cell_range<3>>
+    (m.one_dart_per_cell<3>(), "One_dart_per_cell_range<0>");
+
+  res=res && test_empty_it<Map3::One_dart_per_cell_const_range<3>>
+    (const_cast<const Map3&>(m).one_dart_per_cell<3>(),
+     "One_dart_per_cell_const_range<0>");
+
+  if(!res)
+  { return(EXIT_FAILURE); }
+
+  std::cout<<"ALL SUCCESS."<<std::endl;
+  return EXIT_SUCCESS;
+}

Generalized_map/include/CGAL/GMap_cell_iterators.h

@@ -73,7 +73,8 @@ namespace CGAL {
       static_assert(std::is_same<typename Base::Basic_iterator,
                               Tag_true>::value);
       CGAL_assertion(amap.is_whole_map_unmarked(mmark_number));
-      mark_cell<Map,i,dim>(amap, (*this), mmark_number);
+      if(this->cont())
+      { mark_cell<Map,i,dim>(amap, (*this), mmark_number); }
     }
 
    /// Constructor with a dart in parameter (for end iterator).
@@ -81,8 +82,8 @@ namespace CGAL {
       Base(amap, adart),
       mmark_number(amap.get_new_mark())
     {
-      if (adart!=this->mmap->null_descriptor)
-        mark_cell<Map,i,dim>(amap, (*this), mmark_number);
+      if (this->cont())
+      { mark_cell<Map,i,dim>(amap, (*this), mmark_number); }
     }
 
     /// Destructor.
@@ -136,7 +137,7 @@ namespace CGAL {
              this->mmap->is_marked((*this), mmark_number));
 
       if (this->cont())
-        mark_cell<Map,i,dim>(*this->mmap, (*this), mmark_number);
+      { mark_cell<Map,i,dim>(*this->mmap, (*this), mmark_number); }
       return *this;
     }
 
@@ -249,7 +250,7 @@ namespace CGAL {
              this->mmap->is_marked((*this), mmark_number));
 
       if (this->cont())
-        mark_cell<Map,i,dim>(*this->mmap, (*this), mmark_number);
+      { mark_cell<Map,i,dim>(*this->mmap, (*this), mmark_number); }
       return *this;
     }