From 44f16fe823b066a7ac1a6e0fba62947c4a997f7c Mon Sep 17 00:00:00 2001 From: Aysha Afrah Ziya Date: Wed, 30 Sep 2026 21:35:40 +0530 Subject: [PATCH] fix locale-dependent std::stod fallback in Any and SwitchNode The fallback used where the floating-point std::from_chars overload is missing (Apple libc++) honors LC_NUMERIC, so under a comma-decimal locale "3.5" silently parsed as 3. Route both fallbacks through the locale-independent BT::parseDouble already used by convertFromString. --- include/behaviortree_cpp/utils/safe_any.hpp | 17 +++++- src/controls/switch_node.cpp | 12 +---- tests/gtest_any.cpp | 19 +++++++ tests/gtest_switch.cpp | 17 ++++++ tests/test_helper.hpp | 59 +++++++++++++++++++++ 5 files changed, 113 insertions(+), 11 deletions(-) diff --git a/include/behaviortree_cpp/utils/safe_any.hpp b/include/behaviortree_cpp/utils/safe_any.hpp index 4a0708896..7a9a9cb53 100644 --- a/include/behaviortree_cpp/utils/safe_any.hpp +++ b/include/behaviortree_cpp/utils/safe_any.hpp @@ -25,6 +25,7 @@ #include #include +#include #include #include @@ -33,6 +34,12 @@ namespace BT static std::type_index UndefinedAnyType = typeid(nullptr); +// Declared here (defined in basic_types.cpp, documented in basic_types.h) so that +// Any can share the locale-independent double parser without including +// basic_types.h, which itself includes this header. +[[nodiscard]] bool parseDouble(std::string_view str, double& out, + bool require_full_consumption); + // Trait to detect std::shared_ptr types (used for polymorphic port support) template struct is_shared_ptr : std::false_type @@ -458,7 +465,15 @@ inline nonstd::expected Any::stringToNumber() const } if constexpr(std::is_floating_point_v) { - return std::stod(str.toStdString()); + // std::stod honors LC_NUMERIC, so under a locale that uses ',' as decimal + // separator "3.5" would silently parse as 3. parseDouble reproduces the + // std::from_chars semantics of the branch above, on every platform. + double value = 0.0; + if(parseDouble(str.toStdStringView(), value, /*require_full_consumption=*/false)) + { + return static_cast(value); + } + return nonstd::make_unexpected("Any failed string to number conversion"); } } catch(...) diff --git a/src/controls/switch_node.cpp b/src/controls/switch_node.cpp index 2bc00e4dc..0d2a880cb 100644 --- a/src/controls/switch_node.cpp +++ b/src/controls/switch_node.cpp @@ -77,16 +77,8 @@ bool CheckStringEquality(const std::string& v1, const std::string& v2, auto [ptr, ec] = std::from_chars(str.data(), end, result); return ec == std::errc() && ptr == end; #else - try - { - std::size_t pos = 0; - result = std::stod(str, &pos); - return pos == str.size(); - } - catch(...) - { - return false; - } + // locale-independent, unlike std::stod (see parseDouble) + return parseDouble(str, result, /*require_full_consumption=*/true); #endif }; double v1_real = 0; diff --git a/tests/gtest_any.cpp b/tests/gtest_any.cpp index 1086f9b06..74da3454a 100644 --- a/tests/gtest_any.cpp +++ b/tests/gtest_any.cpp @@ -10,6 +10,8 @@ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +#include "test_helper.hpp" + #include // std::{from_chars,from_chars_result}, #include #include // std::errc. @@ -250,3 +252,20 @@ TEST(Any, Cast) EXPECT_EQ(a.cast>(), v); } } + +TEST(Any, StringToNumberIgnoresLocale) +{ + // The string -> number conversion must use '.' as decimal separator + // regardless of the locale selected by the host application. + const ScopedCommaDecimalLocale comma_locale; + if(!comma_locale.active()) + { + GTEST_SKIP() << "no locale with ',' as decimal separator on this machine"; + } + + EXPECT_DOUBLE_EQ(Any(std::string("3.5")).cast(), 3.5); + EXPECT_DOUBLE_EQ(Any(std::string("-0.25")).cast(), -0.25); + EXPECT_FLOAT_EQ(Any(std::string("1.5e2")).cast(), 150.0f); + EXPECT_EQ(Any(std::string("42")).cast(), 42); + EXPECT_ANY_THROW(auto res = Any(std::string("fifty")).cast()); +} diff --git a/tests/gtest_switch.cpp b/tests/gtest_switch.cpp index ac75b3c61..04f18c263 100644 --- a/tests/gtest_switch.cpp +++ b/tests/gtest_switch.cpp @@ -1,5 +1,6 @@ #include "action_test_node.h" #include "condition_test_node.h" +#include "test_helper.hpp" #include "behaviortree_cpp/behavior_tree.h" #include "behaviortree_cpp/bt_factory.h" @@ -264,3 +265,19 @@ TEST(SwitchStringEquality, RejectsTrailingCharacters) EXPECT_FALSE(CheckStringEquality("5 ", "5", nullptr)); EXPECT_FALSE(CheckStringEquality("none", "1", nullptr)); } + +TEST(SwitchStringEquality, RealComparisonIgnoresLocale) +{ + using BT::details::CheckStringEquality; + + const ScopedCommaDecimalLocale comma_locale; + if(!comma_locale.active()) + { + GTEST_SKIP() << "no locale with ',' as decimal separator on this machine"; + } + + EXPECT_TRUE(CheckStringEquality("3.50", "3.5", nullptr)); + EXPECT_TRUE(CheckStringEquality("5", "5.0", nullptr)); + EXPECT_FALSE(CheckStringEquality("3.5", "3", nullptr)); + EXPECT_FALSE(CheckStringEquality("0.5", "1", nullptr)); +} diff --git a/tests/test_helper.hpp b/tests/test_helper.hpp index d79123f68..c03b80fab 100644 --- a/tests/test_helper.hpp +++ b/tests/test_helper.hpp @@ -4,6 +4,65 @@ #include "behaviortree_cpp/bt_factory.h" #include +#include +#include + +#if !defined(_WIN32) +#include +#if defined(__APPLE__) +#include +#endif +#endif + +/** + * Switch the calling thread (and only this thread) to a locale whose decimal + * separator is ',' for the duration of the scope. Use active() to skip the test + * when no such locale is available on the machine. + */ +class ScopedCommaDecimalLocale +{ +public: + ScopedCommaDecimalLocale() + { +#if !defined(_WIN32) + locale_ = ::newlocale(LC_NUMERIC_MASK, "de_DE.UTF-8", static_cast<::locale_t>(0)); + if(locale_ != static_cast<::locale_t>(0)) + { + previous_ = ::uselocale(locale_); + } +#endif + } + + ~ScopedCommaDecimalLocale() + { +#if !defined(_WIN32) + if(locale_ != static_cast<::locale_t>(0)) + { + ::uselocale(previous_); + ::freelocale(locale_); + } +#endif + } + + ScopedCommaDecimalLocale(const ScopedCommaDecimalLocale&) = delete; + ScopedCommaDecimalLocale& operator=(const ScopedCommaDecimalLocale&) = delete; + + /// true if the C library now parses "0,5" as one half on this thread + [[nodiscard]] bool active() const + { +#if !defined(_WIN32) + return locale_ != static_cast<::locale_t>(0) && std::strtod("0,5", nullptr) == 0.5; +#else + return false; +#endif + } + +private: +#if !defined(_WIN32) + ::locale_t locale_ = static_cast<::locale_t>(0); + ::locale_t previous_ = static_cast<::locale_t>(0); +#endif +}; inline BT::NodeStatus TestTick(int* tick_counter) {