From 4fde9dba72271fc478824214d0bdd7ca0564f56a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Jun 2026 18:35:55 +0000 Subject: [PATCH 1/2] Address review comment fixes --- merklecpp.h | 117 +++++++++++++++++----------------------------- test/coverage.cpp | 4 +- 2 files changed, 46 insertions(+), 75 deletions(-) diff --git a/merklecpp.h b/merklecpp.h index 2ef483a..2698ceb 100644 --- a/merklecpp.h +++ b/merklecpp.h @@ -747,22 +747,9 @@ namespace merkle /// @brief Moves a tree /// @param other Tree to move - TreeT(TreeT&& other) noexcept : - leaf_nodes(std::move(other.leaf_nodes)), - uninserted_leaf_nodes(std::move(other.uninserted_leaf_nodes)), - _root(std::move(other._root)), - num_flushed(other.num_flushed), - insertion_stack(std::move(other.insertion_stack)), - hashing_stack(std::move(other.hashing_stack)), - walk_stack(std::move(other.walk_stack)) + TreeT(TreeT&& other) noexcept { - other.leaf_nodes.clear(); - other.uninserted_leaf_nodes.clear(); - other._root = nullptr; - other.num_flushed = 0; - other.insertion_stack.clear(); - other.hashing_stack.clear(); - other.walk_stack.clear(); + move_from(std::move(other)); } /// @brief Deserialises a tree @@ -790,11 +777,7 @@ namespace merkle /// @brief Deconstructor ~TreeT() { - delete (_root); - for (auto n : uninserted_leaf_nodes) - { - delete (n); - } + clear(); } /// @brief Invariant of the tree @@ -981,17 +964,7 @@ namespace merkle { return *this; } - leaf_nodes.clear(); - for (auto n : uninserted_leaf_nodes) - { - delete (n); - } - uninserted_leaf_nodes.clear(); - insertion_stack.clear(); - hashing_stack.clear(); - walk_stack.clear(); - delete (_root); - _root = nullptr; + clear(); size_t to_skip = (other.num_flushed % 2 == 0) ? 0 : 1; _root = Node::copy_node( @@ -1020,33 +993,8 @@ namespace merkle return *this; } - leaf_nodes.clear(); - for (auto n : uninserted_leaf_nodes) - { - delete (n); - } - uninserted_leaf_nodes.clear(); - insertion_stack.clear(); - hashing_stack.clear(); - walk_stack.clear(); - delete (_root); - _root = nullptr; - - leaf_nodes = std::move(other.leaf_nodes); - uninserted_leaf_nodes = std::move(other.uninserted_leaf_nodes); - _root = other._root; - num_flushed = other.num_flushed; - insertion_stack = std::move(other.insertion_stack); - hashing_stack = std::move(other.hashing_stack); - walk_stack = std::move(other.walk_stack); - - other.leaf_nodes.clear(); - other.uninserted_leaf_nodes.clear(); - other._root = nullptr; - other.num_flushed = 0; - other.insertion_stack.clear(); - other.hashing_stack.clear(); - other.walk_stack.clear(); + clear(); + move_from(std::move(other)); return *this; } @@ -1468,17 +1416,7 @@ namespace merkle { MERKLECPP_TRACE(MERKLECPP_TOUT << "> deserialise " << std::endl;); - delete (_root); - leaf_nodes.clear(); - for (auto n : uninserted_leaf_nodes) - { - delete (n); - } - uninserted_leaf_nodes.clear(); - insertion_stack.clear(); - hashing_stack.clear(); - walk_stack.clear(); - _root = nullptr; + clear(); size_t num_leaf_nodes = deserialise_uint64_t(bytes, position); num_flushed = deserialise_uint64_t(bytes, position); @@ -1770,15 +1708,48 @@ namespace merkle protected: void validate_partial_range(size_t from, size_t to) const { - const bool from_out_of_range = from < min_index() || max_index() < from; - const bool to_out_of_range = to < min_index() || max_index() < to; - const bool reversed_range = from > to; - if (empty() || from_out_of_range || to_out_of_range || reversed_range) + if ( + empty() || !(min_index() <= from && from <= to && to <= max_index())) { throw std::runtime_error("invalid leaf indices"); } } + void clear() + { + leaf_nodes.clear(); + for (auto n : uninserted_leaf_nodes) + { + delete (n); + } + uninserted_leaf_nodes.clear(); + insertion_stack.clear(); + hashing_stack.clear(); + walk_stack.clear(); + delete (_root); + _root = nullptr; + num_flushed = 0; + } + + void move_from(TreeT&& other) noexcept + { + leaf_nodes = std::move(other.leaf_nodes); + uninserted_leaf_nodes = std::move(other.uninserted_leaf_nodes); + _root = other._root; + num_flushed = other.num_flushed; + insertion_stack = std::move(other.insertion_stack); + hashing_stack = std::move(other.hashing_stack); + walk_stack = std::move(other.walk_stack); + + other.leaf_nodes.clear(); + other.uninserted_leaf_nodes.clear(); + other._root = nullptr; + other.num_flushed = 0; + other.insertion_stack.clear(); + other.hashing_stack.clear(); + other.walk_stack.clear(); + } + /// @brief Vector of leaf nodes current in the tree std::vector leaf_nodes; diff --git a/test/coverage.cpp b/test/coverage.cpp index ef62271..b09f7d2 100644 --- a/test/coverage.cpp +++ b/test/coverage.cpp @@ -307,12 +307,12 @@ int main() } catch (const std::exception& ex) { - std::cout << "Error: " << ex.what() << '\n'; + std::cerr << "Error: " << ex.what() << '\n'; return 1; } catch (...) { - std::cout << "Error" << '\n'; + std::cerr << "Error" << '\n'; return 1; } From c182cd5146bf914543ebd3dc27177e545a1a54f9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Jun 2026 18:50:33 +0000 Subject: [PATCH 2/2] Fix clang-tidy move helper warning --- merklecpp.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/merklecpp.h b/merklecpp.h index 2698ceb..532b495 100644 --- a/merklecpp.h +++ b/merklecpp.h @@ -749,7 +749,7 @@ namespace merkle /// @param other Tree to move TreeT(TreeT&& other) noexcept { - move_from(std::move(other)); + move_from(other); } /// @brief Deserialises a tree @@ -994,7 +994,7 @@ namespace merkle } clear(); - move_from(std::move(other)); + move_from(other); return *this; } @@ -1731,7 +1731,7 @@ namespace merkle num_flushed = 0; } - void move_from(TreeT&& other) noexcept + void move_from(TreeT& other) noexcept { leaf_nodes = std::move(other.leaf_nodes); uninserted_leaf_nodes = std::move(other.uninserted_leaf_nodes);