fix warnings

Author: facontidavide

.clang-tidy

@@ -13,26 +13,14 @@ Checks: [
   "-bugprone-easily-swappable-parameters",
   "-bugprone-narrowing-conversions",
   "-cppcoreguidelines-avoid-c-arrays",
-  "-cppcoreguidelines-avoid-const-or-ref-data-members",
   "-cppcoreguidelines-avoid-magic-numbers",
   "-cppcoreguidelines-avoid-non-const-global-variables",
-  "-cppcoreguidelines-narrowing-conversions",
-  "-cppcoreguidelines-owning-memory",
   "-cppcoreguidelines-pro-bounds-array-to-pointer-decay",
   "-cppcoreguidelines-pro-bounds-constant-array-index",
   "-cppcoreguidelines-pro-bounds-pointer-arithmetic",
-  "-cppcoreguidelines-pro-type-const-cast",
-  "-cppcoreguidelines-pro-type-member-init",
   "-cppcoreguidelines-pro-type-union-access",
   "-cppcoreguidelines-pro-type-vararg",
-  "-cppcoreguidelines-use-default-member-init",
-  "-misc-no-recursion",
-  "-modernize-avoid-c-arrays",
-  "-modernize-use-emplace",
-  "-performance-avoid-const-params",
-  "-performance-avoid-endl",
-  "-performance-noexcept-move-constructor",
-  "-readability-magic-numbers"
+  "-misc-no-recursion"
 ]
 
 

include/behaviortree_cpp/actions/sleep_node.h

@@ -35,7 +35,7 @@ class SleepNode : public StatefulActionNode
 
 private:
   TimerQueue<> timer_;
-  uint64_t timer_id_;
+  uint64_t timer_id_ = 0;
 
   std::atomic_bool timer_waiting_ = false;
   std::mutex delay_mutex_;

include/behaviortree_cpp/utils/convert_impl.hpp

@@ -157,13 +157,17 @@ void convertNumber(const SRC& source, DST& target)
   // these conversions are always safe:
   // - same type
   // - float -> double
-  // - conversion to bool
-  if constexpr(is_same<SRC, DST>() || (is_same<SRC, float>() && is_same<DST, double>()) ||
-               (is_convertible_to_bool<SRC>() && is_same<DST, bool>()))
+  if constexpr(is_same<SRC, DST>() || (is_same<SRC, float>() && is_same<DST, double>()))
   {
     // No check needed
     target = static_cast<DST>(source);
   }
