From 13f00211d3cfdfb7cd107469fb2e8e1eb28cd1b5 Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Mon, 18 Apr 2022 16:34:14 +0100 Subject: [PATCH 01/11] Surface_mesh: Add move semantics --- .../include/CGAL/Surface_mesh/Properties.h | 21 +++++++- .../include/CGAL/Surface_mesh/Surface_mesh.h | 54 +++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/Surface_mesh/include/CGAL/Surface_mesh/Properties.h b/Surface_mesh/include/CGAL/Surface_mesh/Properties.h index 825d414e7071..7a02ef0ed738 100644 --- a/Surface_mesh/include/CGAL/Surface_mesh/Properties.h +++ b/Surface_mesh/include/CGAL/Surface_mesh/Properties.h @@ -242,6 +242,12 @@ class Property_container // copy constructor: performs deep copy of property arrays Property_container(const Property_container& _rhs) { operator=(_rhs); } + Property_container(Property_container&& c) noexcept + : size_(0), capacity_(0) + { + c.swap(*this); + } + // assignment: performs deep copy of property arrays Property_container& operator=(const Property_container& _rhs) { @@ -257,6 +263,14 @@ class Property_container return *this; } + Property_container& operator=(Property_container&& c) noexcept + { + Self tmp(std::move(c)); + tmp.swap(*this); + return *this; + } + + void transfer(const Property_container& _rhs) { for(std::size_t i=0; iparrays_.swap (other.parrays_); - std::swap(this->size_, other.size_); + std::swap(this->size_, other.size_); // AF: why not the same for capacity_ ? } private: @@ -554,6 +568,11 @@ class Property_map_base /// @cond CGAL_DOCUMENT_INTERNALS Property_map_base(Property_array* p=nullptr) : parray_(p) {} + Property_map_base(Property_map_base&& pm) noexcept + : parray_(std::exchange(pm.parray_, nullptr)) + {} + + Property_map_base(const Property_map_base& pm) = default; void reset() { parray_ = nullptr; diff --git a/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h b/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h index 2127f3c8ed34..d1cc535344ab 100644 --- a/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h +++ b/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h @@ -343,6 +343,12 @@ class Surface_mesh typedef typename Base::reference reference; Property_map() : Base() {} Property_map(const Base& pm): Base(pm) {} + + Property_map(const Property_map& pm) = default; + + Property_map(Property_map&& pm) noexcept + : Base(pm) + {} }; template @@ -914,9 +920,57 @@ class Surface_mesh /// Copy constructor: copies `rhs` to `*this`. Performs a deep copy of all properties. Surface_mesh(const Surface_mesh& rhs) { *this = rhs; } + Surface_mesh(Surface_mesh&& sm) + : vprops_(std::move(sm.vprops_)) + , hprops_(std::move(sm.hprops_)) + , eprops_(std::move(sm.eprops_)) + , fprops_(std::move(sm.fprops_)) + , vconn_(std::move(sm.vconn_)) + , hconn_(std::move(sm.hconn_)) + , fconn_(std::move(sm.fconn_)) + , vremoved_(std::move(sm.vremoved_)) + , eremoved_(std::move(sm.eremoved_)) + , fremoved_(std::move(sm.fremoved_)) + , vpoint_(std::move(sm.vpoint_)) + , removed_vertices_(sd::exchange(sm.removed_vertices, 0)) + , removed_edges_(sd::exchange(sm.removed_edges, 0)) + , removed_faces_(sd::exchange(sm.removed_faces, 0)) + , vertices_freelist_(std::exchange(sm.vertices_freelist_,(std::numeric_limits::max)())) + , edgces_freelist_(std::exchange(sm.edges_freelist_,(std::numeric_limits::max)())) + , faces_freelist_(std::exchange(sm.faces_freelist_,(std::numeric_limits::max)())) + , garbage_(std::exchange(sm.garbage_, false)) + , recycle_(std::exchange(sm.recycle_, true)) + , anonymous_property_(std::exchange(sm.anonymous_property_, 0)) + {} + /// assigns `rhs` to `*this`. Performs a deep copy of all properties. Surface_mesh& operator=(const Surface_mesh& rhs); + + Surface_mesh& operator=(Surface_mesh&& sm) + { + vprops_ = std::move(sm.vprops_); + hprops_ = std::move(sm.hprops_); + eprops_ = std::move(sm.eprops_); + fprops_ = std::move(sm.fprops_); + vconn_ = std::move(sm.vconn_); + hconn_ = std::move(sm.hconn_); + fconn_ = std::move(sm.fconn_); + vremoved_ = std::move(sm.vremoved_); + eremoved_ = std::move(sm.eremoved_); + fremoved_ = std::move(sm.fremoved_); + vpoint_ = std::move(sm.vpoint_); + removed_vertices_ = sd::exchange(sm.removed_vertices, 0); + removed_edges_ = sd::exchange(sm.removed_edges, 0); + removed_faces_ = sd::exchange(sm.removed_faces, 0); + vertices_freelist_ = std::exchange(sm.vertices_freelist_, (std::numeric_limits::max)();) + edgces_freelist_ = std::exchange(sm.edges_freelist_,(std::numeric_limits::max)();) + faces_freelist_ = std::exchange(sm.faces_freelist_,(std::numeric_limits::max)();) + garbage_ = std::exchange(sm.garbage_, false); + recycle_ = std::exchange(sm.recycle_, true); + anonymous_property_ = std::exchange(sm.anonymous_property_, 0); + } + /// assigns `rhs` to `*this`. Does not copy custom properties. Surface_mesh& assign(const Surface_mesh& rhs); From 92f7344f3c08284c4e65ca343748ddbc29791a47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 18 Apr 2022 20:55:56 +0200 Subject: [PATCH 02/11] fix syntax errors --- .../include/CGAL/Surface_mesh/Properties.h | 2 +- .../include/CGAL/Surface_mesh/Surface_mesh.h | 20 +++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Surface_mesh/include/CGAL/Surface_mesh/Properties.h b/Surface_mesh/include/CGAL/Surface_mesh/Properties.h index 7a02ef0ed738..71f3306fc5e1 100644 --- a/Surface_mesh/include/CGAL/Surface_mesh/Properties.h +++ b/Surface_mesh/include/CGAL/Surface_mesh/Properties.h @@ -265,7 +265,7 @@ class Property_container Property_container& operator=(Property_container&& c) noexcept { - Self tmp(std::move(c)); + Property_container tmp(std::move(c)); tmp.swap(*this); return *this; } diff --git a/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h b/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h index d1cc535344ab..3810b53d1be2 100644 --- a/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h +++ b/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h @@ -932,11 +932,11 @@ class Surface_mesh , eremoved_(std::move(sm.eremoved_)) , fremoved_(std::move(sm.fremoved_)) , vpoint_(std::move(sm.vpoint_)) - , removed_vertices_(sd::exchange(sm.removed_vertices, 0)) - , removed_edges_(sd::exchange(sm.removed_edges, 0)) - , removed_faces_(sd::exchange(sm.removed_faces, 0)) + , removed_vertices_(std::exchange(sm.removed_vertices, 0)) + , removed_edges_(std::exchange(sm.removed_edges, 0)) + , removed_faces_(std::exchange(sm.removed_faces, 0)) , vertices_freelist_(std::exchange(sm.vertices_freelist_,(std::numeric_limits::max)())) - , edgces_freelist_(std::exchange(sm.edges_freelist_,(std::numeric_limits::max)())) + , edges_freelist_(std::exchange(sm.edges_freelist_,(std::numeric_limits::max)())) , faces_freelist_(std::exchange(sm.faces_freelist_,(std::numeric_limits::max)())) , garbage_(std::exchange(sm.garbage_, false)) , recycle_(std::exchange(sm.recycle_, true)) @@ -960,12 +960,12 @@ class Surface_mesh eremoved_ = std::move(sm.eremoved_); fremoved_ = std::move(sm.fremoved_); vpoint_ = std::move(sm.vpoint_); - removed_vertices_ = sd::exchange(sm.removed_vertices, 0); - removed_edges_ = sd::exchange(sm.removed_edges, 0); - removed_faces_ = sd::exchange(sm.removed_faces, 0); - vertices_freelist_ = std::exchange(sm.vertices_freelist_, (std::numeric_limits::max)();) - edgces_freelist_ = std::exchange(sm.edges_freelist_,(std::numeric_limits::max)();) - faces_freelist_ = std::exchange(sm.faces_freelist_,(std::numeric_limits::max)();) + removed_vertices_ = std::exchange(sm.removed_vertices, 0); + removed_edges_ = std::exchange(sm.removed_edges, 0); + removed_faces_ = std::exchange(sm.removed_faces, 0); + vertices_freelist_ = std::exchange(sm.vertices_freelist_, (std::numeric_limits::max)()); + edges_freelist_ = std::exchange(sm.edges_freelist_,(std::numeric_limits::max)()); + faces_freelist_ = std::exchange(sm.faces_freelist_,(std::numeric_limits::max)()); garbage_ = std::exchange(sm.garbage_, false); recycle_ = std::exchange(sm.recycle_, true); anonymous_property_ = std::exchange(sm.anonymous_property_, 0); From 514e4de9b7868e2a2b4b8e88c85113b1ad6e9aec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Tue, 19 Apr 2022 07:18:31 +0200 Subject: [PATCH 03/11] add no-move copy --- Surface_mesh/include/CGAL/Surface_mesh/Properties.h | 10 +++++++++- Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h | 7 ++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/Surface_mesh/include/CGAL/Surface_mesh/Properties.h b/Surface_mesh/include/CGAL/Surface_mesh/Properties.h index 71f3306fc5e1..b4d50aa4dba4 100644 --- a/Surface_mesh/include/CGAL/Surface_mesh/Properties.h +++ b/Surface_mesh/include/CGAL/Surface_mesh/Properties.h @@ -572,7 +572,15 @@ class Property_map_base : parray_(std::exchange(pm.parray_, nullptr)) {} - Property_map_base(const Property_map_base& pm) = default; + Property_map_base(const Property_map_base& pm) + : parray_(pm.parray_) + {} + + Property_map_base& operator=(const Property_map_base& pm) + { + parray_ = pm.parray_; + } + void reset() { parray_ = nullptr; diff --git a/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h b/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h index 3810b53d1be2..dd36aae862b2 100644 --- a/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h +++ b/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h @@ -347,8 +347,13 @@ class Surface_mesh Property_map(const Property_map& pm) = default; Property_map(Property_map&& pm) noexcept - : Base(pm) + : Base(std::forward(pm)) {} + + Property_map& operator=(const Property_map& pm) + { + static_cast(*this) = static_cast(pm); + } }; template From 5098cad84173ea7b85353801282875ed86ca9d13 Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Tue, 19 Apr 2022 16:52:18 +0100 Subject: [PATCH 04/11] Test the move semantics --- .../include/CGAL/Surface_mesh/Properties.h | 1 + .../include/CGAL/Surface_mesh/Surface_mesh.h | 14 ++++++++------ Surface_mesh/test/Surface_mesh/SM_common.h | 2 +- .../test/Surface_mesh/surface_mesh_test.cpp | 14 ++++++++++++++ 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/Surface_mesh/include/CGAL/Surface_mesh/Properties.h b/Surface_mesh/include/CGAL/Surface_mesh/Properties.h index b4d50aa4dba4..94a4292fb427 100644 --- a/Surface_mesh/include/CGAL/Surface_mesh/Properties.h +++ b/Surface_mesh/include/CGAL/Surface_mesh/Properties.h @@ -579,6 +579,7 @@ class Property_map_base Property_map_base& operator=(const Property_map_base& pm) { parray_ = pm.parray_; + return *this; } void reset() diff --git a/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h b/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h index dd36aae862b2..d574baa6156a 100644 --- a/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h +++ b/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h @@ -353,6 +353,7 @@ class Surface_mesh Property_map& operator=(const Property_map& pm) { static_cast(*this) = static_cast(pm); + return *this; } }; @@ -937,9 +938,9 @@ class Surface_mesh , eremoved_(std::move(sm.eremoved_)) , fremoved_(std::move(sm.fremoved_)) , vpoint_(std::move(sm.vpoint_)) - , removed_vertices_(std::exchange(sm.removed_vertices, 0)) - , removed_edges_(std::exchange(sm.removed_edges, 0)) - , removed_faces_(std::exchange(sm.removed_faces, 0)) + , removed_vertices_(std::exchange(sm.removed_vertices_, 0)) + , removed_edges_(std::exchange(sm.removed_edges_, 0)) + , removed_faces_(std::exchange(sm.removed_faces_, 0)) , vertices_freelist_(std::exchange(sm.vertices_freelist_,(std::numeric_limits::max)())) , edges_freelist_(std::exchange(sm.edges_freelist_,(std::numeric_limits::max)())) , faces_freelist_(std::exchange(sm.faces_freelist_,(std::numeric_limits::max)())) @@ -965,15 +966,16 @@ class Surface_mesh eremoved_ = std::move(sm.eremoved_); fremoved_ = std::move(sm.fremoved_); vpoint_ = std::move(sm.vpoint_); - removed_vertices_ = std::exchange(sm.removed_vertices, 0); - removed_edges_ = std::exchange(sm.removed_edges, 0); - removed_faces_ = std::exchange(sm.removed_faces, 0); + removed_vertices_ = std::exchange(sm.removed_vertices_, 0); + removed_edges_ = std::exchange(sm.removed_edges_, 0); + removed_faces_ = std::exchange(sm.removed_faces_, 0); vertices_freelist_ = std::exchange(sm.vertices_freelist_, (std::numeric_limits::max)()); edges_freelist_ = std::exchange(sm.edges_freelist_,(std::numeric_limits::max)()); faces_freelist_ = std::exchange(sm.faces_freelist_,(std::numeric_limits::max)()); garbage_ = std::exchange(sm.garbage_, false); recycle_ = std::exchange(sm.recycle_, true); anonymous_property_ = std::exchange(sm.anonymous_property_, 0); + return *this; } /// assigns `rhs` to `*this`. Does not copy custom properties. diff --git a/Surface_mesh/test/Surface_mesh/SM_common.h b/Surface_mesh/test/Surface_mesh/SM_common.h index a0099d9ef953..ada97bc221f1 100644 --- a/Surface_mesh/test/Surface_mesh/SM_common.h +++ b/Surface_mesh/test/Surface_mesh/SM_common.h @@ -38,7 +38,7 @@ struct Surface_fixture { f3 = m.add_face(v,x,y); } - Sm m; + Sm m, m2,m3; Sm::Vertex_index u, v, w, x, y; Sm::Face_index f1, f2, f3; diff --git a/Surface_mesh/test/Surface_mesh/surface_mesh_test.cpp b/Surface_mesh/test/Surface_mesh/surface_mesh_test.cpp index 949d2538bff6..80c54f6e029c 100644 --- a/Surface_mesh/test/Surface_mesh/surface_mesh_test.cpp +++ b/Surface_mesh/test/Surface_mesh/surface_mesh_test.cpp @@ -230,6 +230,19 @@ void properties () { assert(created == false); } +void move () { + Surface_fixture f; + + int nf = num_faces(f.m); + f.m2 = f.m; + + assert(num_faces(f.m2) == nf); + + f.m3 = std::move(f.m); + assert(num_faces(f.m3) == nf); + assert(num_faces(f.m) == 0); +} + int main() { @@ -244,6 +257,7 @@ int main() border_vertex_check(); point_position_accessor(); properties(); + move(); std::cout << "done" << std::endl; return 0; } From 3c83a2bb6fea427fb73eef980c5d000a10420408 Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Tue, 19 Apr 2022 18:58:18 +0100 Subject: [PATCH 05/11] Use move semantcs in the Polyhedron demo --- .../Plugins/PMP/Join_and_split_polyhedra_plugin.cpp | 2 +- Polyhedron/demo/Polyhedron/Scene_surface_mesh_item.cpp | 8 +++++++- Polyhedron/demo/Polyhedron/Scene_surface_mesh_item.h | 3 ++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Polyhedron/demo/Polyhedron/Plugins/PMP/Join_and_split_polyhedra_plugin.cpp b/Polyhedron/demo/Polyhedron/Plugins/PMP/Join_and_split_polyhedra_plugin.cpp index 4a4710cdb2de..7753a38eb9d4 100644 --- a/Polyhedron/demo/Polyhedron/Plugins/PMP/Join_and_split_polyhedra_plugin.cpp +++ b/Polyhedron/demo/Polyhedron/Plugins/PMP/Join_and_split_polyhedra_plugin.cpp @@ -168,7 +168,7 @@ void Polyhedron_demo_join_and_split_polyhedra_plugin::on_actionSplitPolyhedra_tr scene->addItem(group); for(FaceGraph& poly : new_polyhedra) { - Scene_facegraph_item* new_item=new Scene_facegraph_item(poly); + Scene_facegraph_item* new_item=new Scene_facegraph_item(std::move(poly)); new_item->setName(tr("%1 - CC %2").arg(item->name()).arg(cc)); new_item->setColor(color_map[cc]); ++cc; diff --git a/Polyhedron/demo/Polyhedron/Scene_surface_mesh_item.cpp b/Polyhedron/demo/Polyhedron/Scene_surface_mesh_item.cpp index 7e5d45a15b27..1a40394e27b8 100644 --- a/Polyhedron/demo/Polyhedron/Scene_surface_mesh_item.cpp +++ b/Polyhedron/demo/Polyhedron/Scene_surface_mesh_item.cpp @@ -51,6 +51,7 @@ #include "id_printing.h" #include #include +#include #endif typedef CGAL::Three::Triangle_container Tri; @@ -350,11 +351,16 @@ Scene_surface_mesh_item::Scene_surface_mesh_item(SMesh* sm) standard_constructor(sm); } -Scene_surface_mesh_item::Scene_surface_mesh_item(SMesh sm) +Scene_surface_mesh_item::Scene_surface_mesh_item(const SMesh& sm) { standard_constructor(new SMesh(sm)); } +Scene_surface_mesh_item::Scene_surface_mesh_item(SMesh&& sm) +{ + standard_constructor(new SMesh(std::forward(sm))); +} + Scene_surface_mesh_item* Scene_surface_mesh_item::clone() const { return new Scene_surface_mesh_item(*this); } diff --git a/Polyhedron/demo/Polyhedron/Scene_surface_mesh_item.h b/Polyhedron/demo/Polyhedron/Scene_surface_mesh_item.h index d04f771899b3..68ffb958dfb9 100644 --- a/Polyhedron/demo/Polyhedron/Scene_surface_mesh_item.h +++ b/Polyhedron/demo/Polyhedron/Scene_surface_mesh_item.h @@ -48,7 +48,8 @@ class SCENE_SURFACE_MESH_ITEM_EXPORT Scene_surface_mesh_item Scene_surface_mesh_item(); // Takes ownership of the argument. Scene_surface_mesh_item(SMesh*); - Scene_surface_mesh_item(SMesh); + Scene_surface_mesh_item(const SMesh&); + Scene_surface_mesh_item(SMesh&&); Scene_surface_mesh_item(const Scene_surface_mesh_item& other); ~Scene_surface_mesh_item(); From 9e964b07979df7f735e0146f7135c9ccd7c81eba Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Wed, 20 Apr 2022 17:03:31 +0200 Subject: [PATCH 06/11] Simplify constructors with default member initializers --- Surface_mesh/include/CGAL/Surface_mesh/Properties.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Surface_mesh/include/CGAL/Surface_mesh/Properties.h b/Surface_mesh/include/CGAL/Surface_mesh/Properties.h index 94a4292fb427..f24a50fb2038 100644 --- a/Surface_mesh/include/CGAL/Surface_mesh/Properties.h +++ b/Surface_mesh/include/CGAL/Surface_mesh/Properties.h @@ -234,7 +234,7 @@ class Property_container public: // default constructor - Property_container() : size_(0), capacity_(0) {} + Property_container() = default; // destructor (deletes all property arrays) virtual ~Property_container() { clear(); } @@ -243,7 +243,6 @@ class Property_container Property_container(const Property_container& _rhs) { operator=(_rhs); } Property_container(Property_container&& c) noexcept - : size_(0), capacity_(0) { c.swap(*this); } @@ -513,8 +512,8 @@ class Property_container private: std::vector parrays_; - size_t size_; - size_t capacity_; + size_t size_ = 0; + size_t capacity_ = 0; }; /// @endcond From 644b88924df86e8b80e17bc9028fd9d7f1cc1da6 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Wed, 20 Apr 2022 17:03:58 +0200 Subject: [PATCH 07/11] Rewrite the tetst --- Surface_mesh/test/Surface_mesh/SM_common.h | 2 +- .../test/Surface_mesh/surface_mesh_test.cpp | 33 +++++++++++++++---- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/Surface_mesh/test/Surface_mesh/SM_common.h b/Surface_mesh/test/Surface_mesh/SM_common.h index ada97bc221f1..a0099d9ef953 100644 --- a/Surface_mesh/test/Surface_mesh/SM_common.h +++ b/Surface_mesh/test/Surface_mesh/SM_common.h @@ -38,7 +38,7 @@ struct Surface_fixture { f3 = m.add_face(v,x,y); } - Sm m, m2,m3; + Sm m; Sm::Vertex_index u, v, w, x, y; Sm::Face_index f1, f2, f3; diff --git a/Surface_mesh/test/Surface_mesh/surface_mesh_test.cpp b/Surface_mesh/test/Surface_mesh/surface_mesh_test.cpp index 80c54f6e029c..5112583bb8ec 100644 --- a/Surface_mesh/test/Surface_mesh/surface_mesh_test.cpp +++ b/Surface_mesh/test/Surface_mesh/surface_mesh_test.cpp @@ -233,14 +233,35 @@ void properties () { void move () { Surface_fixture f; - int nf = num_faces(f.m); - f.m2 = f.m; + auto nf = num_faces(f.m); - assert(num_faces(f.m2) == nf); - - f.m3 = std::move(f.m); - assert(num_faces(f.m3) == nf); + // test move-constructor + Sm m2{std::move(f.m)}; + assert(f.m.is_valid()); + assert(m2.is_valid()); + assert(num_faces(m2) == nf); assert(num_faces(f.m) == 0); + + // test move-assignment + f.m = std::move(m2); + assert(f.m.is_valid()); + assert(m2.is_valid()); + assert(num_faces(f.m) == nf); + assert(num_faces(m2) == 0); + + // test copy-assignment + m2 = f.m; + assert(f.m.is_valid()); + assert(m2.is_valid()); + assert(num_faces(f.m) == nf); + assert(num_faces(m2) == nf); + + // test copy-assignment + m2 = f.m; + assert(f.m.is_valid()); + assert(m2.is_valid()); + assert(num_faces(f.m) == nf); + assert(num_faces(m2) == nf); } From 6c08c98a4e04135b6ddd5e3723fe3cab3a3b87a9 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Wed, 20 Apr 2022 17:04:13 +0200 Subject: [PATCH 08/11] Use std::move instead of std::forward --- Polyhedron/demo/Polyhedron/Scene_surface_mesh_item.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Polyhedron/demo/Polyhedron/Scene_surface_mesh_item.cpp b/Polyhedron/demo/Polyhedron/Scene_surface_mesh_item.cpp index 1a40394e27b8..bf2ceccb0f71 100644 --- a/Polyhedron/demo/Polyhedron/Scene_surface_mesh_item.cpp +++ b/Polyhedron/demo/Polyhedron/Scene_surface_mesh_item.cpp @@ -358,7 +358,7 @@ Scene_surface_mesh_item::Scene_surface_mesh_item(const SMesh& sm) Scene_surface_mesh_item::Scene_surface_mesh_item(SMesh&& sm) { - standard_constructor(new SMesh(std::forward(sm))); + standard_constructor(new SMesh(std::move(sm))); } Scene_surface_mesh_item* From 090c61c6b78feb2c37a5f6b993ba12f192745d8e Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Wed, 20 Apr 2022 17:08:05 +0200 Subject: [PATCH 09/11] Swap capacity_ as well --- Surface_mesh/include/CGAL/Surface_mesh/Properties.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Surface_mesh/include/CGAL/Surface_mesh/Properties.h b/Surface_mesh/include/CGAL/Surface_mesh/Properties.h index f24a50fb2038..9fc752bb6639 100644 --- a/Surface_mesh/include/CGAL/Surface_mesh/Properties.h +++ b/Surface_mesh/include/CGAL/Surface_mesh/Properties.h @@ -507,7 +507,8 @@ class Property_container void swap (Property_container& other) { this->parrays_.swap (other.parrays_); - std::swap(this->size_, other.size_); // AF: why not the same for capacity_ ? + std::swap(this->size_, other.size_); + std::swap(this->capacity_, other.capacity_); } private: From df43db067a4b2ca3658cbf6b7673242ba594bd04 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Wed, 20 Apr 2022 17:09:35 +0200 Subject: [PATCH 10/11] Use std::move instead of std::forward --- Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h b/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h index d574baa6156a..1813483f032f 100644 --- a/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h +++ b/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h @@ -347,7 +347,7 @@ class Surface_mesh Property_map(const Property_map& pm) = default; Property_map(Property_map&& pm) noexcept - : Base(std::forward(pm)) + : Base(std::move(pm)) {} Property_map& operator=(const Property_map& pm) From 3072b9394289a54b078e0a06ff1c34f85f639ed9 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Sat, 23 Apr 2022 16:11:18 +0200 Subject: [PATCH 11/11] After review --- .../include/CGAL/Surface_mesh/Surface_mesh.h | 14 +------------- .../test/Surface_mesh/surface_mesh_test.cpp | 4 ++-- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h b/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h index 1813483f032f..580d8b6819c5 100644 --- a/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h +++ b/Surface_mesh/include/CGAL/Surface_mesh/Surface_mesh.h @@ -341,20 +341,8 @@ class Surface_mesh { typedef Properties::Property_map_base > Base; typedef typename Base::reference reference; - Property_map() : Base() {} + Property_map() = default; Property_map(const Base& pm): Base(pm) {} - - Property_map(const Property_map& pm) = default; - - Property_map(Property_map&& pm) noexcept - : Base(std::move(pm)) - {} - - Property_map& operator=(const Property_map& pm) - { - static_cast(*this) = static_cast(pm); - return *this; - } }; template diff --git a/Surface_mesh/test/Surface_mesh/surface_mesh_test.cpp b/Surface_mesh/test/Surface_mesh/surface_mesh_test.cpp index 5112583bb8ec..16fb699982c5 100644 --- a/Surface_mesh/test/Surface_mesh/surface_mesh_test.cpp +++ b/Surface_mesh/test/Surface_mesh/surface_mesh_test.cpp @@ -256,8 +256,8 @@ void move () { assert(num_faces(f.m) == nf); assert(num_faces(m2) == nf); - // test copy-assignment - m2 = f.m; + // test copy-constructor + Sm m3 {f.m}; assert(f.m.is_valid()); assert(m2.is_valid()); assert(num_faces(f.m) == nf);