Merge pull request #525 from Becheler/fix/subgraph-124-ci

fix: undefined behavior when transforming non-existing edges in a subgraph

Author: Becheler

doc/modules/ROOT/pages/adaptors/subgraph.adoc

@@ -30,18 +30,47 @@ template <typename Graph>
 class subgraph;
 ----
 
-The `subgraph` class wraps an `adjacency_list` and maintains a tree of
-subgraphs. The root graph owns all vertices and edges. Child subgraphs hold
-subsets of the root's vertices, and their edge sets are _induced_: any edge in
-the root whose both endpoints are in the child automatically appears in the
-child.
-
-Each subgraph uses _local_ descriptors (0-based within that subgraph). Use
-`local_to_global()` and `global_to_local()` to convert between local and root
-descriptors.
-
-Adding an edge to a child subgraph also adds it to all ancestor subgraphs.
-Adding a vertex to a child subgraph also adds it to all ancestors.
+`subgraph` represents parts of a single graph as independent graphs, without
+copying the data and without the parts drifting out of sync with the whole. All
+vertices and edges are stored once, in the root graph. Each subgraph is a
+lightweight view onto a subset of that storage which still behaves like a full
+graph, so it can be passed to any BGL algorithm.
+
+[TIP]
+====
+Consider a national road network: intersections are vertices and roads are
+edges. An analysis might focus on one city, then one district within that city,
+then compare against the whole country. Rather than three separate copies kept
+consistent by hand, `subgraph` keeps one dataset and offers lightweight views
+onto its parts. The country is the root, a city is a child subgraph, and a
+district is a child of that city. Each view behaves like a normal graph and can
+be handed to an algorithm such as `dijkstra_shortest_paths` exactly like any
+other BGL graph.
+====
+
+These views nest into a tree. The _root_ subgraph holds the entire graph and
+owns all of its vertices and edges. Each _child_ is a subset of its parent's
+vertices, and a child may in turn have children of its own. A child's edges are
+_induced_: an edge belongs to a child exactly when both of its endpoints do, so
+only vertices are ever added to a child, never edges directly.
+
+Because the storage is shared rather than copied, the levels stay linked in both
+directions. A property set at one level, such as an edge weight or a vertex
+color, is visible from every level. Adding a vertex or edge to a child also adds
+it to every ancestor up to the root, so an edit made while working on one part
+keeps the whole hierarchy consistent.
+
+Each subgraph numbers its own vertices and edges 0-based, as required by the
+many algorithms that assume a contiguous index space. A descriptor is therefore
+meaningful only within one subgraph. `local_to_global()` and `global_to_local()`
+translate a descriptor between a subgraph's local numbering and the root's.
+
+Choosing among the graph views:
+
+* `filtered_graph` for a read-only view that hides some vertices or edges.
+* `copy_graph` for an independent copy that does not stay linked to the original.
+* `subgraph` when mutability, per-region local numbering, and a parent/child
+  hierarchy over shared storage are all needed.
 
 == Template Parameters
 
@@ -102,16 +131,49 @@ edge_descriptor global_to_local(edge_descriptor e_global) const;
 
 Convert between local (subgraph-relative) and global (root) descriptors.
 
