Merge pull request #14 from hit9/dev

0.4.2: Enable Move for base Node class

bt.h

@@ -1,5 +1,5 @@
 // Copyright (c) 2024 Chao Wang <[email protected]>.
-// License: BSD, Version: 0.4.1.  https://github.com/hit9/bt.cc
+// License: BSD, Version: 0.4.2.  https://github.com/hit9/bt.cc
 // A lightweight behavior tree library that separates data and behavior.
 
 #ifndef HIT9_BT_H
@@ -73,7 +73,11 @@ class ITreeBlob {
   virtual void reserve(const std::size_t cap) {}
 
  public:
-  virtual ~ITreeBlob() {}
+  // virtual destructor is required for unique_ptr.
+  // this also disables move, but we don't declare default move constructor generation here, since there's no
+  // member stored here.
+  virtual ~ITreeBlob() = default;
+
   // Returns a pointer to given NodeBlob B for the node with given id.
   // Allocates if not exist.
   // Parameter cb is an optional function to be called after the blob is first allocated.
@@ -196,7 +200,13 @@ class Node {
   using Blob = NodeBlob;
 
   explicit Node(std::string_view name = "Node") : name(name) {}
+  // Destructor is required by unique_ptr.
+  // And this disabled default generation for move constructors.
   virtual ~Node() = default;
+  // We have to declare move constructor and assignment methods generation explicitly.
+  // So that the sub classes will support fully move semantics.
+  Node(Node&&) noexcept = default;             // move constructor
+  Node& operator=(Node&&) noexcept = default;  // move assignment operator
 
   // Simple Getters
   // ~~~~~~~~~~~~~~
@@ -852,7 +862,6 @@ class Builder : public _InternalBuilderBase {
 
  public:
   Builder() : _InternalBuilderBase() {}  // cppcheck-suppress uninitMemberVar
-  ~Builder() {}
 
   // Should be called on the end of the build process.
   void End() {
@@ -882,6 +891,7 @@ class Builder : public _InternalBuilderBase {
       return attachInternalNode(make<T>(false, std::forward<Args>(args)...));
   }
   // Attach a node through move, rarely used.
+  // The inst should be a node object that supports move semantics.
   template <TNode T>
   auto& M(T&& inst) {
     if constexpr (std::is_base_of_v<LeafNode, T>)  // LeafNode
@@ -1083,7 +1093,7 @@ class Builder : public _InternalBuilderBase {
   //      .End();
   auto& Subtree(RootNode&& subtree) {
     onSubtreeAttach(subtree, root);
-    return M<RootNode>(std::move(subtree));  // move
+    return M<RootNode>(std::move(subtree));  // move the subtree object
   }
 };
 

changelog

@@ -1,3 +1,9 @@
+0.4.2
+-----
+
+* Enable default move constructor and move assigment generation for class Node.
+* Remove unnessary destructor for class Builder.
+
 0.4.1
 -----
 

tests/Makefile

@@ -18,4 +18,7 @@ run:
 clean:
 	make -C build clean
 
+run-benchmark:
+	cd build && ./bt_benchmark
+
 .PHONY: build

tests/misc_test.cc

@@ -0,0 +1,19 @@
+#include <catch2/catch_template_test_macros.hpp>
+#include <catch2/catch_test_macros.hpp>
+#include <string>
+
+#include "types.h"
+
+TEST_CASE("Misc/1", "[any node should be movable]") {
+  J j1{"a really looooooooong name to avoid the string SSO optimization",
+       "a really looooooooong s string to avoid the string SSO optimization"};
+  auto addr11 = static_cast<void*>(j1.s.data());
+  auto addr12 = static_cast<const void*>(j1.Name().data());
+  J j2(std::move(j1));
+  auto addr21 = static_cast<void*>(j2.s.data());
+  auto addr22 = static_cast<const void*>(j2.Name().data());
+  // The two address of underlying data should be the same if the movement successful done.
+  REQUIRE(addr11 == addr21);
+  // And the base class Node's underlying storage should also movable.
+  REQUIRE(addr12 == addr22);
+}

tests/types.h

@@ -1,3 +1,4 @@
+#include <string>
 
 #include "bt.h"
 
@@ -146,3 +147,9 @@ class I : public bt::ActionNode {
     return bb->shouldPriorityI;
   }
 };
+
+class J : public bt::ActionNode {
+ public:
+  std::string s;
+  J(const std::string& name, const std::string& s) : bt::ActionNode(name), s(s) {}
+};