From be6caa9ebd2c1d3cfd69a171972bbc33d1edb5d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Ag=C3=BCero?= Date: Tue, 2 Jul 2024 22:53:39 +0200 Subject: [PATCH 1/5] Avoid constructor/destructor fiascos in graph class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Carlos Agüero --- include/gz/math/graph/Edge.hh | 24 +++++++++--------------- include/gz/math/graph/Graph.hh | 20 ++++++++++---------- include/gz/math/graph/Vertex.hh | 12 +++++++----- 3 files changed, 26 insertions(+), 30 deletions(-) diff --git a/include/gz/math/graph/Edge.hh b/include/gz/math/graph/Edge.hh index 92f4b937c..48b8a4ed7 100644 --- a/include/gz/math/graph/Edge.hh +++ b/include/gz/math/graph/Edge.hh @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -204,9 +205,6 @@ namespace graph template class UndirectedEdge : public Edge { - /// \brief An invalid undirected edge. - public: static UndirectedEdge NullEdge; - /// \brief Constructor. /// \param[in] _vertices The vertices of the edge. /// \param[in] _data The data stored in the edge. @@ -256,20 +254,12 @@ namespace graph } }; - /// \brief An invalid undirected edge. - template - UndirectedEdge UndirectedEdge::NullEdge( - VertexId_P(kNullId, kNullId), E(), 1.0, kNullId); - /// \brief A directed edge represents a connection between two vertices. /// The connection is unidirectional, it's only possible to traverse the edge /// in one direction (from the tail to the head). template class DirectedEdge : public Edge { - /// \brief An invalid directed edge. - public: static DirectedEdge NullEdge; - /// \brief Constructor. /// \param[in] _vertices The vertices of the edge. /// \param[in] _data The data stored in the edge. @@ -331,10 +321,14 @@ namespace graph } }; - /// \brief An invalid directed edge. - template - DirectedEdge DirectedEdge::NullEdge( - VertexId_P(kNullId, kNullId), E(), 1.0, kNullId); + /// \brief An invalid edge. + template + EdgeType &NullEdge() + { + static auto e = std::make_unique( + VertexId_P(kNullId, kNullId), E(), 1.0, kNullId); + return *e; + } } } } diff --git a/include/gz/math/graph/Graph.hh b/include/gz/math/graph/Graph.hh index 0b48ea2dc..ec61cb078 100644 --- a/include/gz/math/graph/Graph.hh +++ b/include/gz/math/graph/Graph.hh @@ -150,7 +150,7 @@ namespace graph { std::cerr << "[Graph::AddVertex()] The limit of vertices has been " << "reached. Ignoring vertex." << std::endl; - return Vertex::NullVertex; + return NullVertex(); } } @@ -163,7 +163,7 @@ namespace graph { std::cerr << "[Graph::AddVertex()] Repeated vertex [" << id << "]" << std::endl; - return Vertex::NullVertex; + return NullVertex(); } // Link the vertex with an empty list of edges. @@ -219,7 +219,7 @@ namespace graph { std::cerr << "[Graph::AddEdge()] The limit of edges has been reached. " << "Ignoring edge." << std::endl; - return EdgeType::NullEdge; + return NullEdge(); } EdgeType newEdge(_vertices, _data, _weight, id); @@ -240,7 +240,7 @@ namespace graph for (auto const &v : {edgeVertices.first, edgeVertices.second}) { if (this->vertices.find(v) == this->vertices.end()) - return EdgeType::NullEdge; + return NullEdge(); } // Link the new edge. @@ -611,7 +611,7 @@ namespace graph { auto iter = this->vertices.find(_id); if (iter == this->vertices.end()) - return Vertex::NullVertex; + return NullVertex(); return iter->second; } @@ -624,7 +624,7 @@ namespace graph { auto iter = this->vertices.find(_id); if (iter == this->vertices.end()) - return Vertex::NullVertex; + return NullVertex(); return iter->second; } @@ -646,7 +646,7 @@ namespace graph // Quit early if there is no adjacency entry if (adjIt == this->adjList.end()) - return EdgeType::NullEdge; + return NullEdge(); // Loop over the edges in the source vertex's adjacency list for (std::set::const_iterator edgIt = adjIt->second.begin(); @@ -665,7 +665,7 @@ namespace graph } } - return EdgeType::NullEdge; + return NullEdge(); } /// \brief Get a reference to an edge using its Id. @@ -676,7 +676,7 @@ namespace graph { auto iter = this->edges.find(_id); if (iter == this->edges.end()) - return EdgeType::NullEdge; + return NullEdge(); return iter->second; } @@ -689,7 +689,7 @@ namespace graph { auto iter = this->edges.find(_id); if (iter == this->edges.end()) - return EdgeType::NullEdge; + return NullEdge(); return iter->second; } diff --git a/include/gz/math/graph/Vertex.hh b/include/gz/math/graph/Vertex.hh index e61fa22df..5132fb0fd 100644 --- a/include/gz/math/graph/Vertex.hh +++ b/include/gz/math/graph/Vertex.hh @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -45,7 +46,7 @@ namespace graph using VertexId_P = std::pair; /// \brief Represents an invalid Id. - static const VertexId kNullId = MAX_UI64; + constexpr VertexId kNullId = MAX_UI64; /// \brief A vertex of a graph. It stores user information, an optional name, /// and keeps an internal unique Id. This class does not enforce to choose a @@ -53,9 +54,6 @@ namespace graph template class Vertex { - /// \brief An invalid vertex. - public: static Vertex NullVertex; - /// \brief Constructor. /// \param[in] _name Non-unique vertex name. /// \param[in] _data User information. @@ -137,7 +135,11 @@ namespace graph /// \brief An invalid vertex. template - Vertex Vertex::NullVertex("__null__", V(), kNullId); + Vertex &NullVertex() + { + static auto v = std::make_unique>("__null__", V(), kNullId); + return *v; + } /// \def VertexRef_M /// \brief Map of vertices. The key is the vertex Id. The value is a From 15b8975131bdef5f44c7932f4d120a15d21eca64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Ag=C3=BCero?= Date: Thu, 11 Jul 2024 21:37:54 +0200 Subject: [PATCH 2/5] Deprecations and migration guide. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Carlos Agüero --- Migration.md | 14 ++++++++++++++ include/gz/math/graph/Edge.hh | 20 ++++++++++++++++++++ include/gz/math/graph/Vertex.hh | 9 +++++++++ 3 files changed, 43 insertions(+) diff --git a/Migration.md b/Migration.md index e5444ebd5..8bd35f880 100644 --- a/Migration.md +++ b/Migration.md @@ -5,6 +5,20 @@ Deprecated code produces compile-time warnings. These warning serve as notification to users that their code should be upgraded. The next major release will remove the deprecated code. +## Gazebo Math 8.X to 9.X + +### Deprecations + +1. **graph/Vertex.hh** + + The `Vertex::NullVertex` static member is deprecated. Please use + `Vertex::NullVertex()` instead. + E.g.: https://github.com/gazebosim/gz-math/pull/606/files#diff-0c0220a7e72be70337975433eeddc3f5e072ade5cd80dfb1ac03da233c39c983L153-R153 + +1. **graph/Edge.hh** + + The `Edge::NullEdge` static member is deprecated. Please use + `Vertex::NullEdge()` instead. + E.g.: https://github.com/gazebosim/gz-math/pull/606/files#diff-0c0220a7e72be70337975433eeddc3f5e072ade5cd80dfb1ac03da233c39c983L222-R222 + ## Gazebo Math 7.X to 8.X ### Breaking Changes diff --git a/include/gz/math/graph/Edge.hh b/include/gz/math/graph/Edge.hh index 48b8a4ed7..61e4e1bec 100644 --- a/include/gz/math/graph/Edge.hh +++ b/include/gz/math/graph/Edge.hh @@ -205,6 +205,10 @@ namespace graph template class UndirectedEdge : public Edge { + /// \brief An invalid undirected edge. + // Deprecated in favor of NullEdge(). + public: static UndirectedEdge GZ_DEPRECATED(8) NullEdge; + /// \brief Constructor. /// \param[in] _vertices The vertices of the edge. /// \param[in] _data The data stored in the edge. @@ -254,12 +258,22 @@ namespace graph } }; + /// \brief An invalid undirected edge. + // Deprecated in favor of NullEdge(). + template + UndirectedEdge UndirectedEdge::NullEdge( + VertexId_P(kNullId, kNullId), E(), 1.0, kNullId); + /// \brief A directed edge represents a connection between two vertices. /// The connection is unidirectional, it's only possible to traverse the edge /// in one direction (from the tail to the head). template class DirectedEdge : public Edge { + /// \brief An invalid directed edge. + // Deprecated in favor of NullEdge(). + public: static DirectedEdge GZ_DEPRECATED(8) NullEdge; + /// \brief Constructor. /// \param[in] _vertices The vertices of the edge. /// \param[in] _data The data stored in the edge. @@ -321,6 +335,12 @@ namespace graph } }; + /// \brief An invalid directed edge. + // Deprecated in favor of NullEdge(). + template + DirectedEdge DirectedEdge::NullEdge( + VertexId_P(kNullId, kNullId), E(), 1.0, kNullId); + /// \brief An invalid edge. template EdgeType &NullEdge() diff --git a/include/gz/math/graph/Vertex.hh b/include/gz/math/graph/Vertex.hh index 5132fb0fd..db0092ed0 100644 --- a/include/gz/math/graph/Vertex.hh +++ b/include/gz/math/graph/Vertex.hh @@ -54,6 +54,10 @@ namespace graph template class Vertex { + /// \brief An invalid vertex. + // Deprecated in favor of NullVertex(). + public: static Vertex GZ_DEPRECATED(8) NullVertex; + /// \brief Constructor. /// \param[in] _name Non-unique vertex name. /// \param[in] _data User information. @@ -133,6 +137,11 @@ namespace graph private: VertexId id = kNullId; }; + /// \brief An invalid vertex. + // Deprecated in favor of NullVertex(). + template + Vertex Vertex::NullVertex("__null__", V(), kNullId); + /// \brief An invalid vertex. template Vertex &NullVertex() From 5f606f0b6acfc11ec94ad077f7ea7229cd8ee44e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Ag=C3=BCero?= Date: Thu, 11 Jul 2024 21:58:09 +0200 Subject: [PATCH 3/5] Breaking changes. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Carlos Agüero --- Migration.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Migration.md b/Migration.md index 8bd35f880..e292683d6 100644 --- a/Migration.md +++ b/Migration.md @@ -7,6 +7,18 @@ release will remove the deprecated code. ## Gazebo Math 8.X to 9.X +### Breaking Changes + +1. **graph/Graph.hh** + + The functions `Graph::AddVertex()` and `Graph::VertexFromId()` might + return NullVertex() instead of NullVertex. + E.g.: https://github.com/gazebosim/gz-math/pull/606/files#diff-0c0220a7e72be70337975433eeddc3f5e072ade5cd80dfb1ac03da233c39c983L153-R153 + + + The functions `Graph::AddEdge()`, `Graph::LinkEdge()`, + `Graph::EdgeFromVertices()` and `Graph::EdgeFromId()` functions might + return NullEdge() instead of NullEdge. + E.g.: https://github.com/gazebosim/gz-math/pull/606/files#diff-0c0220a7e72be70337975433eeddc3f5e072ade5cd80dfb1ac03da233c39c983L222-R222 + ### Deprecations 1. **graph/Vertex.hh** From 3ea9feb132567e9e922c20e9f25851d7b32b9a0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Ag=C3=BCero?= Date: Fri, 12 Jul 2024 14:22:14 +0200 Subject: [PATCH 4/5] Update Migration.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Steve Peters Signed-off-by: Carlos Agüero --- Migration.md | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/Migration.md b/Migration.md index e292683d6..3a18602f5 100644 --- a/Migration.md +++ b/Migration.md @@ -5,20 +5,6 @@ Deprecated code produces compile-time warnings. These warning serve as notification to users that their code should be upgraded. The next major release will remove the deprecated code. -## Gazebo Math 8.X to 9.X - -### Breaking Changes - -1. **graph/Graph.hh** - + The functions `Graph::AddVertex()` and `Graph::VertexFromId()` might - return NullVertex() instead of NullVertex. - E.g.: https://github.com/gazebosim/gz-math/pull/606/files#diff-0c0220a7e72be70337975433eeddc3f5e072ade5cd80dfb1ac03da233c39c983L153-R153 - - + The functions `Graph::AddEdge()`, `Graph::LinkEdge()`, - `Graph::EdgeFromVertices()` and `Graph::EdgeFromId()` functions might - return NullEdge() instead of NullEdge. - E.g.: https://github.com/gazebosim/gz-math/pull/606/files#diff-0c0220a7e72be70337975433eeddc3f5e072ade5cd80dfb1ac03da233c39c983L222-R222 - ### Deprecations 1. **graph/Vertex.hh** From a08852b63b240b6df56477f07ec6b14f7f259bde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Ag=C3=BCero?= Date: Fri, 12 Jul 2024 14:23:50 +0200 Subject: [PATCH 5/5] Tweak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Carlos Agüero --- Migration.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Migration.md b/Migration.md index 3a18602f5..8bd35f880 100644 --- a/Migration.md +++ b/Migration.md @@ -5,6 +5,8 @@ Deprecated code produces compile-time warnings. These warning serve as notification to users that their code should be upgraded. The next major release will remove the deprecated code. +## Gazebo Math 8.X to 9.X + ### Deprecations 1. **graph/Vertex.hh**