From af7d21cb58f39d5d50d87a0a774becb17c01bead Mon Sep 17 00:00:00 2001 From: Dennis Liu Date: Mon, 8 Jan 2024 19:26:31 +0800 Subject: [PATCH 1/3] iox-#2055 Add isnormal into conversion Signed-off-by: Dennis Liu --- .../test/moduletests/test_utility_convert.cpp | 28 +++++++++++++------ .../utility/include/iox/detail/convert.inl | 9 ++++-- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/iceoryx_hoofs/test/moduletests/test_utility_convert.cpp b/iceoryx_hoofs/test/moduletests/test_utility_convert.cpp index 44f286cb07..b42c5abb8e 100644 --- a/iceoryx_hoofs/test/moduletests/test_utility_convert.cpp +++ b/iceoryx_hoofs/test/moduletests/test_utility_convert.cpp @@ -45,9 +45,28 @@ class convert_test : public Test static_assert(std::is_floating_point::value, "fp_to_string requires floating point type"); std::ostringstream oss; - oss << std::scientific << std::setprecision(std::numeric_limits::max_digits10) << value; + oss << std::scientific << std::setprecision(get_digits()) << value; return oss.str(); } + + private: + // see https://github.com/eclipse-iceoryx/iceoryx/pull/2155 + template + constexpr static uint32_t get_digits() + { + if constexpr (std::is_same_v) + { + return 9; + } + else if (std::is_same_v || std::is_same_v) + { + return 19; + } + else + { + static_assert("T should be floating point"); + } + } }; @@ -664,8 +683,6 @@ TEST_F(convert_test, fromString_Float_EdgeCase_SubNormalFloat_ShouldFail) { ::testing::Test::RecordProperty("TEST_ID", "68d4f096-a93c-406b-b081-fe50e4b1a2c9"); - // strtof will trigger ERANGE if the input is a subnormal float, resulting in a nullopt return value. - // note that for MSVC, sub normal float is a valid input! auto normal_float_min_eps = std::nextafter(std::numeric_limits::min(), 0.0F); std::string source = fp_to_string(std::numeric_limits::min() - normal_float_min_eps); auto float_min_dec_eps = iox::convert::from_string(source.c_str()); @@ -673,15 +690,12 @@ TEST_F(convert_test, fromString_Float_EdgeCase_SubNormalFloat_ShouldFail) GTEST_SKIP() << "@todo iox-#2055 temporarily skipped"; #else ASSERT_THAT(float_min_dec_eps.has_value(), Eq(false)); -#endif } TEST_F(convert_test, fromString_Double_EdgeCase_InRange_Success) { ::testing::Test::RecordProperty("TEST_ID", "d5e5e5ad-92ed-4229-8128-4ee82059fbf7"); - GTEST_SKIP() << "@todo iox-#2055 Temporarily skipped due to issues in aarch64"; - std::string source = fp_to_string(std::numeric_limits::min()); auto double_min = iox::convert::from_string(source.c_str()); ASSERT_THAT(double_min.has_value(), Eq(true)); @@ -709,7 +723,6 @@ TEST_F(convert_test, fromString_Double_EdgeCase_SubNormalDouble_ShouldFailExcept GTEST_SKIP() << "@todo iox-#2055 temporarily skipped"; #else ASSERT_THAT(double_min_dec_eps.has_value(), Eq(false)); -#endif } TEST_F(convert_test, fromString_LongDouble_EdgeCase_InRange_Success) @@ -744,7 +757,6 @@ TEST_F(convert_test, fromString_LongDouble_EdgeCase_SubNormalLongDouble_ShouldFa GTEST_SKIP() << "@todo iox-#2055 temporarily skipped"; #else ASSERT_THAT(long_double_min_dec_eps.has_value(), Eq(false)); -#endif } /// NORMAL FLOATING POINT TYPE EDGE CASES END diff --git a/iceoryx_hoofs/utility/include/iox/detail/convert.inl b/iceoryx_hoofs/utility/include/iox/detail/convert.inl index b04881c22d..4c076092b1 100644 --- a/iceoryx_hoofs/utility/include/iox/detail/convert.inl +++ b/iceoryx_hoofs/utility/include/iox/detail/convert.inl @@ -363,7 +363,6 @@ inline bool convert::is_within_range(const SourceType& source_val) noexcept { return true; } - // is_arithmetic_v if constexpr (std::is_floating_point_v) { // special cases for floating point @@ -371,8 +370,13 @@ inline bool convert::is_within_range(const SourceType& source_val) noexcept { return true; } +#ifdef _MSC_VER + if (!std::isnormal(source_val) && (source_val != 0.0)) + { + return false; + } +#endif } - // out of range (upper bound) if (source_val > std::numeric_limits::max()) { @@ -381,7 +385,6 @@ inline bool convert::is_within_range(const SourceType& source_val) noexcept << std::numeric_limits::max()); return false; } - // out of range (lower bound) if (source_val < std::numeric_limits::lowest()) { From f7c6c0cef7cd0fc41cdbc218f494edafb7375899 Mon Sep 17 00:00:00 2001 From: Dennis Liu Date: Thu, 11 Jan 2024 20:59:05 +0800 Subject: [PATCH 2/3] iox-#2055 Fix tests Signed-off-by: Dennis Liu --- .../test/moduletests/test_utility_convert.cpp | 120 +++++++++++------- .../utility/include/iox/detail/convert.inl | 4 +- 2 files changed, 75 insertions(+), 49 deletions(-) diff --git a/iceoryx_hoofs/test/moduletests/test_utility_convert.cpp b/iceoryx_hoofs/test/moduletests/test_utility_convert.cpp index b42c5abb8e..846de5ea40 100644 --- a/iceoryx_hoofs/test/moduletests/test_utility_convert.cpp +++ b/iceoryx_hoofs/test/moduletests/test_utility_convert.cpp @@ -30,6 +30,35 @@ using namespace ::testing; using NumberType = iox::convert::NumberType; +class LongDouble +{ + public: + static bool Eq(long double a, long double b) + { + IOX_LOG(DEBUG, "a: " << a << ", b: " << b); + + long double min_val = std::min(std::fabs(a), std::fabs(b)); + long double epsilon = std::fabs(min_val - std::nextafter(min_val, static_cast(0))); + + IOX_LOG(DEBUG, "epsilon from min_val: " << epsilon); + IOX_LOG(DEBUG, "abs min_val: " << min_val); + + if (epsilon <= 0 || epsilon < std::numeric_limits::min()) + { + epsilon = std::numeric_limits::min(); + } + IOX_LOG(DEBUG, "epsilon: " << epsilon); + + long double abs_diff = std::fabs(a - b); + IOX_LOG(DEBUG, "fabs result: " << abs_diff); + + bool is_equal = abs_diff <= epsilon; + IOX_LOG(DEBUG, "<< a and b " << ((is_equal) ? "IS" : "IS NOT") << " considered equal! >>"); + + return is_equal; + } +}; + class convert_test : public Test { public: @@ -42,30 +71,20 @@ class convert_test : public Test template std::string fp_to_string(T value) { - static_assert(std::is_floating_point::value, "fp_to_string requires floating point type"); + static_assert(std::is_floating_point::value, "requires floating point type"); std::ostringstream oss; - oss << std::scientific << std::setprecision(get_digits()) << value; + oss << std::scientific << std::setprecision(std::numeric_limits::digits10) << value; return oss.str(); } - - private: - // see https://github.com/eclipse-iceoryx/iceoryx/pull/2155 template - constexpr static uint32_t get_digits() + std::string fp_to_string(T value, uint16_t digits) { - if constexpr (std::is_same_v) - { - return 9; - } - else if (std::is_same_v || std::is_same_v) - { - return 19; - } - else - { - static_assert("T should be floating point"); - } + static_assert(std::is_floating_point::value, "requires floating point type"); + + std::ostringstream oss; + oss << std::scientific << std::setprecision(digits) << value; + return oss.str(); } }; @@ -198,11 +217,13 @@ TEST_F(convert_test, fromString_Double_Fail) TEST_F(convert_test, fromString_LongDouble_Success) { ::testing::Test::RecordProperty("TEST_ID", "2864fbae-ef1c-48ab-97f2-745baadc4dc5"); + constexpr long double VERIFY = 121.01L; std::string source = "121.01"; - constexpr long double VERIFY = 121.01; + auto result = iox::convert::from_string(source.c_str()); ASSERT_THAT(result.has_value(), Eq(true)); - EXPECT_THAT(static_cast(result.value()), DoubleEq(static_cast(VERIFY))); + // NOLINTNEXTLINE(clang-analyzer-core.CallAndMessage) + EXPECT_THAT(LongDouble::Eq(VERIFY, result.value()), Eq(true)); } TEST_F(convert_test, fromString_LongDouble_Fail) @@ -486,7 +507,6 @@ TEST_F(convert_test, fromString_SignedLongLong_EdgeCase_InRange_Success) std::string source = "-9223372036854775808"; auto long_long_min = iox::convert::from_string(source.c_str()); ASSERT_THAT(long_long_min.has_value(), Eq(true)); - // we don't use -9223372036854775808LL here for the compiler will parse it in way we don't want EXPECT_THAT(long_long_min.value(), Eq(std::numeric_limits::min())); source = "9223372036854775807"; @@ -663,32 +683,33 @@ TEST_F(convert_test, fromString_Float_EdgeCase_InRange_Success) { ::testing::Test::RecordProperty("TEST_ID", "cf849d5d-d0ed-4447-89b8-d6b9f47287c7"); - std::string source = fp_to_string(std::numeric_limits::min()); + // the number larger than numeric_limits::digits10 that will pass all tests for all platforms + constexpr uint16_t PLATFORM_DIGIT_WORKAROUND_MIN{7}; + constexpr uint16_t PLATFORM_DIGIT_WORKAROUND_MAX{7}; + + std::string source = fp_to_string(std::numeric_limits::min(), PLATFORM_DIGIT_WORKAROUND_MIN); auto float_min = iox::convert::from_string(source.c_str()); ASSERT_THAT(float_min.has_value(), Eq(true)); EXPECT_THAT(float_min.value(), FloatEq(std::numeric_limits::min())); - source = fp_to_string(std::numeric_limits::lowest()); + source = fp_to_string(std::numeric_limits::lowest(), PLATFORM_DIGIT_WORKAROUND_MAX); auto float_lowest = iox::convert::from_string(source.c_str()); ASSERT_THAT(float_lowest.has_value(), Eq(true)); EXPECT_THAT(float_lowest.value(), FloatEq(std::numeric_limits::lowest())); - source = fp_to_string(std::numeric_limits::max()); + source = fp_to_string(std::numeric_limits::max(), PLATFORM_DIGIT_WORKAROUND_MAX); auto float_max = iox::convert::from_string(source.c_str()); ASSERT_THAT(float_max.has_value(), Eq(true)); EXPECT_THAT(float_max.value(), FloatEq(std::numeric_limits::max())); } -TEST_F(convert_test, fromString_Float_EdgeCase_SubNormalFloat_ShouldFail) +TEST_F(convert_test, fromString_Float_EdgeCase_SubNormalFloat_ShouldFailExceptMsvc) { ::testing::Test::RecordProperty("TEST_ID", "68d4f096-a93c-406b-b081-fe50e4b1a2c9"); auto normal_float_min_eps = std::nextafter(std::numeric_limits::min(), 0.0F); std::string source = fp_to_string(std::numeric_limits::min() - normal_float_min_eps); auto float_min_dec_eps = iox::convert::from_string(source.c_str()); -#ifdef _WIN32 - GTEST_SKIP() << "@todo iox-#2055 temporarily skipped"; -#else ASSERT_THAT(float_min_dec_eps.has_value(), Eq(false)); } @@ -696,32 +717,33 @@ TEST_F(convert_test, fromString_Double_EdgeCase_InRange_Success) { ::testing::Test::RecordProperty("TEST_ID", "d5e5e5ad-92ed-4229-8128-4ee82059fbf7"); - std::string source = fp_to_string(std::numeric_limits::min()); + // the number larger than numeric_limits::digits10 that will pass all tests for all platforms + constexpr uint16_t PLATFORM_DIGIT_WORKAROUND_MIN{19}; + constexpr uint16_t PLATFORM_DIGIT_WORKAROUND_MAX{18}; + + std::string source = fp_to_string(std::numeric_limits::min(), PLATFORM_DIGIT_WORKAROUND_MIN); auto double_min = iox::convert::from_string(source.c_str()); ASSERT_THAT(double_min.has_value(), Eq(true)); EXPECT_THAT(double_min.value(), DoubleEq(std::numeric_limits::min())); - source = fp_to_string(std::numeric_limits::lowest()); + source = fp_to_string(std::numeric_limits::lowest(), PLATFORM_DIGIT_WORKAROUND_MAX); auto double_lowest = iox::convert::from_string(source.c_str()); ASSERT_THAT(double_lowest.has_value(), Eq(true)); EXPECT_THAT(double_lowest.value(), DoubleEq(std::numeric_limits::lowest())); - source = fp_to_string(std::numeric_limits::max()); + source = fp_to_string(std::numeric_limits::max(), PLATFORM_DIGIT_WORKAROUND_MAX); auto double_max = iox::convert::from_string(source.c_str()); ASSERT_THAT(double_max.has_value(), Eq(true)); EXPECT_THAT(double_max.value(), DoubleEq(std::numeric_limits::max())); } -TEST_F(convert_test, fromString_Double_EdgeCase_SubNormalDouble_ShouldFailExcept) +TEST_F(convert_test, fromString_Double_EdgeCase_SubNormalDouble_ShouldFailExceptMsvc) { ::testing::Test::RecordProperty("TEST_ID", "af7ca2e6-ba7e-41f7-a321-5f68617d3566"); auto normal_double_min_eps = std::nextafter(std::numeric_limits::min(), 0.0); std::string source = fp_to_string(std::numeric_limits::min() - normal_double_min_eps); auto double_min_dec_eps = iox::convert::from_string(source.c_str()); -#ifdef _WIN32 - GTEST_SKIP() << "@todo iox-#2055 temporarily skipped"; -#else ASSERT_THAT(double_min_dec_eps.has_value(), Eq(false)); } @@ -729,33 +751,36 @@ TEST_F(convert_test, fromString_LongDouble_EdgeCase_InRange_Success) { ::testing::Test::RecordProperty("TEST_ID", "cab1c90b-1de0-4654-bbea-4bb4e55e4fc3"); - std::string source = fp_to_string(std::numeric_limits::min()); + // the number larger than numeric_limits::digits10 that will pass all tests for all platforms + constexpr uint16_t PLATFORM_DIGIT_WORKAROUND_MIN{36}; + constexpr uint16_t PLATFORM_DIGIT_WORKAROUND_MAX{34}; + + std::string source = fp_to_string(std::numeric_limits::min(), PLATFORM_DIGIT_WORKAROUND_MIN); auto long_double_min = iox::convert::from_string(source.c_str()); ASSERT_THAT(long_double_min.has_value(), Eq(true)); - // There's no LongDoubleEq - EXPECT_THAT(long_double_min.value(), Eq(std::numeric_limits::min())); + // NOLINTNEXTLINE(clang-analyzer-core.CallAndMessage) + EXPECT_THAT(LongDouble::Eq(long_double_min.value(), std::numeric_limits::min()), Eq(true)); - source = fp_to_string(std::numeric_limits::lowest()); + source = fp_to_string(std::numeric_limits::lowest(), PLATFORM_DIGIT_WORKAROUND_MAX); auto long_double_lowest = iox::convert::from_string(source.c_str()); ASSERT_THAT(long_double_lowest.has_value(), Eq(true)); - EXPECT_THAT(long_double_lowest.value(), Eq(std::numeric_limits::lowest())); + // NOLINTNEXTLINE(clang-analyzer-core.CallAndMessage) + EXPECT_THAT(LongDouble::Eq(long_double_lowest.value(), std::numeric_limits::lowest()), Eq(true)); - source = fp_to_string(std::numeric_limits::max()); + source = fp_to_string(std::numeric_limits::max(), PLATFORM_DIGIT_WORKAROUND_MAX); auto long_double_max = iox::convert::from_string(source.c_str()); ASSERT_THAT(long_double_max.has_value(), Eq(true)); - EXPECT_THAT(long_double_max.value(), Eq(std::numeric_limits::max())); + // NOLINTNEXTLINE(clang-analyzer-core.CallAndMessage) + EXPECT_THAT(LongDouble::Eq(long_double_max.value(), std::numeric_limits::max()), Eq(true)); } -TEST_F(convert_test, fromString_LongDouble_EdgeCase_SubNormalLongDouble_ShouldFail) +TEST_F(convert_test, fromString_LongDouble_EdgeCase_SubNormalLongDouble_ShouldFailExceptMsvc) { ::testing::Test::RecordProperty("TEST_ID", "fb96e526-8fb6-4af9-87f0-dfd4193237a5"); auto normal_long_double_min_eps = std::nextafter(std::numeric_limits::min(), 0.0L); std::string source = fp_to_string(std::numeric_limits::min() - normal_long_double_min_eps); auto long_double_min_dec_eps = iox::convert::from_string(source.c_str()); -#ifdef _WIN32 - GTEST_SKIP() << "@todo iox-#2055 temporarily skipped"; -#else ASSERT_THAT(long_double_min_dec_eps.has_value(), Eq(false)); } @@ -888,7 +913,8 @@ TEST_F(convert_test, fromString_LongDouble_EdgeCase_ZeroDecimalNotation_Success) { auto decimal_ret = iox::convert::from_string(v.c_str()); ASSERT_THAT(decimal_ret.has_value(), Eq(true)); - ASSERT_THAT(decimal_ret.value(), Eq(0.0L)); + // NOLINTNEXTLINE(clang-analyzer-core.CallAndMessage) + ASSERT_THAT(LongDouble::Eq(decimal_ret.value(), 0.0L), Eq(true)); } } diff --git a/iceoryx_hoofs/utility/include/iox/detail/convert.inl b/iceoryx_hoofs/utility/include/iox/detail/convert.inl index 4c076092b1..080337abfc 100644 --- a/iceoryx_hoofs/utility/include/iox/detail/convert.inl +++ b/iceoryx_hoofs/utility/include/iox/detail/convert.inl @@ -366,16 +366,16 @@ inline bool convert::is_within_range(const SourceType& source_val) noexcept if constexpr (std::is_floating_point_v) { // special cases for floating point + // can be nan or inf if (std::isnan(source_val) || std::isinf(source_val)) { return true; } -#ifdef _MSC_VER + // should be normal or zero if (!std::isnormal(source_val) && (source_val != 0.0)) { return false; } -#endif } // out of range (upper bound) if (source_val > std::numeric_limits::max()) From e19b7f21b97b76aa16fa77c8c329a261a300e850 Mon Sep 17 00:00:00 2001 From: Dennis Liu Date: Mon, 15 Jan 2024 21:52:22 +0800 Subject: [PATCH 3/3] iox-#2055 Correct the name of tests Signed-off-by: Dennis Liu --- .../test/moduletests/test_utility_convert.cpp | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/iceoryx_hoofs/test/moduletests/test_utility_convert.cpp b/iceoryx_hoofs/test/moduletests/test_utility_convert.cpp index 846de5ea40..b13d467629 100644 --- a/iceoryx_hoofs/test/moduletests/test_utility_convert.cpp +++ b/iceoryx_hoofs/test/moduletests/test_utility_convert.cpp @@ -69,16 +69,7 @@ class convert_test : public Test { } template - std::string fp_to_string(T value) - { - static_assert(std::is_floating_point::value, "requires floating point type"); - - std::ostringstream oss; - oss << std::scientific << std::setprecision(std::numeric_limits::digits10) << value; - return oss.str(); - } - template - std::string fp_to_string(T value, uint16_t digits) + std::string fp_to_string(T value, uint16_t digits = std::numeric_limits::digits10) { static_assert(std::is_floating_point::value, "requires floating point type"); @@ -703,7 +694,7 @@ TEST_F(convert_test, fromString_Float_EdgeCase_InRange_Success) EXPECT_THAT(float_max.value(), FloatEq(std::numeric_limits::max())); } -TEST_F(convert_test, fromString_Float_EdgeCase_SubNormalFloat_ShouldFailExceptMsvc) +TEST_F(convert_test, fromString_Float_EdgeCase_SubNormalFloat_ShouldFail) { ::testing::Test::RecordProperty("TEST_ID", "68d4f096-a93c-406b-b081-fe50e4b1a2c9"); @@ -737,7 +728,7 @@ TEST_F(convert_test, fromString_Double_EdgeCase_InRange_Success) EXPECT_THAT(double_max.value(), DoubleEq(std::numeric_limits::max())); } -TEST_F(convert_test, fromString_Double_EdgeCase_SubNormalDouble_ShouldFailExceptMsvc) +TEST_F(convert_test, fromString_Double_EdgeCase_SubNormalDouble_ShouldFail) { ::testing::Test::RecordProperty("TEST_ID", "af7ca2e6-ba7e-41f7-a321-5f68617d3566"); @@ -774,7 +765,7 @@ TEST_F(convert_test, fromString_LongDouble_EdgeCase_InRange_Success) EXPECT_THAT(LongDouble::Eq(long_double_max.value(), std::numeric_limits::max()), Eq(true)); } -TEST_F(convert_test, fromString_LongDouble_EdgeCase_SubNormalLongDouble_ShouldFailExceptMsvc) +TEST_F(convert_test, fromString_LongDouble_EdgeCase_SubNormalLongDouble_ShouldFail) { ::testing::Test::RecordProperty("TEST_ID", "fb96e526-8fb6-4af9-87f0-dfd4193237a5");