From 805e2135f09480ce748f7192bc0b1a20ba51be5b Mon Sep 17 00:00:00 2001 From: clockblocker Date: Fri, 6 Nov 2026 10:03:57 +0100 Subject: [PATCH 1/2] Test copy assignment of rfl::CopyableBox rfl::CopyableBox is documented as copyable, but only its copy constructor works. Copy-assigning a CopyableBox, or a struct that holds one directly, does not compile: error: no viable overloaded '=' error: object of type 'DecisionTree::Node' cannot be assigned because its copy assignment operator is implicitly deleted This test does not build until the next commit fixes it. Co-Authored-By: Claude Opus 5.5 --- tests/generic/test_box.cpp | 42 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/generic/test_box.cpp b/tests/generic/test_box.cpp index e91fae931..99f79508a 100644 --- a/tests/generic/test_box.cpp +++ b/tests/generic/test_box.cpp @@ -85,4 +85,46 @@ TEST(generic, copyable_box) { EXPECT_NE(lesser_leaf_tree1.value, lesser_leaf_tree2.value); } + +TEST(generic, copyable_box_copy_assignment) { + const auto make_leaf = [](const double _value) { + return rfl::make_copyable_box( + DecisionTree{DecisionTree::Leaf{.value = _value}}); + }; + + const auto value_of = [](const rfl::CopyableBox& _box) { + return rfl::get(_box->leaf_or_node.get().variant()) + .value; + }; + + const auto box1 = make_leaf(3.0); + auto box2 = make_leaf(5.0); + + box2 = box1; + + EXPECT_EQ(value_of(box2), 3.0); + EXPECT_NE(box1.get(), box2.get()); + + const DecisionTree::Node node1{.critical_value = 10.0, + .lesser = make_leaf(1.0), + .greater = make_leaf(2.0)}; + auto node2 = DecisionTree::Node{.critical_value = 20.0, + .lesser = make_leaf(4.0), + .greater = make_leaf(5.0)}; + + node2 = node1; + + EXPECT_EQ(node2.critical_value.get(), 10.0); + EXPECT_EQ(value_of(node2.lesser), 1.0); + EXPECT_EQ(value_of(node2.greater), 2.0); + EXPECT_NE(node1.lesser.get(), node2.lesser.get()); + + // The source may be part of the value it replaces. + auto root = rfl::make_copyable_box( + DecisionTree{.leaf_or_node = node1}); + root = rfl::get(root->leaf_or_node.get().variant()) + .lesser; + + EXPECT_EQ(value_of(root), 1.0); +} } // namespace test_copyable_box From b97c3b7fe0453897b7b5896ae727b86d60460724 Mon Sep 17 00:00:00 2001 From: clockblocker Date: Fri, 6 Nov 2026 10:07:18 +0100 Subject: [PATCH 2/2] Make rfl::CopyableBox copy-assignable The copy assignment operator for Copyability::COPYABLE took a `const Box&`. Inside the class template, `Box` names `Box`, so for a CopyableBox this was an assignment from the other Box type, not a copy assignment operator. The class declares its own copy assignment (`operator=(const Box&) requires NON_COPYABLE = delete`), so the compiler does not add an implicit one either. A CopyableBox therefore had no usable copy assignment, and neither did any struct that holds a CopyableBox directly. The cross-type operator never compiled when called: `this != &other` compares pointers to distinct types. Take `const Box&` instead. The new object is still copied before the old one is destroyed, so assigning from a Box inside the replaced object (`root = root->...lesser`) is safe. This makes the test from the previous commit pass. Co-Authored-By: Claude Opus 5.5 --- include/rfl/Box.hpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/rfl/Box.hpp b/include/rfl/Box.hpp index e06552203..12765d746 100644 --- a/include/rfl/Box.hpp +++ b/include/rfl/Box.hpp @@ -83,9 +83,12 @@ class Box { T* get() const { return ptr_.get(); } /// Copy assignment operator - only available when C == Copyability::COPYABLE. + /// Replaces the contained object with a deep copy of the other one. The copy + /// is made before the old object is destroyed, so the other Box may be part + /// of the object it replaces. /// @param other The Box to copy from /// @return Reference to this Box - Box& operator=(const Box& other) requires (C == Copyability::COPYABLE) { + Box& operator=(const Box& other) requires (C == Copyability::COPYABLE) { if(this != &other) { ptr_ = std::make_unique(*other); }