Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
L4co77 merged 1 commit into from
Nov 4, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 8 additions & 75 deletions include/behaviortree_cpp/scripting/operators.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down Expand Up @@ -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>;
Expand Down Expand Up @@ -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()))
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
{
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");
Expand Down Expand Up @@ -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)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
{
auto lv = lhs_v.cast<double>();
auto rv = rhs_v.cast<double>();
Expand Down
11 changes: 2 additions & 9 deletions include/behaviortree_cpp/utils/convert_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()) ||
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
from < static_cast<From>(std::numeric_limits<To>::lowest()) ||
from != std::nearbyint(from))
{
throw std::runtime_error("Invalid floating point to integer conversion");
Expand Down
12 changes: 2 additions & 10 deletions include/behaviortree_cpp/utils/safe_any.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()) ||
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
val < static_cast<SRC>(std::numeric_limits<TO>::lowest()))
{
return false;
}
Expand Down
25 changes: 0 additions & 25 deletions tests/gtest_basic_types.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down
172 changes: 0 additions & 172 deletions tests/gtest_subtree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Loading
Loading