Merge pull request #46 from PickNikRobotics/revert/45-script-numeric-strings

Revert script numeric-string sync (#45) until 11.0

Author: L4co77

include/behaviortree_cpp/scripting/operators.hpp

@@ -15,12 +15,8 @@
 #include "behaviortree_cpp/scripting/any_types.hpp"
 #include "behaviortree_cpp/scripting/script_parser.hpp"
 
-#include <cctype>
-#include <charconv>
 #include <cmath>
-#include <locale>
 #include <memory>
-#include <sstream>
 #include <string>
 #include <vector>
 
@@ -54,39 +50,6 @@ inline double StringToDouble(const Any& value, const Environment& env)
   return value.cast<double>();
 }
 
-// Replace a string holding a whole number (e.g. "42" or "1.5") with that number.
-// Unlike StringToDouble, it rejects numeric prefixes, booleans and enums.
-inline void NumericStringToNumber(Any& value)
-{
-  const auto str = value.cast<std::string>();
-  const char* end = str.data() + str.size();
-  int64_t int_val = 0;
-  const auto [ptr, ec] = std::from_chars(str.data(), end, int_val);
-  double dbl_val = 0;
-  if(ec == std::errc() && ptr == end)
-  {
-    value = Any(int_val);
-    return;
-  }
-  // Upstream calls parseDouble(str, dbl_val, true), which its #1167 added and this fork doesn't
-  // have yet: from_chars with a full-consumption check, or a classic-locale stream without it.
-#if __cpp_lib_to_chars >= 201611L
-  const auto [dbl_ptr, dbl_ec] = std::from_chars(str.data(), end, dbl_val);
-  const bool parsed = dbl_ec == std::errc() && dbl_ptr == end;
-#else
-  std::istringstream stream(str);
-  stream.imbue(std::locale::classic());
-  stream >> dbl_val;
-  const bool parsed = !str.empty() &&
-                      !std::isspace(static_cast<unsigned char>(str.front())) &&
-                      !stream.fail() && stream.peek() == std::char_traits<char>::eof();
-#endif
-  if(parsed)
-  {
-    value = Any(dbl_val);
-  }
-}
-
 struct ExprBase
 {
   using Ptr = std::shared_ptr<ExprBase>;
@@ -166,38 +129,18 @@ struct ExprUnaryArithmetic : ExprBase
         case negate:
           return Any(-rv);
         case complement:
-          // Use the checked conversion, like the binary bitwise operators do.
-          // A plain static_cast is undefined for NaN and for values at or
-          // beyond the int64_t range: the hand-written comparison used here
-          // before let NaN (every comparison is false) and 2^63 (not greater
-          // than double(INT64_MAX), which rounds up to 2^63) through, and
-          // silently truncated fractional values.
-          try
-          {
-            return Any(static_cast<double>(~rhs_v.cast<int64_t>()));
-          }
-          catch(const std::exception&)
+          if(rv > static_cast<double>(std::numeric_limits<int64_t>::max()) ||
+             rv < static_cast<double>(std::numeric_limits<int64_t>::min()))
           {
             throw RuntimeError("Number out of range for bitwise operation");
           }
+          return Any(static_cast<double>(~static_cast<int64_t>(rv)));
         case logical_not:
           return Any(static_cast<double>(!static_cast<bool>(rv)));
       }
     }
     else if(rhs_v.isString())
     {
-      if(op == logical_not)
-      {
-        const auto str = rhs_v.cast<std::string>();
-        if(str == "true" || str == "True" || str == "TRUE" || str == "1")
-        {
-          return Any(0.0);
-        }
-        if(str == "false" || str == "False" || str == "FALSE" || str == "0")
-        {
-          return Any(1.0);
-        }
-      }
       throw RuntimeError("Invalid operator for std::string");
     }
     throw RuntimeError("ExprUnaryArithmetic: undefined");
@@ -270,21 +213,11 @@ struct ExprBinaryArithmetic : ExprBase
       throw RuntimeError(ErrorNotInit("right", opStr()));
     }
 