+  // conversion to bool requires validation (only 0 and 1 allowed)
+  else if constexpr(is_convertible_to_bool<SRC>() && is_same<DST, bool>())
+  {
+    checkLowerLimit<SRC, DST>(source);
+    target = static_cast<DST>(source);
+  }
   else if constexpr(both_integers)
   {
     if constexpr(sizeof(SRC) == sizeof(DST) && !is_signed<SRC>() && is_signed<DST>())

include/behaviortree_cpp/utils/simple_string.hpp

@@ -18,7 +18,10 @@ namespace SafeAny
 class SimpleString
 {
 public:
-  SimpleString() = default;
+  SimpleString()
+  {
+    _storage.soo.data[0] = '\0';
+  }
 
   SimpleString(const std::string& str) : SimpleString(str.data(), str.size())
   {}

src/behavior_tree.cpp

@@ -98,9 +98,7 @@ void printTreeRecursively(const TreeNode* root_node, std::ostream& stream)
   stream << "----------------" << std::endl;
 }
 
-namespace
-{
-void buildSerializedStatusSnapshot(TreeNode* root_node,
+void buildSerializedStatusSnapshot(const TreeNode* root_node,
                                    SerializedTreeStatus& serialized_buffer)
 {
   serialized_buffer.clear();
@@ -112,7 +110,6 @@ void buildSerializedStatusSnapshot(TreeNode* root_node,
 
   applyRecursiveVisitor(root_node, visitor);
 }
-}  // namespace
 
 int LibraryVersionNumber()
 {

src/blackboard.cpp

@@ -44,6 +44,7 @@ const Any* Blackboard::getAny(const std::string& key) const
 
 Any* Blackboard::getAny(const std::string& key)
 {
+  // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
   return const_cast<Any*>(getAnyLocked(key).get());
 }
 
@@ -322,6 +323,7 @@ Blackboard::Entry& Blackboard::Entry::operator=(const Entry& other)
 Blackboard* BT::Blackboard::rootBlackboard()
 {
   auto bb = static_cast<const Blackboard&>(*this).rootBlackboard();
+  // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
   return const_cast<Blackboard*>(bb);
 }
 

src/loggers/groot2_publisher.cpp

@@ -37,16 +37,16 @@ std::array<char, 16> CreateRandomUUID()
   std::random_device rd;
   std::mt19937 gen(rd());
   std::uniform_int_distribution<uint32_t> dist;
-  std::array<char, 16> out;
+  std::array<char, 16> out{};
   char* bytes = out.data();
   for(int i = 0; i < 16; i += 4)
   {
     // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
     *reinterpret_cast<uint32_t*>(bytes + i) = dist(gen);
   }
   // variant must be 10xxxxxx
-  bytes[8] &= 0xBF;
-  bytes[8] |= 0x80;
+  bytes[8] &= static_cast<char>(0xBF);
+  bytes[8] |= static_cast<char>(0x80);
 
   // version must be 0100xxxx
   bytes[6] &= 0x4F;
@@ -100,7 +100,7 @@ struct Groot2Publisher::PImpl
 
   std::atomic_bool recording = false;
   std::deque<Transition> transitions_buffer;
-  std::chrono::microseconds recording_fist_time;
+  std::chrono::microseconds recording_fist_time{};
 
   std::thread heartbeat_thread;
 
@@ -209,13 +209,13 @@ void Groot2Publisher::callback(Duration ts, const TreeNode& node, NodeStatus pre
 
   if(new_status == NodeStatus::IDLE)
   {
-    status = 10 + static_cast<char>(prev_status);
+    status = static_cast<char>(10 + static_cast<int>(prev_status));
   }
   *(_p->status_buffermap.at(node.UID())) = status;
 
   if(_p->recording)
   {
-    Transition trans;
+    Transition trans{};
     trans.node_uid = node.UID();
     trans.status = static_cast<uint8_t>(new_status);
     auto timestamp = ts - _p->recording_fist_time;

src/xml_parsing.cpp

@@ -118,15 +118,15 @@ struct XMLParser::PImpl
   std::list<std::unique_ptr<XMLDocument> > opened_documents;
   std::map<std::string, const XMLElement*> tree_roots;
 
-  const BehaviorTreeFactory& factory;
+  const BehaviorTreeFactory* factory = nullptr;
 
   std::filesystem::path current_path;
   std::map<std::string, SubtreeModel> subtree_models;
 
-  int suffix_count;
+  int suffix_count = 0;
 
   explicit PImpl(const BehaviorTreeFactory& fact)
-    : factory(fact), current_path(std::filesystem::current_path()), suffix_count(0)
+    : factory(&fact), current_path(std::filesystem::current_path())
   {}
 
   void clear()
@@ -162,7 +162,7 @@ XMLParser::~XMLParser()
 
 void XMLParser::loadFromFile(const std::filesystem::path& filepath, bool add_includes)
 {
-  _p->opened_documents.emplace_back(new XMLDocument());
+  _p->opened_documents.push_back(std::make_unique<XMLDocument>());
 
   XMLDocument* doc = _p->opened_documents.back().get();
   doc->LoadFile(filepath.string().c_str());
@@ -174,7 +174,7 @@ void XMLParser::loadFromFile(const std::filesystem::path& filepath, bool add_inc
 
 void XMLParser::loadFromText(const std::string& xml_text, bool add_includes)
 {
-  _p->opened_documents.emplace_back(new XMLDocument());
+  _p->opened_documents.push_back(std::make_unique<XMLDocument>());
 
   XMLDocument* doc = _p->opened_documents.back().get();
   doc->Parse(xml_text.c_str(), xml_text.size());
@@ -311,7 +311,7 @@ void XMLParser::PImpl::loadDocImpl(XMLDocument* doc, bool add_includes)
       file_path = current_path / file_path;
     }
 
-    opened_documents.emplace_back(new XMLDocument());
+    opened_documents.push_back(std::make_unique<XMLDocument>());
     XMLDocument* next_doc = opened_documents.back().get();
 
     // change current path to the included file for handling additional relative paths
@@ -327,7 +327,7 @@ void XMLParser::PImpl::loadDocImpl(XMLDocument* doc, bool add_includes)
 
   // Collect the names of all nodes registered with the behavior tree factory
   std::unordered_map<std::string, BT::NodeType> registered_nodes;
-  for(const auto& it : factory.manifests())
+  for(const auto& it : factory->manifests())
   {
     registered_nodes.insert({ it.first, it.second.type });
   }
@@ -629,7 +629,7 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
   {
     // This is the case of nodes like <MyCustomAction>
     // check if the factory has this name
-    if(factory.builders().count(element_name) == 0)
+    if(factory->builders().count(element_name) == 0)
     {
       throw RuntimeError(element_name, " is not a registered node");
     }
@@ -657,8 +657,8 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
 
   const TreeNodeManifest* manifest = nullptr;
 
-  auto manifest_it = factory.manifests().find(type_ID);
-  if(manifest_it != factory.manifests().end())
+  auto manifest_it = factory->manifests().find(type_ID);
+  if(manifest_it != factory->manifests().end())
   {
     manifest = &manifest_it->second;
   }
@@ -758,7 +758,7 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
   {
     config.input_ports = port_remap;
     new_node =
-        factory.instantiateTreeNode(instance_name, toStr(NodeType::SUBTREE), config);
+        factory->instantiateTreeNode(instance_name, toStr(NodeType::SUBTREE), config);
     auto subtree_node = dynamic_cast<SubTreeNode*>(new_node.get());
     subtree_node->setSubtreeID(type_ID);
   }
@@ -873,7 +873,7 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
       }
     }
 
-    new_node = factory.instantiateTreeNode(instance_name, type_ID, config);
+    new_node = factory->instantiateTreeNode(instance_name, type_ID, config);
   }
 
   // add the pointer of this node to the parent
@@ -1517,25 +1517,6 @@ std::string writeTreeXSD(const BehaviorTreeFactory& factory)
   return std::string(printer.CStr(), size_t(printer.CStrSize() - 1));
 }
 
-namespace
-{
-Tree buildTreeFromText(const BehaviorTreeFactory& factory, const std::string& text,
-                       const Blackboard::Ptr& blackboard)
-{
-  XMLParser parser(factory);
-  parser.loadFromText(text);
-  return parser.instantiateTree(blackboard);
-}
-
-Tree buildTreeFromFile(const BehaviorTreeFactory& factory, const std::string& filename,
-                       const Blackboard::Ptr& blackboard)
-{
-  XMLParser parser(factory);
-  parser.loadFromFile(filename);
-  return parser.instantiateTree(blackboard);
-}
-}  // namespace
-
 std::string WriteTreeToXML(const Tree& tree, bool add_metadata, bool add_builtin_models)
 {
   XMLDocument doc;