From 2fc41daf57cebbd398e58f4004adae47131f7ddf Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sun, 1 Feb 2026 20:16:44 +0100 Subject: [PATCH 1/2] Fix #937: allow BehaviorTreeFactory to be returned by value Move constructor and move assignment were defaulted in the header where PImpl is an incomplete type, preventing compilation when returning BehaviorTreeFactory by value from inline functions. Move the definitions to bt_factory.cpp where PImpl is complete, matching the existing pattern used for the destructor. Co-Authored-By: Claude Opus 4.5 --- include/behaviortree_cpp/bt_factory.h | 4 ++-- src/bt_factory.cpp | 4 ++++ tests/gtest_factory.cpp | 22 ++++++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/include/behaviortree_cpp/bt_factory.h b/include/behaviortree_cpp/bt_factory.h index 3471f51bf..efcf47722 100644 --- a/include/behaviortree_cpp/bt_factory.h +++ b/include/behaviortree_cpp/bt_factory.h @@ -224,8 +224,8 @@ class BehaviorTreeFactory BehaviorTreeFactory(const BehaviorTreeFactory& other) = delete; BehaviorTreeFactory& operator=(const BehaviorTreeFactory& other) = delete; - BehaviorTreeFactory(BehaviorTreeFactory&& other) noexcept = default; - BehaviorTreeFactory& operator=(BehaviorTreeFactory&& other) noexcept = default; + BehaviorTreeFactory(BehaviorTreeFactory&& other) noexcept; + BehaviorTreeFactory& operator=(BehaviorTreeFactory&& other) noexcept; /// Remove a registered ID. bool unregisterBuilder(const std::string& ID); diff --git a/src/bt_factory.cpp b/src/bt_factory.cpp index a0db78827..02d2ca5a7 100644 --- a/src/bt_factory.cpp +++ b/src/bt_factory.cpp @@ -106,6 +106,10 @@ BehaviorTreeFactory::BehaviorTreeFactory() : _p(new PImpl) BehaviorTreeFactory::~BehaviorTreeFactory() = default; +BehaviorTreeFactory::BehaviorTreeFactory(BehaviorTreeFactory&& other) noexcept = default; +BehaviorTreeFactory& +BehaviorTreeFactory::operator=(BehaviorTreeFactory&& other) noexcept = default; + bool BehaviorTreeFactory::unregisterBuilder(const std::string& ID) { if(builtinNodes().count(ID) != 0) diff --git a/tests/gtest_factory.cpp b/tests/gtest_factory.cpp index 13455bda5..93eabb95c 100644 --- a/tests/gtest_factory.cpp +++ b/tests/gtest_factory.cpp @@ -450,6 +450,28 @@ TEST(BehaviorTreeFactory, ManifestMethod) EXPECT_NE(xml.find(expectedXML), std::string::npos); } +TEST(BehaviorTreeFactory, ReturnByValue) +{ + // Issue #937: BehaviorTreeFactory cannot be returned by value from + // a function because the move constructor/assignment were defaulted + // in the header where PImpl is an incomplete type. + auto make_factory = []() -> BT::BehaviorTreeFactory { + BT::BehaviorTreeFactory factory; + factory.registerNodeType("SaySomething"); + return factory; + }; + auto factory = make_factory(); + + // Verify the moved-into factory is fully functional + const auto& manifests = factory.manifests(); + ASSERT_TRUE(manifests.count("SaySomething")); + + // Also test move assignment + BT::BehaviorTreeFactory factory2; + factory2 = std::move(factory); + ASSERT_TRUE(factory2.manifests().count("SaySomething")); +} + TEST(BehaviorTreeFactory, addMetadataToManifest) { BehaviorTreeFactory factory; From 78ab2fc75955e0babba46e5547586e3032a49156 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sun, 1 Feb 2026 20:29:15 +0100 Subject: [PATCH 2/2] Fix clang-tidy readability-implicit-bool-conversion warning Use explicit != nullptr comparison for pointer-to-bool conversion flagged by clang-tidy. Co-Authored-By: Claude Opus 4.5 --- src/tree_node.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tree_node.cpp b/src/tree_node.cpp index d464d6af9..16171a135 100644 --- a/src/tree_node.cpp +++ b/src/tree_node.cpp @@ -481,7 +481,7 @@ AnyPtrLocked BT::TreeNode::getLockedPortContent(const std::string& key) { const auto bb_key = std::string(*remapped_key); auto result = _p->config.blackboard->getAnyLocked(bb_key); - if(!result && _p->config.manifest) + if(!result && _p->config.manifest != nullptr) { // Entry doesn't exist yet. Create it using the port's type info // from the manifest so that getLockedPortContent works even when