diff --git a/include/behaviortree_cpp/utils/convert_impl.hpp b/include/behaviortree_cpp/utils/convert_impl.hpp index f70861802..cb1d0a174 100644 --- a/include/behaviortree_cpp/utils/convert_impl.hpp +++ b/include/behaviortree_cpp/utils/convert_impl.hpp @@ -87,6 +87,29 @@ inline void checkLowerLimit(const From& from) } } +// Rounding to a narrower floating point type is accepted, but a finite value +// beyond the range of To has no representation there: the conversion is +// undefined and in practice produces an infinity. +template +inline void checkFloatingPointRange(const From& from) +{ + if constexpr(std::numeric_limits::max_exponent < + std::numeric_limits::max_exponent) + { + // Values below max + half an ulp still round to max (e.g. 3.4028235e38, + // the usual printout of FLT_MAX); from there on they round to infinity. + constexpr int half_ulp_exp = + std::numeric_limits::max_exponent - std::numeric_limits::digits - 1; + const From limit = static_cast(std::numeric_limits::max()) + + std::ldexp(From(1), half_ulp_exp); + if(std::isfinite(from) && std::fabs(from) >= limit) + { + throw std::runtime_error("Value outside the numeric limits of the floating point " + "type"); + } + } +} + template inline void checkTruncation(const From& from) { @@ -132,6 +155,11 @@ inline void checkTruncation(const From& from) throw std::runtime_error("Invalid floating point to integer conversion"); } } + // Handle floating point to floating point (e.g. double to float) + else if constexpr(std::is_floating_point_v && std::is_floating_point_v) + { + checkFloatingPointRange(from); + } // Handle other conversions else { @@ -202,13 +230,7 @@ void convertNumber(const SRC& source, DST& target) else if constexpr(std::is_floating_point::value || std::is_floating_point::value) { - bool both_float = - std::is_floating_point::value && std::is_floating_point::value; - // to avoid being too pedantic, let's accept casting between double and float - if(!both_float) - { - checkTruncation(source); - } + checkTruncation(source); target = static_cast(source); } } diff --git a/include/behaviortree_cpp/utils/safe_any.hpp b/include/behaviortree_cpp/utils/safe_any.hpp index 05fa1b61e..827e4828f 100644 --- a/include/behaviortree_cpp/utils/safe_any.hpp +++ b/include/behaviortree_cpp/utils/safe_any.hpp @@ -475,7 +475,10 @@ inline nonstd::expected Any::stringToNumber() const double value = 0.0; if(parseDouble(str.toStdStringView(), value, /*require_full_consumption=*/false)) { - return static_cast(value); + // same range check as the integral branch: a value that does not fit + // in a float is an error, like std::from_chars reports above + Any temp_any(value); + return temp_any.convert(); } return nonstd::make_unexpected("Any failed string to number conversion"); } diff --git a/src/basic_types.cpp b/src/basic_types.cpp index e4c1554eb..aa740f766 100644 --- a/src/basic_types.cpp +++ b/src/basic_types.cpp @@ -11,6 +11,8 @@ #if !defined(__cpp_lib_to_chars) || (__cpp_lib_to_chars < 201611L) #include #include +#include +#include #include #if defined(__APPLE__) @@ -299,6 +301,13 @@ float convertFromString(StringView str) { throw RuntimeError(StrCat("Can't convert string [", str, "] to float")); } + // std::from_chars reports result_out_of_range for a finite value that + // does not fit in a float; casting the double would silently give an infinity. + // 0x1.ffffffp+127 is FLT_MAX + half an ulp: values below it round to FLT_MAX. + if(std::isfinite(result) && std::fabs(result) >= 0x1.ffffffp+127) + { + throw RuntimeError(StrCat("Can't convert string [", str, "] to float")); + } return static_cast(result); #endif } diff --git a/tests/gtest_basic_types.cpp b/tests/gtest_basic_types.cpp index 270a1cb57..a224bc127 100644 --- a/tests/gtest_basic_types.cpp +++ b/tests/gtest_basic_types.cpp @@ -167,6 +167,48 @@ TEST(BasicTypes, IntToDoubleBoundary) EXPECT_EQ(bb->get("d"), 1000.0); } +TEST(BasicTypes, FloatConversionLimits) +{ + // A double beyond the float range has no float representation: the cast is + // undefined and used to return an infinity. Reject it like every other + // out-of-range narrowing. + EXPECT_ANY_THROW((void)BT::Any(1e300).cast()); + EXPECT_ANY_THROW((void)BT::Any(-1e300).cast()); + + // Rounding to float is still accepted, as are values close to the limit. + EXPECT_FLOAT_EQ(BT::Any(0.1).cast(), 0.1f); + EXPECT_FLOAT_EQ(BT::Any(3.4e38).cast(), 3.4e38f); + EXPECT_FLOAT_EQ(BT::Any(-3.4e38).cast(), -3.4e38f); + + // 3.4028235e38 (the usual printout of FLT_MAX) is a bit above FLT_MAX as a + // double, but below FLT_MAX + half an ulp, so it rounds to FLT_MAX. + constexpr float flt_max = std::numeric_limits::max(); + EXPECT_EQ(BT::Any(3.4028235e38).cast(), flt_max); + EXPECT_EQ(BT::Any(-3.4028235e38).cast(), -flt_max); + EXPECT_EQ(convertFromString("3.4028235e38"), flt_max); + // FLT_MAX + half an ulp rounds to infinity. + EXPECT_ANY_THROW((void)BT::Any(0x1.ffffffp+127).cast()); + EXPECT_ANY_THROW((void)convertFromString("6.8056473e38")); + // Widening never throws, and non-finite values pass through. + EXPECT_EQ(BT::Any(flt_max).cast(), double(flt_max)); + EXPECT_TRUE(std::isinf(BT::Any(std::numeric_limits::infinity()).cast())); + + // Same path through a blackboard entry holding a double. + auto bb = BT::Blackboard::create(); + bb->set("v", 1e300); + EXPECT_ANY_THROW((void)bb->get("v")); + bb->set("v", 2.5); + EXPECT_FLOAT_EQ(bb->get("v"), 2.5f); + + // The string conversions must agree with std::from_chars, which + // reports result_out_of_range for these literals. + EXPECT_ANY_THROW((void)convertFromString("1e300")); + EXPECT_ANY_THROW((void)convertFromString("-1e300")); + EXPECT_FLOAT_EQ(convertFromString("1.5"), 1.5f); + EXPECT_ANY_THROW((void)BT::Any(std::string("1e300")).cast()); + EXPECT_FLOAT_EQ(BT::Any(std::string("1.5")).cast(), 1.5f); +} + TEST(BasicTypes, ConvertFromString_Bool) { ASSERT_TRUE(convertFromString("true"));