diff --git a/include/behaviortree_cpp/scripting/operators.hpp b/include/behaviortree_cpp/scripting/operators.hpp index 483bc7f4d..dc747609e 100644 --- a/include/behaviortree_cpp/scripting/operators.hpp +++ b/include/behaviortree_cpp/scripting/operators.hpp @@ -15,8 +15,12 @@ #include "behaviortree_cpp/scripting/any_types.hpp" #include "behaviortree_cpp/scripting/script_parser.hpp" +#include +#include #include +#include #include +#include #include #include @@ -50,6 +54,39 @@ inline double StringToDouble(const Any& value, const Environment& env) return value.cast(); } +// 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(); + 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(str.front())) && + !stream.fail() && stream.peek() == std::char_traits::eof(); +#endif + if(parsed) + { + value = Any(dbl_val); + } +} + struct ExprBase { using Ptr = std::shared_ptr; @@ -129,18 +166,38 @@ struct ExprUnaryArithmetic : ExprBase case negate: return Any(-rv); case complement: - if(rv > static_cast(std::numeric_limits::max()) || - rv < static_cast(std::numeric_limits::min())) + // 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(~rhs_v.cast())); + } + catch(const std::exception&) { throw RuntimeError("Number out of range for bitwise operation"); } - return Any(static_cast(~static_cast(rv))); case logical_not: return Any(static_cast(!static_cast(rv))); } } else if(rhs_v.isString()) { + if(op == logical_not) + { + const auto str = rhs_v.cast(); + 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"); @@ -213,11 +270,21 @@ struct ExprBinaryArithmetic : ExprBase throw RuntimeError(ErrorNotInit("right", opStr())); } - // 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().has_value() && lhs_v.isNumber()) != - (rhs_v.isNumber() && lhs_v.tryCast().has_value())); - if(are_numbers) + // 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()) { auto lv = lhs_v.cast(); auto rv = rhs_v.cast(); diff --git a/include/behaviortree_cpp/utils/convert_impl.hpp b/include/behaviortree_cpp/utils/convert_impl.hpp index 2a9f7c34c..e32eefbcf 100644 --- a/include/behaviortree_cpp/utils/convert_impl.hpp +++ b/include/behaviortree_cpp/utils/convert_impl.hpp @@ -114,8 +114,15 @@ inline void checkTruncation(const From& from) // Handle floating point to integer else if constexpr(std::is_floating_point_v && std::is_integral_v) { - if(from > static_cast(std::numeric_limits::max()) || - from < static_cast(std::numeric_limits::lowest()) || + // static_cast(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::digits > std::numeric_limits::digits; + const From max_limit = static_cast(std::numeric_limits::max()); + const bool over = max_rounds_up ? (from >= max_limit) : (from > max_limit); + if(over || from < static_cast(std::numeric_limits::lowest()) || from != std::nearbyint(from)) { throw std::runtime_error("Invalid floating point to integer conversion"); diff --git a/include/behaviortree_cpp/utils/safe_any.hpp b/include/behaviortree_cpp/utils/safe_any.hpp index 3a87caf8f..65c6cc022 100644 --- a/include/behaviortree_cpp/utils/safe_any.hpp +++ b/include/behaviortree_cpp/utils/safe_any.hpp @@ -259,8 +259,16 @@ inline bool ValidCast(const SRC& val) // Handle conversion to integral else if constexpr(std::is_integral_v) { - if(val > static_cast(std::numeric_limits::max()) || - val < static_cast(std::numeric_limits::lowest())) + // static_cast(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 && + std::numeric_limits::digits > std::numeric_limits::digits; + const SRC max_limit = static_cast(std::numeric_limits::max()); + const bool over = max_rounds_up ? (val >= max_limit) : (val > max_limit); + if(over || val < static_cast(std::numeric_limits::lowest())) { return false; } diff --git a/tests/gtest_basic_types.cpp b/tests/gtest_basic_types.cpp index 57f8bafca..680c45ce3 100644 --- a/tests/gtest_basic_types.cpp +++ b/tests/gtest_basic_types.cpp @@ -123,6 +123,31 @@ TEST(BasicTypes, ConvertFromString_Double) ASSERT_THROW((void)convertFromString("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()); + + // 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()); + + // In-range integral doubles still convert cleanly. + const double in_range = 4611686018427387904.0; // 2^62 + EXPECT_EQ(BT::Any(in_range).cast(), int64_t(4611686018427387904LL)); + EXPECT_EQ(BT::Any(1000.0).cast(), 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("true")); diff --git a/tests/gtest_subtree.cpp b/tests/gtest_subtree.cpp index f543de7bf..185937fc3 100644 --- a/tests/gtest_subtree.cpp +++ b/tests/gtest_subtree.cpp @@ -1089,3 +1089,175 @@ 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"( + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +)"; + // 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("enabled"), "true"); + ASSERT_EQ(tree.subtrees[1]->blackboard->get("disabled"), "false"); + ASSERT_EQ(tree.subtrees[1]->blackboard->get("enabled_upper"), "TRUE"); + ASSERT_EQ(tree.subtrees[1]->blackboard->get("disabled_title"), "False"); +} + +TEST(SubTree, LiteralNonBooleanPortsRejectLogicalNot) +{ + // clang-format off + static const char* xml_text = R"( + + + + + + + + + + + +)"; + // clang-format on + + BehaviorTreeFactory factory; + auto tree = factory.createTreeFromText(xml_text); + + ASSERT_THROW((void)tree.tickWhileRunning(), RuntimeError); + ASSERT_EQ(tree.subtrees[1]->blackboard->get("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"( + + + + + + + + + + + + + + + + + +)"; + // 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"( + + + + + + + + + + + + + + + + +)"; + // clang-format on + + BehaviorTreeFactory factory; + std::vector strings; + int int_val = 0; + double dbl_val = 0; + factory.registerSimpleAction("ReadString", + [&](TreeNode& node) { + strings.push_back( + node.getInput("value").value()); + return NodeStatus::SUCCESS; + }, + { InputPort("value") }); + factory.registerSimpleAction("ReadInt", + [&](TreeNode& node) { + int_val = node.getInput("value").value(); + return NodeStatus::SUCCESS; + }, + { InputPort("value") }); + factory.registerSimpleAction("ReadDouble", + [&](TreeNode& node) { + dbl_val = node.getInput("value").value(); + return NodeStatus::SUCCESS; + }, + { InputPort("value") }); + + auto tree = factory.createTreeFromText(xml_text); + ASSERT_EQ(tree.tickWhileRunning(), NodeStatus::SUCCESS); + ASSERT_EQ(strings, (std::vector{ "1", "007" })); + ASSERT_EQ(int_val, 1); + ASSERT_EQ(dbl_val, 1.0); +} diff --git a/tests/script_parser_test.cpp b/tests/script_parser_test.cpp index 64ed0b1c2..c2c0ccc5c 100644 --- a/tests/script_parser_test.cpp +++ b/tests/script_parser_test.cpp @@ -323,6 +323,37 @@ TEST(ParserTest, Equations) EXPECT_EQ(GetResult(" par1:='3'; par2:=4; par1!=par2").cast(), 1); } +// Issue #1246: a numeric string mixed with a number is used as that number +TEST(ParserTest, NumericStringArithmetic) +{ + BT::Ast::Environment environment = { BT::Blackboard::create(), {} }; + + auto GetResult = [&environment](const char* text) -> BT::Any { + return GetScriptResult(environment, text); + }; + environment.vars->set("int_str", std::string("42")); + environment.vars->set("dbl_str", std::string("1.5")); + environment.vars->set("text", std::string("hello")); + + EXPECT_EQ(GetResult("int_str + 1").cast(), 43.0); + EXPECT_EQ(GetResult("1 + int_str").cast(), 43.0); + EXPECT_EQ(GetResult("int_str - 2").cast(), 40.0); + EXPECT_EQ(GetResult("int_str * 2").cast(), 84.0); + EXPECT_EQ(GetResult("3 / dbl_str").cast(), 2.0); + EXPECT_EQ(GetResult("'1e3' + 1").cast(), 1001.0); + + // the whole string must be a number + EXPECT_ANY_THROW(GetResult("text + 1")); + EXPECT_ANY_THROW(GetResult("'42abc' + 1")); + EXPECT_ANY_THROW(GetResult("'true' + 1")); + EXPECT_ANY_THROW(GetResult("' 42' + 1")); + + // unchanged: two strings concatenate, bitwise operators require numbers + EXPECT_EQ(GetResult("int_str + int_str").cast(), "4242"); + EXPECT_EQ(GetResult("int_str .. 1").cast(), "421"); + EXPECT_ANY_THROW(GetResult("int_str & 1")); +} + TEST(ParserTest, NotInitializedComparison) { BT::Ast::Environment environment = { BT::Blackboard::create(), {} }; @@ -723,6 +754,26 @@ TEST(ParserTest, UnaryOperators) EXPECT_EQ(Parse("10 + -3").value().cast(), 7.0); } +TEST(ParserTest, BitwiseComplementRejectsNonIntegers) +{ + BT::Ast::Environment env = { BT::Blackboard::create(), {} }; + auto Parse = [&env](const char* str) { return BT::ParseScriptAndExecute(env, str); }; + + // The operand was cast to int64_t after a hand-written range check that + // accepted NaN and exactly 2^63 (undefined conversion, reported by UBSan) + // and silently truncated fractional values. + EXPECT_FALSE(Parse("~(0.0 / 0.0)").has_value()); + EXPECT_FALSE(Parse("~9223372036854775808.0").has_value()); // 2^63 + EXPECT_FALSE(Parse("~1.5").has_value()); + EXPECT_FALSE(Parse("~(1.0 / 0.0)").has_value()); + + // Integral values, including the int64_t boundaries, still work. + EXPECT_EQ(Parse("~0").value().cast(), ~int64_t(0)); + EXPECT_EQ(Parse("~(2 * 3)").value().cast(), ~int64_t(6)); + EXPECT_EQ(Parse("~-9223372036854775808.0").value().cast(), + static_cast(~std::numeric_limits::min())); +} + TEST(ParserTest, TernaryExpressions) { BT::Ast::Environment env = { BT::Blackboard::create(), {} };