Fixup C++ Graph constructors and assignement operators

dwave/optimization/include/dwave-optimization/graph.hpp

@@ -71,8 +71,18 @@ struct Decision {
 
 class Graph {
  public:
-    Graph();
-    ~Graph();
+    Graph() noexcept = default;
+    ~Graph() noexcept = default;
+
+    // We disallow copy construction and assignment because it would only be
+    // a shallow copy/assignment in terms of the underlying nodes.
+    // We could implement these in the future if desired but it would be quite
+    // non-trivial.
+    Graph(const Graph&) = delete;
+    Graph& operator=(const Graph&) = delete;
+
+    Graph(Graph&&) noexcept = default;
+    Graph& operator=(Graph&&) noexcept = default;
 
     template <class NodeType, class... Args>
     NodeType* emplace_node(Args&&... args);
@@ -177,7 +187,8 @@ class Graph {
 
     std::vector<std::unique_ptr<Node>> nodes_;
 
-    // The nodes with important semantic meanings to the model
+    // The nodes with important semantic meanings to the model.
+    // All of these pointers are non-owning!
     ArrayNode* objective_ptr_ = nullptr;
     std::vector<ArrayNode*> constraints_;
     std::vector<DecisionNode*> decisions_;

dwave/optimization/src/graph.cpp

@@ -26,9 +26,6 @@ namespace dwave::optimization {
 
 // Graph **********************************************************************
 
-Graph::Graph() = default;
-Graph::~Graph() = default;
-
 void Graph::topological_sort() {
     if (topologically_sorted_) return;
 

releasenotes/notes/Graph-constructor-assignment-20de69b40825424b.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - Move C++ ``Graph`` constructor and assignment operator definitions into the ``dwave-optimization/graph.hpp``.
+  - Explicitly disallow copy construction and copy assignment for C++ ``Graph``.

tests/cpp/test_graph.cpp

@@ -174,6 +174,69 @@ TEST_CASE("Topological Sort", "[topological_sort]") {
     // test edge cases.
 }
 
+TEST_CASE("Graph constructors, assignment operators, and swapping") {
+    static_assert(std::is_nothrow_default_constructible<Graph>::value);
+    static_assert(!std::is_copy_constructible<Graph>::value);
+    static_assert(!std::is_copy_assignable<Graph>::value);
+    static_assert(std::is_nothrow_move_constructible<Graph>::value);
+    static_assert(std::is_nothrow_move_assignable<Graph>::value);
+    static_assert(std::is_swappable<Graph>::value);
+
+    GIVEN("A graph with a few nodes in it") {
+        auto graph = Graph();
+        auto a_ptr = graph.emplace_node<ConstantNode>(1.5);
+        auto b_ptr = graph.emplace_node<ConstantNode>(2);
+        auto c_ptr = graph.emplace_node<AddNode>(a_ptr, b_ptr);
+        graph.set_objective(c_ptr);
+
+        WHEN("We construct another graph using the move constructor") {
+            auto other = Graph(std::move(graph));
+
+            THEN("The nodes have all be moved over") {
+                // these tests are far from complete, but we're using default
+                // constructors/operators so this is intended to be a sanity
+                // check
+                CHECK(other.nodes().size() == 3);
+                CHECK(graph.nodes().size() == 0);
+            }
+        }
+
+        WHEN("We construct another graph using the move assignment operator") {
+            auto other = Graph();
+            other = std::move(graph);
+
+            THEN("The nodes have all be moved over") {
+                // these tests are far from complete, but we're using default
+                // constructors/operators so this is intended to be a sanity
+                // check
+                CHECK(other.nodes().size() == 3);
+                CHECK(graph.nodes().size() == 0);
+            }
+        }
+
+        AND_GIVEN("A different graph with at least one node") {
+            auto other = Graph();
+            auto d_ptr = other.emplace_node<BinaryNode>(std::initializer_list<ssize_t>{1});
+            other.add_constraint(d_ptr);
+
+            WHEN("We swap the graphs") {
+                std::swap(graph, other);
+
+                THEN("Their content have been swapped as expected") {
+                    // these tests are far from complete, but we're using default
+                    // constructors/operators so this is intended to be a sanity
+                    // check
+                    CHECK(other.nodes().size() == 3);
+                    CHECK(other.constraints().size() == 0);
+
+                    CHECK(graph.nodes().size() == 1);
+                    CHECK(graph.constraints().size() == 1);
+                }
+            }
+        }
+    }
+}
+
 TEST_CASE("Graph::objective()") {
     GIVEN("A graph") {
         auto graph = Graph();