feature: stateful visitors for shortest paths algorithms

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.

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.

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<null_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<null_visitor>`
 |===
 
 == Complexity

include/boost/graph/bellman_ford_shortest_paths.hpp

@@ -23,6 +23,7 @@
 
 #include <boost/config.hpp>
 #include <boost/graph/graph_traits.hpp>
+#include <boost/graph/detail/visitor_wrapper.hpp>
 #include <boost/graph/graph_concepts.hpp>
 #include <boost/graph/properties.hpp>
 #include <boost/graph/relax.hpp>
@@ -101,21 +102,23 @@ 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)
     {
         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;
 }

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 <boost/graph/detail/visitor_wrapper.hpp>
 #include <boost/graph/topological_sort.hpp>
 #include <boost/graph/dijkstra_shortest_paths.hpp>
 
@@ -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);
     }
 }
 

include/boost/graph/dijkstra_shortest_paths_no_color_map.hpp

@@ -14,6 +14,7 @@
 #include <boost/pending/indirect_cmp.hpp>
 #include <boost/graph/relax.hpp>
 #include <boost/graph/detail/d_ary_heap.hpp>
+#include <boost/graph/detail/visitor_wrapper.hpp>
 #include <boost/graph/dijkstra_shortest_paths.hpp>
 #include <boost/throw_exception.hpp>
 #include <boost/graph/iteration_macros.hpp>
@@ -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);

test/bellman-test.cpp

@@ -20,6 +20,61 @@ B: 2147483647 B
 #include <boost/cstdlib.hpp>
 #include <boost/core/lightweight_test.hpp>
 
+#include <cstddef>
+#include <functional>
+#include <limits>
+#include <vector>
+
+// 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();
 }

test/dag_longest_paths.cpp

@@ -9,11 +9,67 @@
 #include <boost/property_map/vector_property_map.hpp>
 #include <boost/core/lightweight_test.hpp>
 
+#include <algorithm>
+#include <cstddef>
+#include <functional>
+#include <limits>
+#include <vector>
+
 using namespace boost;
 
 #include <iostream>
 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();
 }