reject out-of-range values in the double to float conversions

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 <typename From, typename To>
+inline void checkFloatingPointRange(const From& from)
+{
+  if constexpr(std::numeric_limits<To>::max_exponent <
+               std::numeric_limits<From>::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<To>::max_exponent - std::numeric_limits<To>::digits - 1;
+    const From limit = static_cast<From>(std::numeric_limits<To>::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 <typename From, typename To>
 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<From> && std::is_floating_point_v<To>)
+  {
+    checkFloatingPointRange<From, To>(from);
+  }
   // Handle other conversions
   else
   {
@@ -202,13 +230,7 @@ void convertNumber(const SRC& source, DST& target)
   else if constexpr(std::is_floating_point<SRC>::value ||
                     std::is_floating_point<DST>::value)
   {
-    bool both_float =
-        std::is_floating_point<SRC>::value && std::is_floating_point<DST>::value;
-    // to avoid being too pedantic, let's accept casting between double and float
-    if(!both_float)
-    {
-      checkTruncation<SRC, DST>(source);
-    }
+    checkTruncation<SRC, DST>(source);
     target = static_cast<DST>(source);
   }
 }

include/behaviortree_cpp/utils/safe_any.hpp

@@ -475,7 +475,10 @@ inline nonstd::expected<T, std::string> Any::stringToNumber() const
       double value = 0.0;
       if(parseDouble(str.toStdStringView(), value, /*require_full_consumption=*/false))
       {
-        return static_cast<T>(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<T>();
       }
       return nonstd::make_unexpected("Any failed string to number conversion");
     }

src/basic_types.cpp

@@ -11,6 +11,8 @@
 #if !defined(__cpp_lib_to_chars) || (__cpp_lib_to_chars < 201611L)
 #include <cctype>
 #include <cerrno>
+#include <cmath>
+#include <limits>
 
 #include <locale.h>
 #if defined(__APPLE__)
@@ -299,6 +301,13 @@ float convertFromString<float>(StringView str)
   {
     throw RuntimeError(StrCat("Can't convert string [", str, "] to float"));
   }
+  // std::from_chars<float> 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<float>(result);
 #endif
 }

tests/gtest_basic_types.cpp

@@ -167,6 +167,48 @@ TEST(BasicTypes, IntToDoubleBoundary)
   EXPECT_EQ(bb->get<double>("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<float>());
+  EXPECT_ANY_THROW((void)BT::Any(-1e300).cast<float>());
+
+  // Rounding to float is still accepted, as are values close to the limit.
+  EXPECT_FLOAT_EQ(BT::Any(0.1).cast<float>(), 0.1f);
+  EXPECT_FLOAT_EQ(BT::Any(3.4e38).cast<float>(), 3.4e38f);
+  EXPECT_FLOAT_EQ(BT::Any(-3.4e38).cast<float>(), -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<float>::max();
+  EXPECT_EQ(BT::Any(3.4028235e38).cast<float>(), flt_max);
+  EXPECT_EQ(BT::Any(-3.4028235e38).cast<float>(), -flt_max);
+  EXPECT_EQ(convertFromString<float>("3.4028235e38"), flt_max);
+  // FLT_MAX + half an ulp rounds to infinity.
+  EXPECT_ANY_THROW((void)BT::Any(0x1.ffffffp+127).cast<float>());
+  EXPECT_ANY_THROW((void)convertFromString<float>("6.8056473e38"));
+  // Widening never throws, and non-finite values pass through.
+  EXPECT_EQ(BT::Any(flt_max).cast<double>(), double(flt_max));
+  EXPECT_TRUE(std::isinf(BT::Any(std::numeric_limits<double>::infinity()).cast<float>()));
+
+  // Same path through a blackboard entry holding a double.
+  auto bb = BT::Blackboard::create();
+  bb->set("v", 1e300);
+  EXPECT_ANY_THROW((void)bb->get<float>("v"));
+  bb->set("v", 2.5);
+  EXPECT_FLOAT_EQ(bb->get<float>("v"), 2.5f);
+
+  // The string conversions must agree with std::from_chars<float>, which
+  // reports result_out_of_range for these literals.
+  EXPECT_ANY_THROW((void)convertFromString<float>("1e300"));
+  EXPECT_ANY_THROW((void)convertFromString<float>("-1e300"));
+  EXPECT_FLOAT_EQ(convertFromString<float>("1.5"), 1.5f);
+  EXPECT_ANY_THROW((void)BT::Any(std::string("1e300")).cast<float>());
+  EXPECT_FLOAT_EQ(BT::Any(std::string("1.5")).cast<float>(), 1.5f);
+}
+
 TEST(BasicTypes, ConvertFromString_Bool)
 {
   ASSERT_TRUE(convertFromString<bool>("true"));