From 65e70b71909532d3fa24240effc88e9edb88c944 Mon Sep 17 00:00:00 2001 From: Junwang Zhao Date: Sun, 22 Feb 2026 15:22:43 +0800 Subject: [PATCH 1/4] fix: use strtod fallback when std::from_chars(float) unavailable closes #571 --- src/iceberg/util/string_util.h | 59 ++++++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 10 deletions(-) diff --git a/src/iceberg/util/string_util.h b/src/iceberg/util/string_util.h index 3528204ee..14b96d58b 100644 --- a/src/iceberg/util/string_util.h +++ b/src/iceberg/util/string_util.h @@ -20,12 +20,14 @@ #pragma once #include +#include #include +#include #include #include #include +#include #include -#include #include "iceberg/iceberg_export.h" #include "iceberg/result.h" @@ -71,19 +73,56 @@ class ICEBERG_EXPORT StringUtils { requires std::is_arithmetic_v && (!std::same_as) static Result ParseNumber(std::string_view str) { T value = 0; - auto [ptr, ec] = std::from_chars(str.data(), str.data() + str.size(), value); - if (ec == std::errc()) [[likely]] { - return value; - } - if (ec == std::errc::invalid_argument) { + if constexpr (std::is_integral_v) { + auto [ptr, ec] = std::from_chars(str.data(), str.data() + str.size(), value); + if (ec == std::errc() && ptr == str.data() + str.size()) [[likely]] { + return value; + } + if (ec == std::errc::result_out_of_range) { + return InvalidArgument("Failed to parse {} from string '{}': value out of range", + typeid(T).name(), str); + } return InvalidArgument("Failed to parse {} from string '{}': invalid argument", typeid(T).name(), str); - } - if (ec == std::errc::result_out_of_range) { - return InvalidArgument("Failed to parse {} from string '{}': value out of range", + } else { +// libc++ 20+ provides floating-point std::from_chars. Use fallback for older libc++ +#if defined(_LIBCPP_VERSION) && _LIBCPP_VERSION >= 200000 + auto [ptr, ec] = std::from_chars(str.data(), str.data() + str.size(), value); + if (ec == std::errc() && ptr == str.data() + str.size()) [[likely]] { + return value; + } + if (ec == std::errc::result_out_of_range) { + return InvalidArgument("Failed to parse {} from string '{}': value out of range", + typeid(T).name(), str); + } + return InvalidArgument("Failed to parse {} from string '{}': invalid argument", typeid(T).name(), str); +#else + // strto* require null-terminated input; string_view does not guarantee it. + std::string owned(str); + const char* start = owned.c_str(); + char* end = nullptr; + errno = 0; + + if constexpr (std::same_as) { + value = std::strtof(start, &end); + } else if constexpr (std::same_as) { + value = std::strtod(start, &end); + } else { + value = std::strtold(start, &end); + } + + if (end == start || end != start + static_cast(owned.size())) { + return InvalidArgument("Failed to parse {} from string '{}': invalid argument", + typeid(T).name(), str); + } + if (errno == ERANGE) { + return InvalidArgument("Failed to parse {} from string '{}': value out of range", + typeid(T).name(), str); + } + return value; +#endif } - std::unreachable(); } }; From 0c7e4d599432f1e3d87c1819986f589854fd28e7 Mon Sep 17 00:00:00 2001 From: Junwang Zhao Date: Mon, 23 Feb 2026 23:03:26 +0800 Subject: [PATCH 2/4] try using SFINAE instead of macro --- src/iceberg/util/string_util.h | 45 +++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/src/iceberg/util/string_util.h b/src/iceberg/util/string_util.h index 14b96d58b..e2def1b48 100644 --- a/src/iceberg/util/string_util.h +++ b/src/iceberg/util/string_util.h @@ -70,23 +70,26 @@ class ICEBERG_EXPORT StringUtils { } template - requires std::is_arithmetic_v && (!std::same_as) + requires std::is_integral_v && (!std::same_as) static Result ParseNumber(std::string_view str) { - T value = 0; - if constexpr (std::is_integral_v) { - auto [ptr, ec] = std::from_chars(str.data(), str.data() + str.size(), value); - if (ec == std::errc() && ptr == str.data() + str.size()) [[likely]] { - return value; - } - if (ec == std::errc::result_out_of_range) { - return InvalidArgument("Failed to parse {} from string '{}': value out of range", - typeid(T).name(), str); - } - return InvalidArgument("Failed to parse {} from string '{}': invalid argument", + T value{}; + auto [ptr, ec] = std::from_chars(str.data(), str.data() + str.size(), value); + if (ec == std::errc() && ptr == str.data() + str.size()) [[likely]] { + return value; + } + if (ec == std::errc::result_out_of_range) { + return InvalidArgument("Failed to parse {} from string '{}': value out of range", typeid(T).name(), str); - } else { -// libc++ 20+ provides floating-point std::from_chars. Use fallback for older libc++ -#if defined(_LIBCPP_VERSION) && _LIBCPP_VERSION >= 200000 + } + return InvalidArgument("Failed to parse {} from string '{}': invalid argument", + typeid(T).name(), str); + } + + template + requires std::is_floating_point_v + static Result ParseNumber(std::string_view str) { + T value{}; + if constexpr (kHasFloatFromChars) { auto [ptr, ec] = std::from_chars(str.data(), str.data() + str.size(), value); if (ec == std::errc() && ptr == str.data() + str.size()) [[likely]] { return value; @@ -97,7 +100,7 @@ class ICEBERG_EXPORT StringUtils { } return InvalidArgument("Failed to parse {} from string '{}': invalid argument", typeid(T).name(), str); -#else + } else { // strto* require null-terminated input; string_view does not guarantee it. std::string owned(str); const char* start = owned.c_str(); @@ -121,9 +124,17 @@ class ICEBERG_EXPORT StringUtils { typeid(T).name(), str); } return value; -#endif } } + + private: + // libc++ 20+ provides floating-point std::from_chars; use strto* fallback for older + // versions. +#if defined(_LIBCPP_VERSION) && _LIBCPP_VERSION < 200000 + static constexpr bool kHasFloatFromChars = false; +#else + static constexpr bool kHasFloatFromChars = true; +#endif }; /// \brief Transparent hash function that supports std::string_view as lookup key From 82a0dfa91d7f98f3ce6276f08b041aa35e173a90 Mon Sep 17 00:00:00 2001 From: Junwang Zhao Date: Tue, 24 Feb 2026 17:44:46 +0800 Subject: [PATCH 3/4] fix: use concept instead of MACRO --- src/iceberg/util/string_util.h | 82 ++++++++++++++-------------------- 1 file changed, 33 insertions(+), 49 deletions(-) diff --git a/src/iceberg/util/string_util.h b/src/iceberg/util/string_util.h index e2def1b48..c623b0f59 100644 --- a/src/iceberg/util/string_util.h +++ b/src/iceberg/util/string_util.h @@ -22,18 +22,21 @@ #include #include #include -#include #include #include #include #include #include +#include #include "iceberg/iceberg_export.h" #include "iceberg/result.h" namespace iceberg { +template +concept FromChars = requires(const char* p, T& v) { std::from_chars(p, p, v); }; + class ICEBERG_EXPORT StringUtils { public: static std::string ToLower(std::string_view str) { @@ -70,71 +73,52 @@ class ICEBERG_EXPORT StringUtils { } template - requires std::is_integral_v && (!std::same_as) + requires std::is_arithmetic_v && (!std::same_as) static Result ParseNumber(std::string_view str) { - T value{}; + T value = 0; auto [ptr, ec] = std::from_chars(str.data(), str.data() + str.size(), value); - if (ec == std::errc() && ptr == str.data() + str.size()) [[likely]] { + if (ec == std::errc()) [[likely]] { return value; } + if (ec == std::errc::invalid_argument) { + return InvalidArgument("Failed to parse {} from string '{}': invalid argument", + typeid(T).name(), str); + } if (ec == std::errc::result_out_of_range) { return InvalidArgument("Failed to parse {} from string '{}': value out of range", typeid(T).name(), str); } - return InvalidArgument("Failed to parse {} from string '{}': invalid argument", - typeid(T).name(), str); + std::unreachable(); } template - requires std::is_floating_point_v + requires std::is_floating_point_v && (!FromChars) static Result ParseNumber(std::string_view str) { T value{}; - if constexpr (kHasFloatFromChars) { - auto [ptr, ec] = std::from_chars(str.data(), str.data() + str.size(), value); - if (ec == std::errc() && ptr == str.data() + str.size()) [[likely]] { - return value; - } - if (ec == std::errc::result_out_of_range) { - return InvalidArgument("Failed to parse {} from string '{}': value out of range", - typeid(T).name(), str); - } - return InvalidArgument("Failed to parse {} from string '{}': invalid argument", - typeid(T).name(), str); + // strto* require null-terminated input; string_view does not guarantee it. + std::string owned(str); + const char* start = owned.c_str(); + char* end = nullptr; + errno = 0; + + if constexpr (std::same_as) { + value = std::strtof(start, &end); + } else if constexpr (std::same_as) { + value = std::strtod(start, &end); } else { - // strto* require null-terminated input; string_view does not guarantee it. - std::string owned(str); - const char* start = owned.c_str(); - char* end = nullptr; - errno = 0; - - if constexpr (std::same_as) { - value = std::strtof(start, &end); - } else if constexpr (std::same_as) { - value = std::strtod(start, &end); - } else { - value = std::strtold(start, &end); - } + value = std::strtold(start, &end); + } - if (end == start || end != start + static_cast(owned.size())) { - return InvalidArgument("Failed to parse {} from string '{}': invalid argument", - typeid(T).name(), str); - } - if (errno == ERANGE) { - return InvalidArgument("Failed to parse {} from string '{}': value out of range", - typeid(T).name(), str); - } - return value; + if (end == start || end != start + static_cast(owned.size())) { + return InvalidArgument("Failed to parse {} from string '{}': invalid argument", + typeid(T).name(), str); } + if (errno == ERANGE) { + return InvalidArgument("Failed to parse {} from string '{}': value out of range", + typeid(T).name(), str); + } + return value; } - - private: - // libc++ 20+ provides floating-point std::from_chars; use strto* fallback for older - // versions. -#if defined(_LIBCPP_VERSION) && _LIBCPP_VERSION < 200000 - static constexpr bool kHasFloatFromChars = false; -#else - static constexpr bool kHasFloatFromChars = true; -#endif }; /// \brief Transparent hash function that supports std::string_view as lookup key From e9e8aa648fa35d8460e8b6789019515b5c7bd6bf Mon Sep 17 00:00:00 2001 From: Junwang Zhao Date: Tue, 24 Feb 2026 18:57:50 +0800 Subject: [PATCH 4/4] fix: review comments --- src/iceberg/util/string_util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/iceberg/util/string_util.h b/src/iceberg/util/string_util.h index c623b0f59..fc202f0e6 100644 --- a/src/iceberg/util/string_util.h +++ b/src/iceberg/util/string_util.h @@ -73,7 +73,7 @@ class ICEBERG_EXPORT StringUtils { } template - requires std::is_arithmetic_v && (!std::same_as) + requires std::is_arithmetic_v && FromChars && (!std::same_as) static Result ParseNumber(std::string_view str) { T value = 0; auto [ptr, ec] = std::from_chars(str.data(), str.data() + str.size(), value);