-    // A numeric string mixed with a number is used as that number, e.g. the
-    // literal SubTree port int_val="42" in "int_val + 1"
-    if(op == plus || op == minus || op == times || op == div)
-    {
-      if(lhs_v.isString() && rhs_v.isNumber())
-      {
-        NumericStringToNumber(lhs_v);
-      }
-      else if(lhs_v.isNumber() && rhs_v.isString())
-      {
-        NumericStringToNumber(rhs_v);
-      }
-    }
-
-    if(rhs_v.isNumber() && lhs_v.isNumber())
+    // If just one value is strongly typed as a number, and the other can be casted to one, do arithmetic. If both are not strongly typed, continue. In the case of `+`, non-strongly typed values will result in concatenation.
+    bool are_numbers = (rhs_v.isNumber() && lhs_v.isNumber()) ||
+                       ((rhs_v.tryCast<double>().has_value() && lhs_v.isNumber()) !=
+                        (rhs_v.isNumber() && lhs_v.tryCast<double>().has_value()));
+    if(are_numbers)
     {
       auto lv = lhs_v.cast<double>();
       auto rv = rhs_v.cast<double>();

include/behaviortree_cpp/utils/convert_impl.hpp

@@ -114,15 +114,8 @@ inline void checkTruncation(const From& from)
   // Handle floating point to integer
   else if constexpr(std::is_floating_point_v<From> && std::is_integral_v<To>)
   {
-    // static_cast<From>(To::max) rounds up to an out-of-range value when To has
-    // more significant bits than From's mantissa (e.g. double -> int64_t). In
-    // that case a plain `>` lets the rounded boundary through and the cast to To
-    // below overflows, so use a half-open upper bound to reject it first.
-    constexpr bool max_rounds_up =
-        std::numeric_limits<To>::digits > std::numeric_limits<From>::digits;
-    const From max_limit = static_cast<From>(std::numeric_limits<To>::max());
-    const bool over = max_rounds_up ? (from >= max_limit) : (from > max_limit);
-    if(over || from < static_cast<From>(std::numeric_limits<To>::lowest()) ||
+    if(from > static_cast<From>(std::numeric_limits<To>::max()) ||
+       from < static_cast<From>(std::numeric_limits<To>::lowest()) ||
        from != std::nearbyint(from))
     {
       throw std::runtime_error("Invalid floating point to integer conversion");

include/behaviortree_cpp/utils/safe_any.hpp

@@ -259,16 +259,8 @@ inline bool ValidCast(const SRC& val)
     // Handle conversion to integral
     else if constexpr(std::is_integral_v<TO>)
     {
-      // static_cast<SRC>(TO::max) rounds up to an out-of-range value when TO has
-      // more significant bits than a floating SRC's mantissa (e.g. double ->
-      // int64_t). A plain `>` would let that rounded boundary through and the
-      // cast to TO below overflows, so use a half-open upper bound in that case.
-      constexpr bool max_rounds_up =
-          std::is_floating_point_v<SRC> &&
-          std::numeric_limits<TO>::digits > std::numeric_limits<SRC>::digits;
-      const SRC max_limit = static_cast<SRC>(std::numeric_limits<TO>::max());
-      const bool over = max_rounds_up ? (val >= max_limit) : (val > max_limit);
-      if(over || val < static_cast<SRC>(std::numeric_limits<TO>::lowest()))
+      if(val > static_cast<SRC>(std::numeric_limits<TO>::max()) ||
+         val < static_cast<SRC>(std::numeric_limits<TO>::lowest()))
       {
         return false;
       }

tests/gtest_basic_types.cpp

@@ -123,31 +123,6 @@ TEST(BasicTypes, ConvertFromString_Double)
   ASSERT_THROW((void)convertFromString<double>("not_a_number"), RuntimeError);
 }
 
-TEST(BasicTypes, DoubleToIntBoundary)
-{
-  // A double that lands exactly on 2^63 must be rejected before the cast, not
-  // static_cast to int64_t (which is out of range and undefined behavior).
-  // 2^63 = 9223372036854775808.0; INT64_MAX is 9223372036854775807.
-  const double two_pow_63 = 9223372036854775808.0;
-  EXPECT_ANY_THROW((void)BT::Any(two_pow_63).cast<int64_t>());
-
-  // 2^64 lands one past UINT64_MAX and must likewise be rejected.
-  const double two_pow_64 = 18446744073709551616.0;
-  EXPECT_ANY_THROW((void)BT::Any(two_pow_64).cast<uint64_t>());
-
-  // In-range integral doubles still convert cleanly.
-  const double in_range = 4611686018427387904.0;  // 2^62
-  EXPECT_EQ(BT::Any(in_range).cast<int64_t>(), int64_t(4611686018427387904LL));
-  EXPECT_EQ(BT::Any(1000.0).cast<int64_t>(), int64_t(1000));
-
-  // Same boundary reached through Blackboard::set, which routes the numeric
-  // safe-cast check (isCastingSafe/ValidCast) instead of Any::convert.
-  auto bb = BT::Blackboard::create();
-  bb->set("k", int64_t(1));  // lock the entry to int64_t
-  EXPECT_ANY_THROW(bb->set("k", two_pow_63));
-  EXPECT_NO_THROW(bb->set("k", in_range));
-}
-
 TEST(BasicTypes, ConvertFromString_Bool)
 {
   ASSERT_TRUE(convertFromString<bool>("true"));

tests/gtest_subtree.cpp

@@ -1089,175 +1089,3 @@ TEST(SubTree, NoNameAttribute_AutoGeneratesUniquePaths)
   auto status = tree.tickWhileRunning();
   ASSERT_EQ(status, NodeStatus::SUCCESS);
 }
-
-// Regression test: logical NOT should accept literal boolean values passed to
-// subtrees without changing their string representation in the blackboard.
-TEST(SubTree, LiteralBooleanPortsSupportLogicalNot)
-{
-  // clang-format off
-  static const char* xml_text = R"(
-<root BTCPP_format="4" main_tree_to_execute="MainTree">
-
-    <BehaviorTree ID="MainTree">
-        <SubTree ID="CheckBooleans" enabled="true" disabled="false"
-                 enabled_upper="TRUE" disabled_title="False"
-                 enabled_numeric="{enabled_numeric}"
-                 disabled_numeric="{disabled_numeric}" />
-    </BehaviorTree>
-
-    <BehaviorTree ID="CheckBooleans">
-        <Sequence>
-            <AlwaysSuccess _skipIf="!enabled" />
-            <ScriptCondition code="!disabled" />
-            <ScriptCondition code="!!enabled_upper" />
-            <ScriptCondition code="!disabled_title" />
-            <ScriptCondition code="!!enabled_numeric" />
-            <ScriptCondition code="!disabled_numeric" />
-        </Sequence>
-    </BehaviorTree>
-
-    <TreeNodesModel>
-        <SubTree ID="CheckBooleans">
-            <input_port name="enabled" type="bool" />
-            <input_port name="disabled" type="bool" />
-            <input_port name="enabled_upper" type="bool" />
-            <input_port name="disabled_title" type="bool" />
-            <input_port name="enabled_numeric" type="bool" />
-            <input_port name="disabled_numeric" type="bool" />
-        </SubTree>
-    </TreeNodesModel>
-
-</root>
-)";
-  // clang-format on
-
-  BehaviorTreeFactory factory;
-  auto tree = factory.createTreeFromText(xml_text);
-  tree.rootBlackboard()->set("enabled_numeric", std::string("1"));
-  tree.rootBlackboard()->set("disabled_numeric", std::string("0"));
-
-  ASSERT_EQ(tree.tickWhileRunning(), NodeStatus::SUCCESS);
-  ASSERT_EQ(tree.subtrees[1]->blackboard->get<std::string>("enabled"), "true");
-  ASSERT_EQ(tree.subtrees[1]->blackboard->get<std::string>("disabled"), "false");
-  ASSERT_EQ(tree.subtrees[1]->blackboard->get<std::string>("enabled_upper"), "TRUE");
-  ASSERT_EQ(tree.subtrees[1]->blackboard->get<std::string>("disabled_title"), "False");
-}
-
-TEST(SubTree, LiteralNonBooleanPortsRejectLogicalNot)
-{
-  // clang-format off
-  static const char* xml_text = R"(
-<root BTCPP_format="4" main_tree_to_execute="MainTree">
-
-    <BehaviorTree ID="MainTree">
-        <SubTree ID="CheckValue" value="1not_bool" />
-    </BehaviorTree>
-
-    <BehaviorTree ID="CheckValue">
-        <ScriptCondition code="!value" />
-    </BehaviorTree>
-
-</root>
-)";
-  // clang-format on
-
-  BehaviorTreeFactory factory;
-  auto tree = factory.createTreeFromText(xml_text);
-
-  ASSERT_THROW((void)tree.tickWhileRunning(), RuntimeError);
-  ASSERT_EQ(tree.subtrees[1]->blackboard->get<std::string>("value"), "1not_bool");
-}
-
-// Regression test (#1121): Script expressions can do arithmetic with literal
-// numeric values passed to subtrees.
-TEST(SubTree, LiteralNumericPortsPreserveType)
-{
-  // clang-format off
-  static const char* xml_text = R"(
-<root BTCPP_format="4" main_tree_to_execute="MainTree">
-
-    <BehaviorTree ID="MainTree">
-        <Sequence>
-            <SubTree ID="DoMath" int_val="42" dbl_val="3.14" str_val="hello"
-                     remapped_val="{from_parent}" />
-        </Sequence>
-    </BehaviorTree>
-
-    <BehaviorTree ID="DoMath">
-        <Sequence>
-            <ScriptCondition code=" int_val + 1 == 43 " />
-            <ScriptCondition code=" dbl_val > 3.0 " />
-            <ScriptCondition code=" remapped_val + 1 == 101 " />
-        </Sequence>
-    </BehaviorTree>
-
-</root>
-)";
-  // clang-format on
-
-  BehaviorTreeFactory factory;
-
-  auto tree = factory.createTreeFromText(xml_text);
-
-  // Set the remapped parent value as an integer
-  tree.rootBlackboard()->set("from_parent", 100);
-
-  const auto status = tree.tickWhileRunning();
-  ASSERT_EQ(status, NodeStatus::SUCCESS);
-}
-
-// Issue #1246: a literal passed to a SubTree stays a string, so ports of different
-// types can read it, and its spelling is preserved.
-TEST(SubTree, LiteralPortsAreReadByAnyPortType)
-{
-  // clang-format off
-  static const char* xml_text = R"(
-<root BTCPP_format="4" main_tree_to_execute="MainTree">
-
-    <BehaviorTree ID="MainTree">
-        <SubTree ID="Sub" mode="1" code="007" />
-    </BehaviorTree>
-
-    <BehaviorTree ID="Sub">
-        <Sequence>
-            <ReadString value="{mode}" />
-            <ReadInt value="{mode}" />
-            <ReadDouble value="{mode}" />
-            <ReadString value="{code}" />
-        </Sequence>
-    </BehaviorTree>
-
-</root>
-)";
-  // clang-format on
-
-  BehaviorTreeFactory factory;
-  std::vector<std::string> strings;
-  int int_val = 0;
-  double dbl_val = 0;
-  factory.registerSimpleAction("ReadString",
-                               [&](TreeNode& node) {
-                                 strings.push_back(
-                                     node.getInput<std::string>("value").value());
-                                 return NodeStatus::SUCCESS;
-                               },
-                               { InputPort<std::string>("value") });
-  factory.registerSimpleAction("ReadInt",
-                               [&](TreeNode& node) {
-                                 int_val = node.getInput<int>("value").value();
-                                 return NodeStatus::SUCCESS;
-                               },
-                               { InputPort<int>("value") });
-  factory.registerSimpleAction("ReadDouble",
-                               [&](TreeNode& node) {
-                                 dbl_val = node.getInput<double>("value").value();
-                                 return NodeStatus::SUCCESS;
-                               },
-                               { InputPort<double>("value") });
-
-  auto tree = factory.createTreeFromText(xml_text);
-  ASSERT_EQ(tree.tickWhileRunning(), NodeStatus::SUCCESS);
-  ASSERT_EQ(strings, (std::vector<std::string>{ "1", "007" }));
-  ASSERT_EQ(int_val, 1);
-  ASSERT_EQ(dbl_val, 1.0);
-}