Skip to content

Commit

Permalink
Revert "Fix potential unsafe initialization in the Graph class (#606)"
Browse files Browse the repository at this point in the history
This reverts commit 17585a9.

Signed-off-by: Steve Peters <[email protected]>
  • Loading branch information
scpeters committed Jul 17, 2024
1 parent 187f10b commit 797da92
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 53 deletions.
14 changes: 0 additions & 14 deletions Migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

### 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
Expand Down
18 changes: 2 additions & 16 deletions include/gz/math/graph/Edge.hh
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include <cstdint>
#include <functional>
#include <map>
#include <memory>
#include <ostream>
#include <set>

Expand Down Expand Up @@ -206,8 +205,7 @@ namespace graph
class UndirectedEdge : public Edge<E>
{
/// \brief An invalid undirected edge.
// Deprecated in favor of NullEdge().
public: static UndirectedEdge<E> GZ_DEPRECATED(8) NullEdge;
public: static UndirectedEdge<E> NullEdge;

/// \brief Constructor.
/// \param[in] _vertices The vertices of the edge.
Expand Down Expand Up @@ -259,7 +257,6 @@ namespace graph
};

/// \brief An invalid undirected edge.
// Deprecated in favor of NullEdge().
template<typename E>
UndirectedEdge<E> UndirectedEdge<E>::NullEdge(
VertexId_P(kNullId, kNullId), E(), 1.0, kNullId);
Expand All @@ -271,8 +268,7 @@ namespace graph
class DirectedEdge : public Edge<E>
{
/// \brief An invalid directed edge.
// Deprecated in favor of NullEdge().
public: static DirectedEdge<E> GZ_DEPRECATED(8) NullEdge;
public: static DirectedEdge<E> NullEdge;

/// \brief Constructor.
/// \param[in] _vertices The vertices of the edge.
Expand Down Expand Up @@ -336,19 +332,9 @@ namespace graph
};

/// \brief An invalid directed edge.
// Deprecated in favor of NullEdge().
template<typename E>
DirectedEdge<E> DirectedEdge<E>::NullEdge(
VertexId_P(kNullId, kNullId), E(), 1.0, kNullId);

/// \brief An invalid edge.
template<typename E, typename EdgeType>
EdgeType &NullEdge()
{
static auto e = std::make_unique<EdgeType>(
VertexId_P(kNullId, kNullId), E(), 1.0, kNullId);
return *e;
}
}
}
}
Expand Down
20 changes: 10 additions & 10 deletions include/gz/math/graph/Graph.hh
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ namespace graph
{
std::cerr << "[Graph::AddVertex()] The limit of vertices has been "
<< "reached. Ignoring vertex." << std::endl;
return NullVertex<V>();
return Vertex<V>::NullVertex;
}
}

Expand All @@ -163,7 +163,7 @@ namespace graph
{
std::cerr << "[Graph::AddVertex()] Repeated vertex [" << id << "]"
<< std::endl;
return NullVertex<V>();
return Vertex<V>::NullVertex;
}

// Link the vertex with an empty list of edges.
Expand Down Expand Up @@ -219,7 +219,7 @@ namespace graph
{
std::cerr << "[Graph::AddEdge()] The limit of edges has been reached. "
<< "Ignoring edge." << std::endl;
return NullEdge<E, EdgeType>();
return EdgeType::NullEdge;
}

EdgeType newEdge(_vertices, _data, _weight, id);
Expand All @@ -240,7 +240,7 @@ namespace graph
for (auto const &v : {edgeVertices.first, edgeVertices.second})
{
if (this->vertices.find(v) == this->vertices.end())
return NullEdge<E, EdgeType>();
return EdgeType::NullEdge;
}

// Link the new edge.
Expand Down Expand Up @@ -611,7 +611,7 @@ namespace graph
{
auto iter = this->vertices.find(_id);
if (iter == this->vertices.end())
return NullVertex<V>();
return Vertex<V>::NullVertex;

return iter->second;
}
Expand All @@ -624,7 +624,7 @@ namespace graph
{
auto iter = this->vertices.find(_id);
if (iter == this->vertices.end())
return NullVertex<V>();
return Vertex<V>::NullVertex;

return iter->second;
}
Expand All @@ -646,7 +646,7 @@ namespace graph

// Quit early if there is no adjacency entry
if (adjIt == this->adjList.end())
return NullEdge<E, EdgeType>();
return EdgeType::NullEdge;

// Loop over the edges in the source vertex's adjacency list
for (std::set<EdgeId>::const_iterator edgIt = adjIt->second.begin();
Expand All @@ -665,7 +665,7 @@ namespace graph
}
}

return NullEdge<E, EdgeType>();
return EdgeType::NullEdge;
}

/// \brief Get a reference to an edge using its Id.
Expand All @@ -676,7 +676,7 @@ namespace graph
{
auto iter = this->edges.find(_id);
if (iter == this->edges.end())
return NullEdge<E, EdgeType>();
return EdgeType::NullEdge;

return iter->second;
}
Expand All @@ -689,7 +689,7 @@ namespace graph
{
auto iter = this->edges.find(_id);
if (iter == this->edges.end())
return NullEdge<E, EdgeType>();
return EdgeType::NullEdge;

return iter->second;
}
Expand Down
15 changes: 2 additions & 13 deletions include/gz/math/graph/Vertex.hh
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include <cstdint>
#include <functional>
#include <map>
#include <memory>
#include <ostream>
#include <string>
#include <utility>
Expand All @@ -46,7 +45,7 @@ namespace graph
using VertexId_P = std::pair<VertexId, VertexId>;

/// \brief Represents an invalid Id.
constexpr VertexId kNullId = MAX_UI64;
static const 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
Expand All @@ -55,8 +54,7 @@ namespace graph
class Vertex
{
/// \brief An invalid vertex.
// Deprecated in favor of NullVertex().
public: static Vertex<V> GZ_DEPRECATED(8) NullVertex;
public: static Vertex<V> NullVertex;

/// \brief Constructor.
/// \param[in] _name Non-unique vertex name.
Expand Down Expand Up @@ -138,18 +136,9 @@ namespace graph
};

/// \brief An invalid vertex.
// Deprecated in favor of NullVertex().
template<typename V>
Vertex<V> Vertex<V>::NullVertex("__null__", V(), kNullId);

/// \brief An invalid vertex.
template<typename V>
Vertex<V> &NullVertex()
{
static auto v = std::make_unique<Vertex<V>>("__null__", V(), kNullId);
return *v;
}

/// \def VertexRef_M
/// \brief Map of vertices. The key is the vertex Id. The value is a
/// reference to the vertex.
Expand Down

0 comments on commit 797da92

Please sign in to comment.