+[WARNING]
+====
+These conversions have preconditions, and the two directions differ.
+
+* `global_to_local` requires that the element is in this subgraph. In debug
+builds a violation trips `BOOST_ASSERT`; with assertions disabled it returns a
+default-constructed descriptor (`null_vertex()` for vertices). When the element
+might not be present, query membership first with `find_vertex()` or
+`find_edge()`.
+* `local_to_global` requires a valid local descriptor of this subgraph. A
+violation is undefined behaviour (an unchecked lookup) in all build
+configurations.
+
+On the root subgraph every conversion is the identity, so any element is
+accepted.
+====
+
 '''
 
 [source,cpp]
 ----
 std::pair<vertex_descriptor, bool>
 find_vertex(vertex_descriptor u_global) const;
+
+std::pair<edge_descriptor, bool>
+find_edge(edge_descriptor e_global) const;
 ----
 
-Returns `(local_descriptor, true)` if the global vertex is in this subgraph,
-`(_, false)` otherwise.
+Membership queries that never assert. `find_vertex` returns
+`(local_descriptor, true)` if the global vertex is in this subgraph, and
+`(null_vertex(), false)` otherwise. `find_edge` behaves the same way for edges,
+returning a default-constructed `edge_descriptor` in the not-found case. These
+are the safe form of `global_to_local`, for the case where the element is not
+known in advance to be in the subgraph.
+
+[NOTE]
+====
+*Descriptor identity and stability.* For an element that is present, the
+conversions round-trip: `global_to_local(local_to_global(x)) == x`, and the
+edge index is preserved. Properties live in the root's storage and are shared
+across the whole tree, so the same logical element carries the same property
+value no matter which subgraph's descriptor reaches it.
+====
 
 === Hierarchy Navigation
 

include/boost/graph/subgraph.hpp

@@ -207,12 +207,14 @@ template < typename Graph > class subgraph
 
     vertex_descriptor global_to_local(vertex_descriptor u_global) const
     {
-        vertex_descriptor u_local;
-        bool in_subgraph;
         if (is_root())
             return u_global;
+        vertex_descriptor u_local;
+        bool in_subgraph;
         boost::tie(u_local, in_subgraph) = this->find_vertex(u_global);
-        BOOST_ASSERT(in_subgraph == true);
+        BOOST_ASSERT_MSG(in_subgraph,
+            "global_to_local: vertex is not in this subgraph. "
+            "Use find_vertex() to check membership first.");
         return u_local;
     }
 
@@ -225,10 +227,15 @@ template < typename Graph > class subgraph
 
     edge_descriptor global_to_local(edge_descriptor e_global) const
     {
-        return is_root() ? e_global
-                         : (*m_local_edge.find(
-                                get(get(edge_index, root().m_graph), e_global)))
-                               .second;
+        if (is_root())
+            return e_global;
+        edge_descriptor e_local;
+        bool in_subgraph;
+        boost::tie(e_local, in_subgraph) = this->find_edge(e_global);
+        BOOST_ASSERT_MSG(in_subgraph,
+            "global_to_local: edge is not in this subgraph. "
+            "Use find_edge() to check membership first.");
+        return e_local;
     }
 
     // Is vertex u (of the root graph) contained in this subgraph?

test/Jamfile.v2

@@ -97,6 +97,7 @@ alias graph_test_regular :
     [ run subgraph_bundled.cpp ]
     [ run subgraph_add.cpp : $(TEST_DIR) ]
     [ run subgraph_props.cpp ]
+    [ run subgraph_global_local.cpp ]
 
     [ run isomorphism.cpp ]
     [ run adjacency_matrix_test.cpp ]

test/subgraph_global_local.cpp

@@ -0,0 +1,141 @@
+//  Copyright (c) 2026 Arnaud Becheler
+//  Distributed under the Boost Software License, Version 1.0. (See
+//  accompanying file LICENSE_1_0.txt or copy at
+//  http://www.boost.org/LICENSE_1_0.txt)
+
+#include <boost/core/lightweight_test.hpp>
+#include <boost/graph/adjacency_list.hpp>
+#include <boost/graph/subgraph.hpp>
+
+#include <utility>
+
+using namespace boost;
+
+template < typename Directedness >
+using subgraph_of = subgraph< adjacency_list< vecS, vecS, Directedness,
+    property< vertex_color_t, int >,
+    property< edge_index_t, std::size_t, property< edge_weight_t, int > > > >;
+
+template < typename Directedness >
+void test_descriptor_conversion()
+{
+    using graph_t = subgraph_of< Directedness >;
+    using vertex_t = typename graph_traits< graph_t >::vertex_descriptor;
+    using edge_t = typename graph_traits< graph_t >::edge_descriptor;
+    using vertex_iterator = typename graph_traits< graph_t >::vertex_iterator;
+    using edge_iterator = typename graph_traits< graph_t >::edge_iterator;
+
+    // root: path 0-1-2-3, with edge indices assigned in insertion order.
+    graph_t root(4);
+    add_edge(0, 1, root); // edge index 0
+    add_edge(1, 2, root); // edge index 1
+    add_edge(2, 3, root); // edge index 2
+
+    // child g1 = {1, 2, 3}: induces edges (1,2) and (2,3), but not (0,1).
+    graph_t& g1 = root.create_subgraph();
+    add_vertex(1, g1);
+    add_vertex(2, g1);
+    add_vertex(3, g1);
+
+    // grandchild g1a = {2, 3}: induces only edge (2,3).
+    graph_t& g1a = g1.create_subgraph();
+    add_vertex(2, g1a);
+    add_vertex(3, g1a);
+
+    // invariant: for an edge in the subgraph, global_to_local(local_to_global(e))
+    // must equal e, and its edge_index must be preserved.
+    edge_iterator ei, ei_end;
+    for (boost::tie(ei, ei_end) = edges(g1); ei != ei_end; ++ei)
+    {
+        edge_t e_local = *ei;
+        edge_t e_round = g1.global_to_local(g1.local_to_global(e_local));
+        BOOST_TEST(e_round == e_local);
+        BOOST_TEST(get(edge_index, g1, e_local) == get(edge_index, g1, e_round));
+    }
+
+    // invariant: for a vertex in the subgraph, global_to_local(local_to_global(v))
+    // must equal v, and its vertex_index must be preserved.
+    vertex_iterator vi, vi_end;
+    for (boost::tie(vi, vi_end) = vertices(g1); vi != vi_end; ++vi)
+    {
+        vertex_t v_local = *vi;
+        vertex_t v_round = g1.global_to_local(g1.local_to_global(v_local));
+        BOOST_TEST(v_round == v_local);
+        BOOST_TEST(get(vertex_index, g1, v_local) == get(vertex_index, g1, v_round));
+    }
+
+    // invariant: a property written through a child must be readable from the
+    // root, because the whole tree shares one property store.
+    edge_t some_local = *edges(g1).first;
+    put(edge_weight, g1, some_local, 42);
+    BOOST_TEST(get(edge_weight, root, g1.local_to_global(some_local)) == 42);
+
+    // invariant: find_vertex must report a present vertex as true, and an absent
+    // one as false with a null_vertex() descriptor.
+    BOOST_TEST(g1.find_vertex(1).second); // vertex 1 is in g1
+    BOOST_TEST(!g1.find_vertex(0).second); // vertex 0 is not in g1
+    BOOST_TEST(g1.find_vertex(0).first == graph_traits< graph_t >::null_vertex());
+
+    // invariant: find_edge must report false with a default descriptor for a
+    // root edge that is absent from the subgraph.
+    edge_t e01 = edge(0, 1, root).first;
+    BOOST_TEST(get(edge_index, root, e01) == 0u);
+    BOOST_TEST(g1.find_edge(e01).first == edge_t()); // absent -> default
+    BOOST_TEST(!g1.find_edge(e01).second); // absent -> false
+
+    // invariant: edge round-trip identity must hold at any nesting depth, 
+    // not just for direct children.
+    for (boost::tie(ei, ei_end) = edges(g1a); ei != ei_end; ++ei)
+    {
+        edge_t e_local = *ei;
+        BOOST_TEST(g1a.global_to_local(g1a.local_to_global(e_local)) == e_local);
+    }
+
+    // invariant: a property written through a grandchild must reach the root.
+    edge_t g1a_local = *edges(g1a).first;
+    put(edge_weight, g1a, g1a_local, 7);
+    BOOST_TEST(get(edge_weight, root, g1a.local_to_global(g1a_local)) == 7);
+
+    // invariant: an edge present in the parent but not the grandchild must
+    // report absent from the grandchild.
+    edge_t e12 = edge(1, 2, root).first;
+    BOOST_TEST(!g1a.find_edge(e12).second);
+}
+
+// idiom: find_edge is the membership query; global_to_local is the transform to
+// run once membership is confirmed.
+template < typename Directedness >
+void test_membership_query_then_convert()
+{
+    using graph_t = subgraph_of< Directedness >;
+    using edge_t = typename graph_traits< graph_t >::edge_descriptor;
+
+    graph_t root(3);
+    add_edge(0, 1, root); // edge index 0
+    add_edge(1, 2, root); // edge index 1
+
+    // sg = {1, 2}: induces edge (1,2), but not edge (0,1).
+    graph_t& sg = root.create_subgraph();
+    add_vertex(1, sg);
+    add_vertex(2, sg);
+
+    // Present edge: the query confirms membership, then the transform agrees
+    // with the local descriptor the query returned.
+    edge_t e12 = edge(1, 2, root).first;
+    std::pair< edge_t, bool > found = sg.find_edge(e12);
+    BOOST_TEST(found.second);
+    BOOST_TEST(sg.global_to_local(e12) == found.first);
+
+    // Absent edge: the query reports it, so the transform is not run.
+    edge_t e01 = edge(0, 1, root).first;
+    BOOST_TEST(!sg.find_edge(e01).second);
+}
+
+int main()
+{
+    test_descriptor_conversion< directedS >();
+    test_descriptor_conversion< bidirectionalS >();
+    test_membership_query_then_convert< directedS >();
+    test_membership_query_then_convert< bidirectionalS >();
+    return boost::report_errors();
+}