From bfc6252d85ecb45a38bb4c8728ffe3487c2098a1 Mon Sep 17 00:00:00 2001 From: Arnaud Becheler <8360330+Becheler@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:57:32 +0200 Subject: [PATCH] feature: stateful visitors for shortest paths algorithms --- .../shortest_paths/bellman_ford_shortest.adoc | 9 ++- .../shortest_paths/dag_shortest_paths.adoc | 9 ++- .../dijkstra_shortest_paths_no_color_map.adoc | 2 +- .../graph/bellman_ford_shortest_paths.hpp | 13 +++-- include/boost/graph/dag_shortest_paths.hpp | 15 +++-- .../dijkstra_shortest_paths_no_color_map.hpp | 21 ++++--- test/bellman-test.cpp | 57 ++++++++++++++++++ test/dag_longest_paths.cpp | 58 +++++++++++++++++++ test/dijkstra_no_color_map_compare.cpp | 52 +++++++++++++++++ 9 files changed, 206 insertions(+), 30 deletions(-) diff --git a/doc/modules/ROOT/pages/algorithms/shortest_paths/bellman_ford_shortest.adoc b/doc/modules/ROOT/pages/algorithms/shortest_paths/bellman_ford_shortest.adoc index b82912806..cb04bed13 100644 --- a/doc/modules/ROOT/pages/algorithms/shortest_paths/bellman_ford_shortest.adoc +++ b/doc/modules/ROOT/pages/algorithms/shortest_paths/bellman_ford_shortest.adoc @@ -310,8 +310,7 @@ for an example of using the Bellman-Ford algorithm. == Notes -[#1]#[1]# Since the visitor parameter is passed by value, if your -visitor contains state then any changes to the state during the algorithm -will be made to a copy of the visitor object, not the visitor object -passed in. Therefore you may want the visitor to hold this state by -pointer or reference. +[#1]#[1]# The visitor is taken by value, so the algorithm works on a copy. +To keep state, give the visitor ordinary data members and pass it with +`std::ref`. The algorithm then operates on the referenced object and its +state survives the call. diff --git a/doc/modules/ROOT/pages/algorithms/shortest_paths/dag_shortest_paths.adoc b/doc/modules/ROOT/pages/algorithms/shortest_paths/dag_shortest_paths.adoc index ff876b269..f248dc390 100644 --- a/doc/modules/ROOT/pages/algorithms/shortest_paths/dag_shortest_paths.adoc +++ b/doc/modules/ROOT/pages/algorithms/shortest_paths/dag_shortest_paths.adoc @@ -301,8 +301,7 @@ for an example of using this algorithm. == Notes -[#1]#[1]# Since the visitor parameter is passed by value, if -your visitor contains state then any changes to the state during the -algorithm will be made to a copy of the visitor object, not the visitor -object passed in. Therefore you may want the visitor to hold this state -by pointer or reference. +[#1]#[1]# The visitor is taken by value, so the algorithm works on a copy. +To keep state, give the visitor ordinary data members and pass it with +`std::ref`. The algorithm then operates on the referenced object and its +state survives the call. diff --git a/doc/modules/ROOT/pages/algorithms/shortest_paths/dijkstra_shortest_paths_no_color_map.adoc b/doc/modules/ROOT/pages/algorithms/shortest_paths/dijkstra_shortest_paths_no_color_map.adoc index 003695889..1e318a3ea 100644 --- a/doc/modules/ROOT/pages/algorithms/shortest_paths/dijkstra_shortest_paths_no_color_map.adoc +++ b/doc/modules/ROOT/pages/algorithms/shortest_paths/dijkstra_shortest_paths_no_color_map.adoc @@ -197,7 +197,7 @@ finish vertex u | OUT | `visitor(DijkstraVisitor v)` -| Use this to specify actions that you would like to happen during certain event points within the algorithm. The type `DijkstraVisitor` must be a model of the xref:visitors/DijkstraVisitor.adoc[Dijkstra Visitor] concept. The visitor object is passed by value. Default: `dijkstra_visitor` +| Use this to specify actions that you would like to happen during certain event points within the algorithm. The type `DijkstraVisitor` must be a model of the xref:visitors/DijkstraVisitor.adoc[Dijkstra Visitor] concept. The visitor is taken by value, so the algorithm works on a copy. To keep state, give the visitor ordinary data members and pass it with `std::ref`. The algorithm then operates on the referenced object and its state survives the call. Default: `dijkstra_visitor` |=== == Complexity diff --git a/include/boost/graph/bellman_ford_shortest_paths.hpp b/include/boost/graph/bellman_ford_shortest_paths.hpp index 2aa4e8083..e56246fe8 100644 --- a/include/boost/graph/bellman_ford_shortest_paths.hpp +++ b/include/boost/graph/bellman_ford_shortest_paths.hpp @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -101,6 +102,8 @@ bool bellman_ford_shortest_paths(EdgeListGraph& g, Size N, WeightMap weight, BOOST_CONCEPT_ASSERT((ReadWritePropertyMapConcept< DistanceMap, Vertex >)); BOOST_CONCEPT_ASSERT((ReadablePropertyMapConcept< WeightMap, Edge >)); + auto& v_ref = ::boost::graph::detail::deref_visitor(v); + typename GTraits::edge_iterator i, end; for (Size k = 0; k < N; ++k) @@ -108,14 +111,14 @@ bool bellman_ford_shortest_paths(EdgeListGraph& g, Size N, WeightMap weight, bool at_least_one_edge_relaxed = false; for (boost::tie(i, end) = edges(g); i != end; ++i) { - v.examine_edge(*i, g); + v_ref.examine_edge(*i, g); if (relax(*i, g, weight, pred, distance, combine, compare)) { at_least_one_edge_relaxed = true; - v.edge_relaxed(*i, g); + v_ref.edge_relaxed(*i, g); } else - v.edge_not_relaxed(*i, g); + v_ref.edge_not_relaxed(*i, g); } if (!at_least_one_edge_relaxed) break; @@ -125,11 +128,11 @@ bool bellman_ford_shortest_paths(EdgeListGraph& g, Size N, WeightMap weight, if (compare(combine(get(distance, source(*i, g)), get(weight, *i)), get(distance, target(*i, g)))) { - v.edge_not_minimized(*i, g); + v_ref.edge_not_minimized(*i, g); return false; } else - v.edge_minimized(*i, g); + v_ref.edge_minimized(*i, g); return true; } diff --git a/include/boost/graph/dag_shortest_paths.hpp b/include/boost/graph/dag_shortest_paths.hpp index a0aecc169..1f3d3d594 100644 --- a/include/boost/graph/dag_shortest_paths.hpp +++ b/include/boost/graph/dag_shortest_paths.hpp @@ -10,6 +10,7 @@ #ifndef BOOST_GRAPH_DAG_SHORTEST_PATHS_HPP #define BOOST_GRAPH_DAG_SHORTEST_PATHS_HPP +#include #include #include @@ -48,25 +49,27 @@ inline void dag_shortest_paths(const VertexListGraph& g, put(pred, *ui, *ui); } + auto& vis_ref = ::boost::graph::detail::deref_visitor(vis); + put(distance, s, zero); - vis.discover_vertex(s, g); + vis_ref.discover_vertex(s, g); typename std::vector< Vertex >::reverse_iterator i; for (i = rev_topo_order.rbegin(); i != rev_topo_order.rend(); ++i) { Vertex u = *i; - vis.examine_vertex(u, g); + vis_ref.examine_vertex(u, g); typename graph_traits< VertexListGraph >::out_edge_iterator e, e_end; for (boost::tie(e, e_end) = out_edges(u, g); e != e_end; ++e) { - vis.discover_vertex(target(*e, g), g); + vis_ref.discover_vertex(target(*e, g), g); bool decreased = relax(*e, g, weight, pred, distance, combine, compare); if (decreased) - vis.edge_relaxed(*e, g); + vis_ref.edge_relaxed(*e, g); else - vis.edge_not_relaxed(*e, g); + vis_ref.edge_not_relaxed(*e, g); } - vis.finish_vertex(u, g); + vis_ref.finish_vertex(u, g); } } diff --git a/include/boost/graph/dijkstra_shortest_paths_no_color_map.hpp b/include/boost/graph/dijkstra_shortest_paths_no_color_map.hpp index 54224ae04..824097d32 100644 --- a/include/boost/graph/dijkstra_shortest_paths_no_color_map.hpp +++ b/include/boost/graph/dijkstra_shortest_paths_no_color_map.hpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -58,18 +59,20 @@ void dijkstra_shortest_paths_no_color_map_no_init(const Graph& graph, graph, index_map, index_in_heap_map_holder); VertexQueue vertex_queue(distance_map, index_in_heap, distance_compare); + auto& visitor_ref = ::boost::graph::detail::deref_visitor(visitor); + // Add vertex to the queue vertex_queue.push(start_vertex); // Starting vertex will always be the first discovered vertex - visitor.discover_vertex(start_vertex, graph); + visitor_ref.discover_vertex(start_vertex, graph); while (!vertex_queue.empty()) { Vertex min_vertex = vertex_queue.top(); vertex_queue.pop(); - visitor.examine_vertex(min_vertex, graph); + visitor_ref.examine_vertex(min_vertex, graph); // Check if any other vertices can be reached Distance min_vertex_distance = get(distance_map, min_vertex); @@ -83,7 +86,7 @@ void dijkstra_shortest_paths_no_color_map_no_init(const Graph& graph, // Examine neighbors of min_vertex BGL_FORALL_OUTEDGES_T(min_vertex, current_edge, graph, Graph) { - visitor.examine_edge(current_edge, graph); + visitor_ref.examine_edge(current_edge, graph); // Check if the edge has a negative weight if (distance_compare(get(weight_map, current_edge), distance_zero)) @@ -105,10 +108,10 @@ void dijkstra_shortest_paths_no_color_map_no_init(const Graph& graph, if (was_edge_relaxed) { - visitor.edge_relaxed(current_edge, graph); + visitor_ref.edge_relaxed(current_edge, graph); if (is_neighbor_undiscovered) { - visitor.discover_vertex(neighbor_vertex, graph); + visitor_ref.discover_vertex(neighbor_vertex, graph); vertex_queue.push(neighbor_vertex); } else @@ -118,12 +121,12 @@ void dijkstra_shortest_paths_no_color_map_no_init(const Graph& graph, } else { - visitor.edge_not_relaxed(current_edge, graph); + visitor_ref.edge_not_relaxed(current_edge, graph); } } // end out edge iteration - visitor.finish_vertex(min_vertex, graph); + visitor_ref.finish_vertex(min_vertex, graph); } // end while queue not empty } @@ -141,10 +144,12 @@ void dijkstra_shortest_paths_no_color_map(const Graph& graph, DistanceInfinity distance_infinity, DistanceZero distance_zero, DijkstraVisitor visitor) { + auto& visitor_ref = ::boost::graph::detail::deref_visitor(visitor); + // Initialize vertices BGL_FORALL_VERTICES_T(current_vertex, graph, Graph) { - visitor.initialize_vertex(current_vertex, graph); + visitor_ref.initialize_vertex(current_vertex, graph); // Default all distances to infinity put(distance_map, current_vertex, distance_infinity); diff --git a/test/bellman-test.cpp b/test/bellman-test.cpp index 94e5332b2..70238d16e 100644 --- a/test/bellman-test.cpp +++ b/test/bellman-test.cpp @@ -20,6 +20,61 @@ B: 2147483647 B #include #include +#include +#include +#include +#include + +// state in a plain data member, so it survives only through std::ref +struct relaxed_tally : boost::bellman_visitor<> +{ + template < class Edge, class Graph > void edge_relaxed(Edge, Graph&) + { + ++count; + } + std::size_t count = 0; +}; + +void test_stateful_visitor_with_ref() +{ + using graph_t = boost::adjacency_list< boost::vecS, boost::vecS, + boost::directedS, boost::no_property, + boost::property< boost::edge_weight_t, int > >; + graph_t g(3); + boost::add_edge(0, 1, 1, g); + boost::add_edge(1, 2, 1, g); + + // bellman_ford does not initialise, the caller seeds the distances + std::vector< std::size_t > parent(boost::num_vertices(g)); + for (std::size_t i = 0; i < parent.size(); ++i) + parent[i] = i; + std::vector< int > distance( + boost::num_vertices(g), (std::numeric_limits< int >::max)()); + distance[0] = 0; + + auto index_map = boost::get(boost::vertex_index, g); + auto parent_map + = boost::make_iterator_property_map(parent.begin(), index_map); + auto distance_map + = boost::make_iterator_property_map(distance.begin(), index_map); + + relaxed_tally tracked; + BOOST_TEST(boost::bellman_ford_shortest_paths(g, + static_cast< int >(boost::num_vertices(g)), + boost::get(boost::edge_weight, g), parent_map, distance_map, + boost::closed_plus< int >(), std::less< int >(), std::ref(tracked))); + BOOST_TEST(tracked.count > 0u); + BOOST_TEST_EQ(distance[2], 2); + + // by value the caller's visitor is left untouched + relaxed_tally copied; + BOOST_TEST(boost::bellman_ford_shortest_paths(g, + static_cast< int >(boost::num_vertices(g)), + boost::get(boost::edge_weight, g), parent_map, distance_map, + boost::closed_plus< int >(), std::less< int >(), copied)); + BOOST_TEST_EQ(copied.count, static_cast< std::size_t >(0)); +} + int main(int, char*[]) { using namespace boost; @@ -120,5 +175,7 @@ int main(int, char*[]) } #endif + test_stateful_visitor_with_ref(); + return boost::report_errors(); } diff --git a/test/dag_longest_paths.cpp b/test/dag_longest_paths.cpp index f45f8aa4c..fde81374a 100644 --- a/test/dag_longest_paths.cpp +++ b/test/dag_longest_paths.cpp @@ -9,11 +9,67 @@ #include #include +#include +#include +#include +#include +#include + using namespace boost; #include using namespace std; +// state in a plain data member, so it survives only through std::ref +struct examine_tally : boost::dijkstra_visitor<> +{ + template < class Vertex, class Graph > void examine_vertex(Vertex, Graph&) + { + ++count; + } + std::size_t count = 0; +}; + +void test_stateful_visitor_with_ref() +{ + using graph_t = boost::adjacency_list< boost::vecS, boost::vecS, + boost::directedS, boost::no_property, + boost::property< boost::edge_weight_t, int > >; + graph_t g(3); + boost::add_edge(0, 1, 1, g); + boost::add_edge(1, 2, 1, g); + + std::vector< int > distance(boost::num_vertices(g)); + std::vector< std::size_t > parent(boost::num_vertices(g)); + std::vector< boost::default_color_type > color(boost::num_vertices(g)); + + auto index_map = boost::get(boost::vertex_index, g); + auto distance_map + = boost::make_iterator_property_map(distance.begin(), index_map); + auto parent_map + = boost::make_iterator_property_map(parent.begin(), index_map); + auto color_map + = boost::make_iterator_property_map(color.begin(), index_map); + + // every vertex is reachable from 0, so each one is examined once + examine_tally tracked; + boost::dag_shortest_paths(g, 0, distance_map, + boost::get(boost::edge_weight, g), color_map, parent_map, + std::ref(tracked), std::less< int >(), boost::closed_plus< int >(), + (std::numeric_limits< int >::max)(), 0); + BOOST_TEST_EQ(tracked.count, boost::num_vertices(g)); + BOOST_TEST_EQ(distance[2], 2); + + // by value the caller's visitor is left untouched + std::fill(color.begin(), color.end(), boost::white_color); + examine_tally copied; + boost::dag_shortest_paths(g, 0, distance_map, + boost::get(boost::edge_weight, g), color_map, parent_map, copied, + std::less< int >(), boost::closed_plus< int >(), + (std::numeric_limits< int >::max)(), 0); + BOOST_TEST_EQ(copied.count, static_cast< std::size_t >(0)); +} + int main(int, char*[]) { typedef adjacency_list< vecS, vecS, directedS, no_property, @@ -50,5 +106,7 @@ int main(int, char*[]) BOOST_TEST(distance[2] == 2); + test_stateful_visitor_with_ref(); + return boost::report_errors(); } diff --git a/test/dijkstra_no_color_map_compare.cpp b/test/dijkstra_no_color_map_compare.cpp index 5cd9863aa..c07a7237b 100644 --- a/test/dijkstra_no_color_map_compare.cpp +++ b/test/dijkstra_no_color_map_compare.cpp @@ -27,6 +27,11 @@ #include #include +#include +#include +#include +#include + #define INITIALIZE_VERTEX 0 #define DISCOVER_VERTEX 1 #define EXAMINE_VERTEX 2 @@ -74,6 +79,51 @@ template < typename Graph > void run_dijkstra_test(const Graph& graph) no_color_map_vertex_double_map.begin())); } +// state in a plain data member, so it survives only through std::ref +struct discover_tally : boost::dijkstra_visitor<> +{ + template < class Vertex, class Graph > void discover_vertex(Vertex, Graph&) + { + ++count; + } + std::size_t count = 0; +}; + +void test_stateful_visitor_with_ref() +{ + using graph_t = boost::adjacency_list< boost::vecS, boost::vecS, + boost::directedS, boost::no_property, + boost::property< boost::edge_weight_t, int > >; + graph_t g(3); + boost::add_edge(0, 1, 1, g); + boost::add_edge(1, 2, 1, g); + + std::vector< int > distance(boost::num_vertices(g)); + std::vector< std::size_t > parent(boost::num_vertices(g)); + + auto index_map = boost::get(boost::vertex_index, g); + auto distance_map + = boost::make_iterator_property_map(distance.begin(), index_map); + auto parent_map + = boost::make_iterator_property_map(parent.begin(), index_map); + + discover_tally tracked; + boost::dijkstra_shortest_paths_no_color_map(g, boost::vertex(0, g), + parent_map, distance_map, boost::get(boost::edge_weight, g), index_map, + std::less< int >(), boost::closed_plus< int >(), + (std::numeric_limits< int >::max)(), 0, std::ref(tracked)); + BOOST_TEST(tracked.count > 0u); + BOOST_TEST_EQ(distance[2], 2); + + // by value the caller's visitor is left untouched + discover_tally copied; + boost::dijkstra_shortest_paths_no_color_map(g, boost::vertex(0, g), + parent_map, distance_map, boost::get(boost::edge_weight, g), index_map, + std::less< int >(), boost::closed_plus< int >(), + (std::numeric_limits< int >::max)(), 0, copied); + BOOST_TEST_EQ(copied.count, static_cast< std::size_t >(0)); +} + int main(int argc, char* argv[]) { using namespace boost; @@ -122,5 +172,7 @@ int main(int argc, char* argv[]) run_dijkstra_test(graph); + test_stateful_visitor_with_ref(); + return boost::report_errors(); }