From f389a8ce67e12bc4454edea6b928109a7ce5be9e Mon Sep 17 00:00:00 2001 From: Ethan Slattery Date: Wed, 24 Apr 2024 13:21:19 -0700 Subject: [PATCH 1/6] use std::to_char for formatting use std::to_char for formatting, fallback to constexpr formatting if needed and the compiler supports it. std::to_char is available widley except for floating point. Updated tests to expect the same value from runtime and constexpr format tests. std::to_char for floating point is the real edge case since it has poor compiler support. Where it is supported it seems most implementations are using a space-inefficient algorithm that can increase code size by about 1.2kB on ARM thumb architectures. This makes it unusable on embedded platforms. --- CMakeLists.txt | 1 + include/snitch/snitch_concepts.hpp | 6 ++ include/snitch/snitch_config.hpp.config | 3 + meson_options.txt | 1 + snitch/meson.build | 1 + src/snitch_append.cpp | 130 ++++++++++++++---------- tests/runtime_tests/string_utility.cpp | 43 +++----- 7 files changed, 106 insertions(+), 79 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ba301444..fed855fb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -23,6 +23,7 @@ set(SNITCH_WITH_MULTITHREADING ON CACHE BOOL "Make the testing fram set(SNITCH_WITH_TIMINGS ON CACHE BOOL "Measure the time taken by each test case -- disable to speed up tests.") set(SNITCH_WITH_SHORTHAND_MACROS ON CACHE BOOL "Use short names for test macros -- disable if this causes conflicts.") set(SNITCH_CONSTEXPR_FLOAT_USE_BITCAST ON CACHE BOOL "Use std::bit_cast if available to implement exact constexpr float-to-string conversion.") +set(SNITCH_APPEND_TO_CHARS ON CACHE BOOL "Use std::to_char for string conversions -- disable for greater compatability with a slight performance cost.") set(SNITCH_DEFAULT_WITH_COLOR ON CACHE BOOL "Enable terminal colors by default -- can also be controlled by command line interface.") set(SNITCH_DECOMPOSE_SUCCESSFUL_ASSERTIONS OFF CACHE BOOL "Enable expression decomposition even for successful assertions -- more expensive.") set(SNITCH_WITH_ALL_REPORTERS ON CACHE BOOL "Allow all built-in reporters to be selected from the command line -- disable for faster compilation.") diff --git a/include/snitch/snitch_concepts.hpp b/include/snitch/snitch_concepts.hpp index 334d15fc..53bd5650 100644 --- a/include/snitch/snitch_concepts.hpp +++ b/include/snitch/snitch_concepts.hpp @@ -12,12 +12,18 @@ concept signed_integral = std::is_signed_v; template concept unsigned_integral = std::is_unsigned_v; +template +concept integral = std::is_integral_v; + template concept floating_point = std::is_floating_point_v; template concept convertible_to = std::is_convertible_v; +template +concept same_as = std::is_same_v; + template concept enumeration = std::is_enum_v; diff --git a/include/snitch/snitch_config.hpp.config b/include/snitch/snitch_config.hpp.config index 2573f126..69ffd632 100644 --- a/include/snitch/snitch_config.hpp.config +++ b/include/snitch/snitch_config.hpp.config @@ -65,6 +65,9 @@ #if !defined(SNITCH_CONSTEXPR_FLOAT_USE_BITCAST) #cmakedefine01 SNITCH_CONSTEXPR_FLOAT_USE_BITCAST #endif +#if !defined(SNITCH_APPEND_TO_CHARS) +#cmakedefine01 SNITCH_APPEND_TO_CHARS +#endif #if !defined(SNITCH_DECOMPOSE_SUCCESSFUL_ASSERTIONS) #cmakedefine01 SNITCH_DECOMPOSE_SUCCESSFUL_ASSERTIONS #endif diff --git a/meson_options.txt b/meson_options.txt index cf889630..a79ce100 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -19,6 +19,7 @@ option('with_multithreading' ,type: 'boolean', value: true, descript option('with_timings' ,type: 'boolean' ,value: true, description: 'Measure the time taken by each test case -- disable to speed up tests.') option('with_shorthand_macros' ,type: 'boolean' ,value: true, description: 'Use short names for test macros -- disable if this causes conflicts.') option('constexpr_float_use_bitcast' ,type: 'boolean' ,value: true, description: 'Use std::bit_cast if available to implement exact constexpr float-to-string conversion.') +option('snitch_append_to_chars' ,type: 'boolean' ,value: true, description: 'Use std::to_char for string conversions -- disable for greater compatability with a slight performance cost.') option('default_with_color' ,type: 'boolean' ,value: true, description: 'Enable terminal colors by default -- can also be controlled by command line interface.') option('decompose_successful_assertions' ,type: 'boolean' ,value: true, description: 'Enable expression decomposition even for successful assertions -- more expensive.') option('with_all_reporters' ,type: 'boolean' ,value: true, description: 'Allow all built-in reporters to be selected from the command line -- disable for faster compilation.') diff --git a/snitch/meson.build b/snitch/meson.build index 9349f3a9..b9f958cd 100644 --- a/snitch/meson.build +++ b/snitch/meson.build @@ -38,6 +38,7 @@ conf_data = configuration_data({ 'SNITCH_WITH_TIMINGS' : get_option('with_timings').to_int(), 'SNITCH_WITH_SHORTHAND_MACROS' : get_option('with_shorthand_macros').to_int(), 'SNITCH_CONSTEXPR_FLOAT_USE_BITCAST' : get_option('constexpr_float_use_bitcast').to_int(), + 'SNITCH_APPEND_TO_CHARS' : get_option('snitch_append_to_chars').to_int(), 'SNITCH_DEFAULT_WITH_COLOR' : get_option('default_with_color').to_int(), 'SNITCH_DECOMPOSE_SUCCESSFUL_ASSERTIONS' : get_option('decompose_successful_assertions').to_int(), 'SNITCH_WITH_ALL_REPORTERS' : get_option('with_all_reporters').to_int(), diff --git a/src/snitch_append.cpp b/src/snitch_append.cpp index 53426674..dd66cdb2 100644 --- a/src/snitch_append.cpp +++ b/src/snitch_append.cpp @@ -1,65 +1,93 @@ #include "snitch/snitch_append.hpp" -#include // for format strings -#include // for std::snprintf +#include "snitch/snitch_concepts.hpp" +#include "snitch/snitch_string.hpp" + +#include // for std::to_chars +#include // for std::uintptr_t #include // for std::memmove +#include // for std::errc namespace snitch::impl { namespace { using snitch::small_string_span; -template -constexpr const char* get_format_code() noexcept { - if constexpr (std::is_same_v) { -#if defined(_MSC_VER) - return "0x%p"; -#else - return "%p"; -#endif - } else if constexpr (std::is_same_v) { - return "%" PRIuMAX; - } else if constexpr (std::is_same_v) { - return "%" PRIdMAX; - } else if constexpr (std::is_same_v) { - return "%.6e"; - } else if constexpr (std::is_same_v) { - return "%.15e"; - } else { - static_assert(!std::is_same_v, "unsupported type"); - } -} - -template -bool append_fmt(small_string_span ss, T value) noexcept { - if (ss.available() <= 1) { - // snprintf will always print a null-terminating character, - // so abort early if only space for one or zero character, as - // this would clobber the original string. - return false; +#if SNITCH_APPEND_TO_CHARS +// libstdc++ version 11 or greater +// libc++ version 14 or greater +// MSVC 19.24 or greater (also known as Visual Studio 2019 16.4) +// Apple clang and others have no support as of April-2024 +# if (defined(_GLIBCXX_RELEASE) && _GLIBCXX_RELEASE > 11) || \ + (defined(_LIBCPP_VERSION) && _LIBCPP_VERSION > 14000) || \ + (defined(_MSC_VER) && _MSC_VER > 1924) +template +bool append_to(small_string_span ss, T value) noexcept { + constexpr auto fmt = std::chars_format::scientific; + constexpr auto precision = same_as> ? 6 : 15; + auto [end, err] = std::to_chars(ss.end(), ss.begin() + ss.capacity(), value, fmt, precision); + if (err != std::errc{}) { + // Not enough space, try into a temporary string that *should* be big enough, + // and copy whatever we can. 32 characters is enough for all integers and floating + // point values encoded on 64 bit or less. + small_string<32> fallback; + auto [end2, err2] = std::to_chars( + fallback.end(), fallback.begin() + fallback.capacity(), value, fmt, precision); + if (err2 != std::errc{}) { + return false; + } + fallback.grow(end2 - fallback.begin()); + return append(ss, fallback); } - // Calculate required length. - const int return_code = std::snprintf(nullptr, 0, get_format_code(), value); - if (return_code < 0) { - return false; + ss.grow(end - ss.end()); + return true; +} +# else +template +bool append_to(small_string_span ss, T value) noexcept { + return append_constexpr(ss, value); +} +# endif + +template +bool append_to(small_string_span ss, T value) noexcept { + auto [end, err] = std::to_chars(ss.end(), ss.begin() + ss.capacity(), value, Base); + if (err != std::errc{}) { + // Not enough space, try into a temporary string that *should* be big enough, + // and copy whatever we can. 32 characters is enough for all integers and floating + // point values encoded on 64 bit or less. + small_string<32> fallback; + auto [end2, err2] = + std::to_chars(fallback.end(), fallback.begin() + fallback.capacity(), value, Base); + if (err2 != std::errc{}) { + return false; + } + fallback.grow(end2 - fallback.begin()); + return append(ss, fallback); } + ss.grow(end - ss.end()); + return true; +} - // 'return_code' holds the number of characters that are required, - // excluding the null-terminating character, which always gets appended, - // so we need to +1. - const std::size_t length = static_cast(return_code) + 1; - const bool could_fit = length <= ss.available(); +bool append_to(small_string_span ss, const void* ptr) noexcept { + return append(ss, "0x") && append_to<16>(ss, reinterpret_cast(ptr)); +} - const std::size_t offset = ss.size(); - const std::size_t prev_space = ss.available(); - ss.resize(std::min(ss.size() + length, ss.capacity())); - std::snprintf(ss.begin() + offset, prev_space, get_format_code(), value); +#else +template +bool append_to(small_string_span ss, T value) noexcept { + return append_constexpr(ss, value); +} - // Pop the null-terminating character, always printed unfortunately. - ss.pop_back(); +template +bool append_to(small_string_span ss, T value) noexcept { + return append_constexpr(ss, value); +} - return could_fit; +bool append_to(small_string_span ss, const void* value) noexcept { + return append_constexpr(ss, value); } +#endif } // namespace bool append_fast(small_string_span ss, std::string_view str) noexcept { @@ -81,23 +109,23 @@ bool append_fast(small_string_span ss, const void* ptr) noexcept { if (ptr == nullptr) { return append(ss, nullptr); } else { - return append_fmt(ss, ptr); + return append(ss, "0x") && append_to(ss, ptr); } } bool append_fast(small_string_span ss, large_uint_t i) noexcept { - return append_fmt(ss, i); + return append_to(ss, i); } bool append_fast(small_string_span ss, large_int_t i) noexcept { - return append_fmt(ss, i); + return append_to(ss, i); } bool append_fast(small_string_span ss, float f) noexcept { - return append_fmt(ss, f); + return append_to(ss, f); } bool append_fast(small_string_span ss, double d) noexcept { - return append_fmt(ss, d); + return append_to(ss, d); } } // namespace snitch::impl diff --git a/tests/runtime_tests/string_utility.cpp b/tests/runtime_tests/string_utility.cpp index 14173fb3..e7b4543b 100644 --- a/tests/runtime_tests/string_utility.cpp +++ b/tests/runtime_tests/string_utility.cpp @@ -247,8 +247,7 @@ TEST_CASE("append misc", "[utility]") { } TEST_CASE("append ints", "[utility]") { - using ae = append_test::append_expected; - using aed = append_test::append_expected_diff; + using ae = append_test::append_expected; SECTION("integers do fit") { constexpr auto a = [](const auto& value) constexpr { @@ -312,18 +311,15 @@ TEST_CASE("append ints", "[utility]") { } }; - // Different expectation at runtime and compile-time. At runtime, - // we are stuck with snprintf, which insists on writing a null-terminator character, - // therefore we loose one character at the end. - CONSTEXPR_CHECK(a(123456) == aed{{"12345"sv, false}, {"1234"sv, false}}); - CONSTEXPR_CHECK(a(1234567) == aed{{"12345"sv, false}, {"1234"sv, false}}); - CONSTEXPR_CHECK(a(12345678) == aed{{"12345"sv, false}, {"1234"sv, false}}); - CONSTEXPR_CHECK(a(-12345) == aed{{"-1234"sv, false}, {"-123"sv, false}}); - CONSTEXPR_CHECK(a(-123456) == aed{{"-1234"sv, false}, {"-123"sv, false}}); - CONSTEXPR_CHECK(a(-1234567) == aed{{"-1234"sv, false}, {"-123"sv, false}}); - CONSTEXPR_CHECK(a(123456u) == aed{{"12345"sv, false}, {"1234"sv, false}}); - CONSTEXPR_CHECK(a(1234567u) == aed{{"12345"sv, false}, {"1234"sv, false}}); - CONSTEXPR_CHECK(a(12345678u) == aed{{"12345"sv, false}, {"1234"sv, false}}); + CONSTEXPR_CHECK(a(123456) == ae{"12345"sv, false}); + CONSTEXPR_CHECK(a(1234567) == ae{"12345"sv, false}); + CONSTEXPR_CHECK(a(12345678) == ae{"12345"sv, false}); + CONSTEXPR_CHECK(a(-12345) == ae{"-1234"sv, false}); + CONSTEXPR_CHECK(a(-123456) == ae{"-1234"sv, false}); + CONSTEXPR_CHECK(a(-1234567) == ae{"-1234"sv, false}); + CONSTEXPR_CHECK(a(123456u) == ae{"12345"sv, false}); + CONSTEXPR_CHECK(a(1234567u) == ae{"12345"sv, false}); + CONSTEXPR_CHECK(a(12345678u) == ae{"12345"sv, false}); } SECTION("enums do fit") { @@ -341,10 +337,7 @@ TEST_CASE("append ints", "[utility]") { return append_test::to_string<3, false>(value); }; - // Different expectation at runtime and compile-time. At runtime, - // we are stuck with snprintf, which insists on writing a null-terminator character, - // therefore we loose one character at the end. - CONSTEXPR_CHECK(a(enum_type::value3) == aed{{"123", false}, {"12", false}}); + CONSTEXPR_CHECK(a(enum_type::value3) == ae{"123", false}); } } @@ -422,11 +415,8 @@ TEST_CASE("append floats", "[utility]") { return append_test::to_string<5, true>(value); }; - // Different expectation at runtime and compile-time. At runtime, - // we are stuck with snprintf, which insists on writing a null-terminator character, - // therefore we loose one character at the end. - CONSTEXPR_CHECK(a(0.0f) == aed{{"0.000"sv, false}, {"0.00"sv, false}}); - CONSTEXPR_CHECK(a(-1.0f) == aed{{"-1.00"sv, false}, {"-1.0"sv, false}}); + CONSTEXPR_CHECK(a(0.0f) == ae{"0.000"sv, false}); + CONSTEXPR_CHECK(a(-1.0f) == ae{"-1.00"sv, false}); } #if 0 @@ -539,11 +529,8 @@ TEST_CASE("append doubles", "[utility]") { return append_test::to_string<5, true>(value); }; - // Different expectation at runtime and compile-time. At runtime, - // we are stuck with snprintf, which insists on writing a null-terminator character, - // therefore we loose one character at the end. - CONSTEXPR_CHECK(a(0.0) == aed{{"0.000"sv, false}, {"0.00"sv, false}}); - CONSTEXPR_CHECK(a(-1.0) == aed{{"-1.00"sv, false}, {"-1.0"sv, false}}); + CONSTEXPR_CHECK(a(0.0) == ae{"0.000"sv, false}); + CONSTEXPR_CHECK(a(-1.0) == ae{"-1.00"sv, false}); } } From 4f06a3770552e2d209d078543e11e60ad2d20064 Mon Sep 17 00:00:00 2001 From: Corentin Schreiber Date: Sat, 27 Apr 2024 20:35:47 +0100 Subject: [PATCH 2/6] cherry pick append_fast pointer implementation constexpr append changes to take into account the base of string conversion so that pointers can be printed out in hex format and with padding zeros. --- include/snitch/snitch_append.hpp | 36 ++++++++++++++---------- src/snitch_append.cpp | 38 ++++++++++++++++---------- tests/runtime_tests/string_utility.cpp | 8 ++++-- 3 files changed, 51 insertions(+), 31 deletions(-) diff --git a/include/snitch/snitch_append.hpp b/include/snitch/snitch_append.hpp index 86c37ee3..8fc39f24 100644 --- a/include/snitch/snitch_append.hpp +++ b/include/snitch/snitch_append.hpp @@ -45,25 +45,30 @@ SNITCH_EXPORT [[nodiscard]] bool append_fast(small_string_span ss, double f) noe return could_fit; } +template [[nodiscard]] constexpr std::size_t num_digits(large_uint_t x) noexcept { - return x >= 10u ? 1u + num_digits(x / 10u) : 1u; + return x >= Base ? 1u + num_digits(x / Base) : 1u; } +template [[nodiscard]] constexpr std::size_t num_digits(large_int_t x) noexcept { - return x >= 10 ? 1u + num_digits(x / 10) : x <= -10 ? 1u + num_digits(x / 10) : x > 0 ? 1u : 2u; + return (x >= Base || x <= -Base) ? 1u + num_digits(x / Base) : x > 0 ? 1u : 2u; } -constexpr std::array digits = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9'}; +constexpr std::array digits = {'0', '1', '2', '3', '4', '5', '6', '7', + '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'}; + constexpr std::size_t max_uint_length = num_digits(std::numeric_limits::max()); constexpr std::size_t max_int_length = max_uint_length + 1; +template [[nodiscard]] constexpr bool append_constexpr(small_string_span ss, large_uint_t i) noexcept { if (i != 0u) { small_string tmp; - tmp.resize(num_digits(i)); + tmp.resize(num_digits(i)); std::size_t k = 1; - for (large_uint_t j = i; j != 0u; j /= 10u, ++k) { - tmp[tmp.size() - k] = digits[j % 10u]; + for (large_uint_t j = i; j != 0u; j /= Base, ++k) { + tmp[tmp.size() - k] = digits[j % Base]; } return append_constexpr(ss, tmp); } else { @@ -71,21 +76,22 @@ constexpr std::size_t max_int_length = max_uint_length + 1; } } +template [[nodiscard]] constexpr bool append_constexpr(small_string_span ss, large_int_t i) noexcept { if (i > 0) { small_string tmp; - tmp.resize(num_digits(i)); + tmp.resize(num_digits(i)); std::size_t k = 1; - for (large_int_t j = i; j != 0; j /= 10, ++k) { - tmp[tmp.size() - k] = digits[j % 10]; + for (large_int_t j = i; j != 0; j /= Base, ++k) { + tmp[tmp.size() - k] = digits[j % Base]; } return append_constexpr(ss, tmp); } else if (i < 0) { small_string tmp; - tmp.resize(num_digits(i)); + tmp.resize(num_digits(i)); std::size_t k = 1; - for (large_int_t j = i; j != 0; j /= 10, ++k) { - tmp[tmp.size() - k] = digits[-(j % 10)]; + for (large_int_t j = i; j != 0; j /= Base, ++k) { + tmp[tmp.size() - k] = digits[-(j % Base)]; } tmp[0] = '-'; return append_constexpr(ss, tmp); @@ -98,7 +104,7 @@ constexpr std::size_t max_int_length = max_uint_length + 1; constexpr std::size_t min_exp_digits = 2u; [[nodiscard]] constexpr std::size_t num_exp_digits(fixed_exp_t x) noexcept { - const std::size_t exp_digits = num_digits(static_cast(x > 0 ? x : -x)); + const std::size_t exp_digits = num_digits<10>(static_cast(x > 0 ? x : -x)); return exp_digits < min_exp_digits ? min_exp_digits : exp_digits; } @@ -106,7 +112,7 @@ constexpr std::size_t min_exp_digits = 2u; // +1 for fractional separator '.' // +1 for exponent separator 'e' // +1 for exponent sign - return num_digits(static_cast(x.digits)) + num_exp_digits(x.exponent) + + return num_digits<10>(static_cast(x.digits)) + num_exp_digits(x.exponent) + (x.sign ? 1u : 0u) + 3u; } @@ -135,7 +141,7 @@ set_precision(signed_fixed_data fd, std::size_t p) noexcept { // and round-half-to-even is the default rounding mode for IEEE 754 floats. We don't follow // the current rounding mode, but we can at least follow the default. - std::size_t base_digits = num_digits(static_cast(fd.digits)); + std::size_t base_digits = num_digits<10>(static_cast(fd.digits)); bool only_zero = true; while (base_digits > p) { diff --git a/src/snitch_append.cpp b/src/snitch_append.cpp index dd66cdb2..fac4529e 100644 --- a/src/snitch_append.cpp +++ b/src/snitch_append.cpp @@ -11,6 +11,7 @@ namespace snitch::impl { namespace { using snitch::small_string_span; +using namespace std::literals; #if SNITCH_APPEND_TO_CHARS // libstdc++ version 11 or greater @@ -68,24 +69,15 @@ bool append_to(small_string_span ss, T value) noexcept { ss.grow(end - ss.end()); return true; } - -bool append_to(small_string_span ss, const void* ptr) noexcept { - return append(ss, "0x") && append_to<16>(ss, reinterpret_cast(ptr)); -} - #else -template +template bool append_to(small_string_span ss, T value) noexcept { return append_constexpr(ss, value); } -template +template bool append_to(small_string_span ss, T value) noexcept { - return append_constexpr(ss, value); -} - -bool append_to(small_string_span ss, const void* value) noexcept { - return append_constexpr(ss, value); + return append_constexpr(ss, value); } #endif } // namespace @@ -108,9 +100,27 @@ bool append_fast(small_string_span ss, std::string_view str) noexcept { bool append_fast(small_string_span ss, const void* ptr) noexcept { if (ptr == nullptr) { return append(ss, nullptr); - } else { - return append(ss, "0x") && append_to(ss, ptr); } + + if (!append_fast(ss, "0x"sv)) { + return false; + } + + const auto int_ptr = reinterpret_cast(ptr); + + // Pad with zeros. + constexpr std::size_t max_digits = 2 * sizeof(void*); + std::size_t padding = max_digits - num_digits<16>(int_ptr); + while (padding > 0) { + constexpr std::string_view zeroes = "0000000000000000"; + const std::size_t batch = std::min(zeroes.size(), padding); + if (!append_fast(ss, zeroes.substr(0, batch))) { + return false; + } + + padding -= batch; + } + return append_to<16>(ss, int_ptr); } bool append_fast(small_string_span ss, large_uint_t i) noexcept { diff --git a/tests/runtime_tests/string_utility.cpp b/tests/runtime_tests/string_utility.cpp index e7b4543b..fcf6a382 100644 --- a/tests/runtime_tests/string_utility.cpp +++ b/tests/runtime_tests/string_utility.cpp @@ -342,8 +342,10 @@ TEST_CASE("append ints", "[utility]") { } TEST_CASE("append floats", "[utility]") { - using ae = append_test::append_expected; + using ae = append_test::append_expected; +#if !SNITCH_CONSTEXPR_FLOAT_USE_BITCAST using aed = append_test::append_expected_diff; +#endif SECTION("floats do fit") { constexpr auto a = [](const auto& value) constexpr { @@ -450,8 +452,10 @@ TEST_CASE("append floats", "[utility]") { } TEST_CASE("append doubles", "[utility]") { - using ae = append_test::append_expected; + using ae = append_test::append_expected; +#if !SNITCH_CONSTEXPR_FLOAT_USE_BITCAST using aed = append_test::append_expected_diff; +#endif SECTION("doubles do fit") { constexpr auto a = [](const auto& value) constexpr { From 3aed57c3e4efac4770adc76ce278776cdd406fc8 Mon Sep 17 00:00:00 2001 From: Ethan Slattery Date: Sun, 28 Apr 2024 15:07:01 -0700 Subject: [PATCH 3/6] remove fallback for floating point std::to_char If the compiler doesn't support std::to_char forfloating point, then the compile time option should disable use of std::to_char for all conversions. This avoids mixing and matching implementations. Either append_constexpr is used for all conversion, or std::to_char is used for all conversions. This slightly complicates the testing as there is now a two variable truth table as to the expected results when formatting -0.0. --- include/snitch/snitch_config.hpp.config | 7 +++++++ src/snitch_append.cpp | 19 ++++--------------- tests/runtime_tests/string_utility.cpp | 20 ++++++++++++++++---- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/include/snitch/snitch_config.hpp.config b/include/snitch/snitch_config.hpp.config index 69ffd632..e8974880 100644 --- a/include/snitch/snitch_config.hpp.config +++ b/include/snitch/snitch_config.hpp.config @@ -114,6 +114,13 @@ # define SNITCH_CONSTEXPR_FLOAT_USE_BITCAST 0 #endif +#if (!defined(__cpp_lib_to_chars)) || (defined(_GLIBCXX_RELEASE) && _GLIBCXX_RELEASE <= 11) || \ + (defined(_LIBCPP_VERSION) && _LIBCPP_VERSION <= 14000) || \ + (defined(_MSC_VER) && _MSC_VER <= 1924) +# undef SNITCH_APPEND_TO_CHARS +# define SNITCH_APPEND_TO_CHARS 0 +#endif + #if SNITCH_SHARED_LIBRARY # if defined(_MSC_VER) # if defined(SNITCH_EXPORTS) diff --git a/src/snitch_append.cpp b/src/snitch_append.cpp index fac4529e..a6e41cc3 100644 --- a/src/snitch_append.cpp +++ b/src/snitch_append.cpp @@ -3,10 +3,12 @@ #include "snitch/snitch_concepts.hpp" #include "snitch/snitch_string.hpp" -#include // for std::to_chars #include // for std::uintptr_t #include // for std::memmove -#include // for std::errc +#if SNITCH_APPEND_TO_CHARS +# include // for std::to_chars +# include // for std::errc +#endif namespace snitch::impl { namespace { @@ -14,13 +16,6 @@ using snitch::small_string_span; using namespace std::literals; #if SNITCH_APPEND_TO_CHARS -// libstdc++ version 11 or greater -// libc++ version 14 or greater -// MSVC 19.24 or greater (also known as Visual Studio 2019 16.4) -// Apple clang and others have no support as of April-2024 -# if (defined(_GLIBCXX_RELEASE) && _GLIBCXX_RELEASE > 11) || \ - (defined(_LIBCPP_VERSION) && _LIBCPP_VERSION > 14000) || \ - (defined(_MSC_VER) && _MSC_VER > 1924) template bool append_to(small_string_span ss, T value) noexcept { constexpr auto fmt = std::chars_format::scientific; @@ -43,12 +38,6 @@ bool append_to(small_string_span ss, T value) noexcept { ss.grow(end - ss.end()); return true; } -# else -template -bool append_to(small_string_span ss, T value) noexcept { - return append_constexpr(ss, value); -} -# endif template bool append_to(small_string_span ss, T value) noexcept { diff --git a/tests/runtime_tests/string_utility.cpp b/tests/runtime_tests/string_utility.cpp index fcf6a382..faf3d6a5 100644 --- a/tests/runtime_tests/string_utility.cpp +++ b/tests/runtime_tests/string_utility.cpp @@ -343,7 +343,9 @@ TEST_CASE("append ints", "[utility]") { TEST_CASE("append floats", "[utility]") { using ae = append_test::append_expected; -#if !SNITCH_CONSTEXPR_FLOAT_USE_BITCAST +#if !SNITCH_CONSTEXPR_FLOAT_USE_BITCAST && SNITCH_APPEND_TO_CHARS + // answers will be different only if no bitcast AND we do have runtime to_char + // otherwise append_constexpr will be used at runtime using aed = append_test::append_expected_diff; #endif @@ -354,11 +356,16 @@ TEST_CASE("append floats", "[utility]") { CONSTEXPR_CHECK(a(0.0f) == ae{"0.000000e+00"sv, true}); #if SNITCH_CONSTEXPR_FLOAT_USE_BITCAST + // std::bit_cast is enabled, and will match the output of std::to_char if used at runtime CONSTEXPR_CHECK(a(-0.0f) == ae{"-0.000000e+00"sv, true}); -#else +#elif SNITCH_APPEND_TO_CHARS // Without std::bit_cast (or C++23), we are unable to tell the difference between -0.0f and // +0.0f in constexpr expressions. Therefore -0.0f in constexpr gets displayed as +0.0f. CONSTEXPR_CHECK(a(-0.0f) == aed{{"0.000000e+00"sv, true}, {"-0.000000e+00"sv, true}}); +#else + // No std::bit_cast, but also no std::to_char. append_constexpr will be used + // for runtime and match the compile time results + CONSTEXPR_CHECK(a(-0.0f) == ae{"0.000000e+00"sv, true}); #endif CONSTEXPR_CHECK(a(1.0f) == ae{"1.000000e+00"sv, true}); CONSTEXPR_CHECK(a(1.5f) == ae{"1.500000e+00"sv, true}); @@ -453,7 +460,7 @@ TEST_CASE("append floats", "[utility]") { TEST_CASE("append doubles", "[utility]") { using ae = append_test::append_expected; -#if !SNITCH_CONSTEXPR_FLOAT_USE_BITCAST +#if !SNITCH_CONSTEXPR_FLOAT_USE_BITCAST && SNITCH_APPEND_TO_CHARS using aed = append_test::append_expected_diff; #endif @@ -464,12 +471,17 @@ TEST_CASE("append doubles", "[utility]") { CONSTEXPR_CHECK(a(0.0) == ae{"0.000000000000000e+00"sv, true}); #if SNITCH_CONSTEXPR_FLOAT_USE_BITCAST + // std::bit_cast is enabled, and will match the output of std::to_char if used at runtime CONSTEXPR_CHECK(a(-0.0) == ae{"-0.000000000000000e+00"sv, true}); -#else +#elif SNITCH_APPEND_TO_CHARS // Without std::bit_cast (or C++23), we are unable to tell the difference between -0.0f and // +0.0f in constexpr expressions. Therefore -0.0f in constexpr gets displayed as +0.0f. CONSTEXPR_CHECK( a(-0.0) == aed{{"0.000000000000000e+00"sv, true}, {"-0.000000000000000e+00"sv, true}}); +#else + // No std::bit_cast, but also no std::to_char. append_constexpr will be used for + // runtime and match the compile time results + CONSTEXPR_CHECK(a(-0.0) == ae{"0.000000000000000e+00"sv, true}); #endif CONSTEXPR_CHECK(a(1.0) == ae{"1.000000000000000e+00"sv, true}); CONSTEXPR_CHECK(a(1.5) == ae{"1.500000000000000e+00"sv, true}); From dc67a89948cd032b0ad4be0e9fe65f3b06bec471 Mon Sep 17 00:00:00 2001 From: Ethan Slattery Date: Sun, 28 Apr 2024 15:38:57 -0700 Subject: [PATCH 4/6] Fix ambiguous overloads on 32-bit platforms On the 32-bit platforms in CI (windows and emscripten) large_int_t and large_uint_t seem to alias each other in function definitions. Fix this by using signed/unsigned concepts for the parameter. That way the incorrect function will not be considered. This required small bug fix in snitch::concepts. signed_integral and unsigned_integral concepts need to check for sign and also make sure the type is integral so that floating point or other types are not considered. --- include/snitch/snitch_append.hpp | 16 ++++++++-------- include/snitch/snitch_concepts.hpp | 7 ++++--- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/include/snitch/snitch_append.hpp b/include/snitch/snitch_append.hpp index 8fc39f24..5a0c6848 100644 --- a/include/snitch/snitch_append.hpp +++ b/include/snitch/snitch_append.hpp @@ -45,13 +45,13 @@ SNITCH_EXPORT [[nodiscard]] bool append_fast(small_string_span ss, double f) noe return could_fit; } -template -[[nodiscard]] constexpr std::size_t num_digits(large_uint_t x) noexcept { +template +[[nodiscard]] constexpr std::size_t num_digits(T x) noexcept { return x >= Base ? 1u + num_digits(x / Base) : 1u; } -template -[[nodiscard]] constexpr std::size_t num_digits(large_int_t x) noexcept { +template +[[nodiscard]] constexpr std::size_t num_digits(T x) noexcept { return (x >= Base || x <= -Base) ? 1u + num_digits(x / Base) : x > 0 ? 1u : 2u; } @@ -61,8 +61,8 @@ constexpr std::array digits = {'0', '1', '2', '3', '4', '5', '6', '7', constexpr std::size_t max_uint_length = num_digits(std::numeric_limits::max()); constexpr std::size_t max_int_length = max_uint_length + 1; -template -[[nodiscard]] constexpr bool append_constexpr(small_string_span ss, large_uint_t i) noexcept { +template +[[nodiscard]] constexpr bool append_constexpr(small_string_span ss, T i) noexcept { if (i != 0u) { small_string tmp; tmp.resize(num_digits(i)); @@ -76,8 +76,8 @@ template } } -template -[[nodiscard]] constexpr bool append_constexpr(small_string_span ss, large_int_t i) noexcept { +template +[[nodiscard]] constexpr bool append_constexpr(small_string_span ss, T i) noexcept { if (i > 0) { small_string tmp; tmp.resize(num_digits(i)); diff --git a/include/snitch/snitch_concepts.hpp b/include/snitch/snitch_concepts.hpp index 53bd5650..1cde48d4 100644 --- a/include/snitch/snitch_concepts.hpp +++ b/include/snitch/snitch_concepts.hpp @@ -6,14 +6,15 @@ #include namespace snitch { + template -concept signed_integral = std::is_signed_v; +concept integral = std::is_integral_v; template -concept unsigned_integral = std::is_unsigned_v; +concept signed_integral = integral && std::is_signed_v; template -concept integral = std::is_integral_v; +concept unsigned_integral = integral && std::is_unsigned_v; template concept floating_point = std::is_floating_point_v; From dea6d07e7288cd77233b3da876b2b982c397f3f0 Mon Sep 17 00:00:00 2001 From: Corentin Schreiber Date: Mon, 29 Apr 2024 08:41:03 +0100 Subject: [PATCH 5/6] Fix 1e-6 not having the right number of digits --- include/snitch/snitch_append.hpp | 5 +++-- tests/runtime_tests/string_utility.cpp | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/include/snitch/snitch_append.hpp b/include/snitch/snitch_append.hpp index 5a0c6848..7914b545 100644 --- a/include/snitch/snitch_append.hpp +++ b/include/snitch/snitch_append.hpp @@ -150,12 +150,13 @@ set_precision(signed_fixed_data fd, std::size_t p) noexcept { only_zero = false; } fd.digits = fd.digits / 10u; + base_digits -= 1u; } else { - fd.digits = round_half_to_even(fd.digits, only_zero); + fd.digits = round_half_to_even(fd.digits, only_zero); + base_digits = num_digits(static_cast(fd.digits)); } fd.exponent += 1; - base_digits -= 1u; } return fd; diff --git a/tests/runtime_tests/string_utility.cpp b/tests/runtime_tests/string_utility.cpp index faf3d6a5..2bb26f6d 100644 --- a/tests/runtime_tests/string_utility.cpp +++ b/tests/runtime_tests/string_utility.cpp @@ -377,6 +377,7 @@ TEST_CASE("append floats", "[utility]") { CONSTEXPR_CHECK(a(-1.0f) == ae{"-1.000000e+00"sv, true}); CONSTEXPR_CHECK(a(10.0f) == ae{"1.000000e+01"sv, true}); CONSTEXPR_CHECK(a(1e4f) == ae{"1.000000e+04"sv, true}); + CONSTEXPR_CHECK(a(1e-6f) == ae{"1.000000e-06"sv, true}); // The number below is a tricky one: it is exactly representable, but intermediate // calculations requires more digits than can be stored on fixed-point 64 bits. // Furthermore, rounding is an exact tie, and exposes the round-half-to-even behavior. @@ -502,6 +503,7 @@ TEST_CASE("append doubles", "[utility]") { CONSTEXPR_CHECK(a(-1.0) == ae{"-1.000000000000000e+00"sv, true}); CONSTEXPR_CHECK(a(10.0) == ae{"1.000000000000000e+01"sv, true}); CONSTEXPR_CHECK(a(1e4) == ae{"1.000000000000000e+04"sv, true}); + CONSTEXPR_CHECK(a(1e-6) == ae{"1.000000000000000e-06"sv, true}); CONSTEXPR_CHECK(a(2.3456e301) == ae{"2.345600000000000e+301"sv, true}); CONSTEXPR_CHECK(a(-2.3456e301) == ae{"-2.345600000000000e+301"sv, true}); CONSTEXPR_CHECK(a(1.797693134862315e308) == ae{"1.797693134862315e+308"sv, true}); From 4096a6c17e75d02e661ea9b69caf4e2c2f13db87 Mon Sep 17 00:00:00 2001 From: Ethan Slattery Date: Wed, 1 May 2024 20:27:37 -0700 Subject: [PATCH 6/6] Fixes from merge request review Explicit base template parameter on num_digits in set_precision function. Also change all instances of `std::to_char` to the correct spelling `std::to_chars`. --- CMakeLists.txt | 2 +- include/snitch/snitch_append.hpp | 2 +- meson_options.txt | 2 +- tests/runtime_tests/string_utility.cpp | 10 +++++----- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fed855fb..4913ed7a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -23,7 +23,7 @@ set(SNITCH_WITH_MULTITHREADING ON CACHE BOOL "Make the testing fram set(SNITCH_WITH_TIMINGS ON CACHE BOOL "Measure the time taken by each test case -- disable to speed up tests.") set(SNITCH_WITH_SHORTHAND_MACROS ON CACHE BOOL "Use short names for test macros -- disable if this causes conflicts.") set(SNITCH_CONSTEXPR_FLOAT_USE_BITCAST ON CACHE BOOL "Use std::bit_cast if available to implement exact constexpr float-to-string conversion.") -set(SNITCH_APPEND_TO_CHARS ON CACHE BOOL "Use std::to_char for string conversions -- disable for greater compatability with a slight performance cost.") +set(SNITCH_APPEND_TO_CHARS ON CACHE BOOL "Use std::to_chars for string conversions -- disable for greater compatability with a slight performance cost.") set(SNITCH_DEFAULT_WITH_COLOR ON CACHE BOOL "Enable terminal colors by default -- can also be controlled by command line interface.") set(SNITCH_DECOMPOSE_SUCCESSFUL_ASSERTIONS OFF CACHE BOOL "Enable expression decomposition even for successful assertions -- more expensive.") set(SNITCH_WITH_ALL_REPORTERS ON CACHE BOOL "Allow all built-in reporters to be selected from the command line -- disable for faster compilation.") diff --git a/include/snitch/snitch_append.hpp b/include/snitch/snitch_append.hpp index 7914b545..1a0f4f7f 100644 --- a/include/snitch/snitch_append.hpp +++ b/include/snitch/snitch_append.hpp @@ -153,7 +153,7 @@ set_precision(signed_fixed_data fd, std::size_t p) noexcept { base_digits -= 1u; } else { fd.digits = round_half_to_even(fd.digits, only_zero); - base_digits = num_digits(static_cast(fd.digits)); + base_digits = num_digits<10>(static_cast(fd.digits)); } fd.exponent += 1; diff --git a/meson_options.txt b/meson_options.txt index a79ce100..097599d2 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -19,7 +19,7 @@ option('with_multithreading' ,type: 'boolean', value: true, descript option('with_timings' ,type: 'boolean' ,value: true, description: 'Measure the time taken by each test case -- disable to speed up tests.') option('with_shorthand_macros' ,type: 'boolean' ,value: true, description: 'Use short names for test macros -- disable if this causes conflicts.') option('constexpr_float_use_bitcast' ,type: 'boolean' ,value: true, description: 'Use std::bit_cast if available to implement exact constexpr float-to-string conversion.') -option('snitch_append_to_chars' ,type: 'boolean' ,value: true, description: 'Use std::to_char for string conversions -- disable for greater compatability with a slight performance cost.') +option('snitch_append_to_chars' ,type: 'boolean' ,value: true, description: 'Use std::to_chars for string conversions -- disable for greater compatability with a slight performance cost.') option('default_with_color' ,type: 'boolean' ,value: true, description: 'Enable terminal colors by default -- can also be controlled by command line interface.') option('decompose_successful_assertions' ,type: 'boolean' ,value: true, description: 'Enable expression decomposition even for successful assertions -- more expensive.') option('with_all_reporters' ,type: 'boolean' ,value: true, description: 'Allow all built-in reporters to be selected from the command line -- disable for faster compilation.') diff --git a/tests/runtime_tests/string_utility.cpp b/tests/runtime_tests/string_utility.cpp index 2bb26f6d..81b9063d 100644 --- a/tests/runtime_tests/string_utility.cpp +++ b/tests/runtime_tests/string_utility.cpp @@ -344,7 +344,7 @@ TEST_CASE("append ints", "[utility]") { TEST_CASE("append floats", "[utility]") { using ae = append_test::append_expected; #if !SNITCH_CONSTEXPR_FLOAT_USE_BITCAST && SNITCH_APPEND_TO_CHARS - // answers will be different only if no bitcast AND we do have runtime to_char + // answers will be different only if no bitcast AND we do have runtime to_chars // otherwise append_constexpr will be used at runtime using aed = append_test::append_expected_diff; #endif @@ -356,14 +356,14 @@ TEST_CASE("append floats", "[utility]") { CONSTEXPR_CHECK(a(0.0f) == ae{"0.000000e+00"sv, true}); #if SNITCH_CONSTEXPR_FLOAT_USE_BITCAST - // std::bit_cast is enabled, and will match the output of std::to_char if used at runtime + // std::bit_cast is enabled, and will match the output of std::to_chars if used at runtime CONSTEXPR_CHECK(a(-0.0f) == ae{"-0.000000e+00"sv, true}); #elif SNITCH_APPEND_TO_CHARS // Without std::bit_cast (or C++23), we are unable to tell the difference between -0.0f and // +0.0f in constexpr expressions. Therefore -0.0f in constexpr gets displayed as +0.0f. CONSTEXPR_CHECK(a(-0.0f) == aed{{"0.000000e+00"sv, true}, {"-0.000000e+00"sv, true}}); #else - // No std::bit_cast, but also no std::to_char. append_constexpr will be used + // No std::bit_cast, but also no std::to_chars. append_constexpr will be used // for runtime and match the compile time results CONSTEXPR_CHECK(a(-0.0f) == ae{"0.000000e+00"sv, true}); #endif @@ -472,7 +472,7 @@ TEST_CASE("append doubles", "[utility]") { CONSTEXPR_CHECK(a(0.0) == ae{"0.000000000000000e+00"sv, true}); #if SNITCH_CONSTEXPR_FLOAT_USE_BITCAST - // std::bit_cast is enabled, and will match the output of std::to_char if used at runtime + // std::bit_cast is enabled, and will match the output of std::to_chars if used at runtime CONSTEXPR_CHECK(a(-0.0) == ae{"-0.000000000000000e+00"sv, true}); #elif SNITCH_APPEND_TO_CHARS // Without std::bit_cast (or C++23), we are unable to tell the difference between -0.0f and @@ -480,7 +480,7 @@ TEST_CASE("append doubles", "[utility]") { CONSTEXPR_CHECK( a(-0.0) == aed{{"0.000000000000000e+00"sv, true}, {"-0.000000000000000e+00"sv, true}}); #else - // No std::bit_cast, but also no std::to_char. append_constexpr will be used for + // No std::bit_cast, but also no std::to_chars. append_constexpr will be used for // runtime and match the compile time results CONSTEXPR_CHECK(a(-0.0) == ae{"0.000000000000000e+00"sv, true}); #endif