From c38a5caa2ebf0738e468f419c4d3ed857001cfb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Sat, 5 Oct 2019 13:23:14 +0200 Subject: [PATCH] Allow full range of target ULP values for the ULPMatcher Previously it was limited to roughly 2 billion ULPs, rather than the roughly 2^64 possible ones. --- include/internal/catch_matchers_floating.cpp | 49 +++++++------------ include/internal/catch_matchers_floating.h | 8 +-- .../Baselines/compact.sw.approved.txt | 3 +- .../Baselines/console.std.approved.txt | 2 +- .../Baselines/console.sw.approved.txt | 7 +-- .../SelfTest/Baselines/junit.sw.approved.txt | 2 +- .../SelfTest/Baselines/xml.sw.approved.txt | 18 ++----- .../SelfTest/UsageTests/Matchers.tests.cpp | 3 +- 8 files changed, 34 insertions(+), 58 deletions(-) diff --git a/include/internal/catch_matchers_floating.cpp b/include/internal/catch_matchers_floating.cpp index 69a451c0..8de8c7aa 100644 --- a/include/internal/catch_matchers_floating.cpp +++ b/include/internal/catch_matchers_floating.cpp @@ -34,34 +34,22 @@ enum class FloatingPointKind : uint8_t { namespace { -template -struct Converter; - -template <> -struct Converter { +int32_t convert(float f) { static_assert(sizeof(float) == sizeof(int32_t), "Important ULP matcher assumption violated"); - Converter(float f) { - std::memcpy(&i, &f, sizeof(f)); - } int32_t i; -}; + std::memcpy(&i, &f, sizeof(f)); + return i; +} -template <> -struct Converter { +int64_t convert(double d) { static_assert(sizeof(double) == sizeof(int64_t), "Important ULP matcher assumption violated"); - Converter(double d) { - std::memcpy(&i, &d, sizeof(d)); - } int64_t i; -}; - -template -auto convert(T t) -> Converter { - return Converter(t); + std::memcpy(&i, &d, sizeof(d)); + return i; } template -bool almostEqualUlps(FP lhs, FP rhs, int maxUlpDiff) { +bool almostEqualUlps(FP lhs, FP rhs, uint64_t maxUlpDiff) { // Comparison with NaN should always be false. // This way we can rule it out before getting into the ugly details if (Catch::isnan(lhs) || Catch::isnan(rhs)) { @@ -71,13 +59,13 @@ bool almostEqualUlps(FP lhs, FP rhs, int maxUlpDiff) { auto lc = convert(lhs); auto rc = convert(rhs); - if ((lc.i < 0) != (rc.i < 0)) { + if ((lc < 0) != (rc < 0)) { // Potentially we can have +0 and -0 return lhs == rhs; } - auto ulpDiff = std::abs(lc.i - rc.i); - return ulpDiff <= maxUlpDiff; + auto ulpDiff = std::abs(lc - rc); + return static_cast(ulpDiff) <= maxUlpDiff; } #if defined(CATCH_CONFIG_GLOBAL_NEXTAFTER) @@ -99,8 +87,8 @@ namespace Catch { #endif template -FP step(FP start, FP direction, int steps) { - for (int i = 0; i < steps; ++i) { +FP step(FP start, FP direction, uint64_t steps) { + for (uint64_t i = 0; i < steps; ++i) { #if defined(CATCH_CONFIG_GLOBAL_NEXTAFTER) start = Catch::nextafter(start, direction); #else @@ -133,10 +121,11 @@ namespace Floating { } - WithinUlpsMatcher::WithinUlpsMatcher(double target, int ulps, FloatingPointKind baseType) + WithinUlpsMatcher::WithinUlpsMatcher(double target, uint64_t ulps, FloatingPointKind baseType) :m_target{ target }, m_ulps{ ulps }, m_type{ baseType } { - CATCH_ENFORCE(ulps >= 0, "Invalid ULP setting: " << ulps << '.' - << " ULPs have to be non-negative."); + CATCH_ENFORCE(m_type == FloatingPointKind::Double + || m_ulps < std::numeric_limits::max(), + "Provided ULP is impossibly large for a float comparison."); } #if defined(__clang__) @@ -190,11 +179,11 @@ namespace Floating { -Floating::WithinUlpsMatcher WithinULP(double target, int maxUlpDiff) { +Floating::WithinUlpsMatcher WithinULP(double target, uint64_t maxUlpDiff) { return Floating::WithinUlpsMatcher(target, maxUlpDiff, Floating::FloatingPointKind::Double); } -Floating::WithinUlpsMatcher WithinULP(float target, int maxUlpDiff) { +Floating::WithinUlpsMatcher WithinULP(float target, uint64_t maxUlpDiff) { return Floating::WithinUlpsMatcher(target, maxUlpDiff, Floating::FloatingPointKind::Float); } diff --git a/include/internal/catch_matchers_floating.h b/include/internal/catch_matchers_floating.h index 3c1ceb07..7db9cc30 100644 --- a/include/internal/catch_matchers_floating.h +++ b/include/internal/catch_matchers_floating.h @@ -26,12 +26,12 @@ namespace Matchers { }; struct WithinUlpsMatcher : MatcherBase { - WithinUlpsMatcher(double target, int ulps, FloatingPointKind baseType); + WithinUlpsMatcher(double target, uint64_t ulps, FloatingPointKind baseType); bool match(double const& matchee) const override; std::string describe() const override; private: double m_target; - int m_ulps; + uint64_t m_ulps; FloatingPointKind m_type; }; @@ -40,8 +40,8 @@ namespace Matchers { // The following functions create the actual matcher objects. // This allows the types to be inferred - Floating::WithinUlpsMatcher WithinULP(double target, int maxUlpDiff); - Floating::WithinUlpsMatcher WithinULP(float target, int maxUlpDiff); + Floating::WithinUlpsMatcher WithinULP(double target, uint64_t maxUlpDiff); + Floating::WithinUlpsMatcher WithinULP(float target, uint64_t maxUlpDiff); Floating::WithinAbsMatcher WithinAbs(double target, double margin); } // namespace Matchers diff --git a/projects/SelfTest/Baselines/compact.sw.approved.txt b/projects/SelfTest/Baselines/compact.sw.approved.txt index 7c93c160..e7919287 100644 --- a/projects/SelfTest/Baselines/compact.sw.approved.txt +++ b/projects/SelfTest/Baselines/compact.sw.approved.txt @@ -408,7 +408,6 @@ Matchers.tests.cpp:: passed: 1., WithinAbs(2., 0.5) || WithinULP(1. Matchers.tests.cpp:: passed: WithinAbs(1., 0.) Matchers.tests.cpp:: passed: WithinAbs(1., -1.), std::domain_error Matchers.tests.cpp:: passed: WithinULP(1., 0) -Matchers.tests.cpp:: passed: WithinULP(1., -1), std::domain_error Matchers.tests.cpp:: passed: 1.f, WithinAbs(1.f, 0) for: 1.0f is within 0.0 of 1.0 Matchers.tests.cpp:: passed: 0.f, WithinAbs(1.f, 1) for: 0.0f is within 1.0 of 1.0 Matchers.tests.cpp:: passed: 0.f, !WithinAbs(1.f, 0.99f) for: 0.0f not is within 0.9900000095 of 1.0 @@ -429,7 +428,7 @@ Matchers.tests.cpp:: passed: 1.f, WithinAbs(2.f, 0.5) || WithinULP( Matchers.tests.cpp:: passed: WithinAbs(1.f, 0.f) Matchers.tests.cpp:: passed: WithinAbs(1.f, -1.f), std::domain_error Matchers.tests.cpp:: passed: WithinULP(1.f, 0) -Matchers.tests.cpp:: passed: WithinULP(1.f, -1), std::domain_error +Matchers.tests.cpp:: passed: WithinULP(1.f, static_cast(-1)), std::domain_error Generators.tests.cpp:: passed: i % 2 == 0 for: 0 == 0 Generators.tests.cpp:: passed: i % 2 == 0 for: 0 == 0 Generators.tests.cpp:: passed: i % 2 == 0 for: 0 == 0 diff --git a/projects/SelfTest/Baselines/console.std.approved.txt b/projects/SelfTest/Baselines/console.std.approved.txt index a729420d..7eaae82c 100644 --- a/projects/SelfTest/Baselines/console.std.approved.txt +++ b/projects/SelfTest/Baselines/console.std.approved.txt @@ -1381,5 +1381,5 @@ due to unexpected exception with message: =============================================================================== test cases: 300 | 226 passed | 70 failed | 4 failed as expected -assertions: 1565 | 1413 passed | 131 failed | 21 failed as expected +assertions: 1564 | 1412 passed | 131 failed | 21 failed as expected diff --git a/projects/SelfTest/Baselines/console.sw.approved.txt b/projects/SelfTest/Baselines/console.sw.approved.txt index 83ca294e..4415fe06 100644 --- a/projects/SelfTest/Baselines/console.sw.approved.txt +++ b/projects/SelfTest/Baselines/console.sw.approved.txt @@ -3020,9 +3020,6 @@ Matchers.tests.cpp:: PASSED: Matchers.tests.cpp:: PASSED: REQUIRE_NOTHROW( WithinULP(1., 0) ) -Matchers.tests.cpp:: PASSED: - REQUIRE_THROWS_AS( WithinULP(1., -1), std::domain_error ) - ------------------------------------------------------------------------------- Floating point matchers: float Margin @@ -3149,7 +3146,7 @@ Matchers.tests.cpp:: PASSED: REQUIRE_NOTHROW( WithinULP(1.f, 0) ) Matchers.tests.cpp:: PASSED: - REQUIRE_THROWS_AS( WithinULP(1.f, -1), std::domain_error ) + REQUIRE_THROWS_AS( WithinULP(1.f, static_cast(-1)), std::domain_error ) ------------------------------------------------------------------------------- Generators -- adapters @@ -12503,5 +12500,5 @@ Misc.tests.cpp:: PASSED: =============================================================================== test cases: 300 | 210 passed | 86 failed | 4 failed as expected -assertions: 1582 | 1413 passed | 148 failed | 21 failed as expected +assertions: 1581 | 1412 passed | 148 failed | 21 failed as expected diff --git a/projects/SelfTest/Baselines/junit.sw.approved.txt b/projects/SelfTest/Baselines/junit.sw.approved.txt index ae12fb07..b16b6638 100644 --- a/projects/SelfTest/Baselines/junit.sw.approved.txt +++ b/projects/SelfTest/Baselines/junit.sw.approved.txt @@ -1,7 +1,7 @@ - + diff --git a/projects/SelfTest/Baselines/xml.sw.approved.txt b/projects/SelfTest/Baselines/xml.sw.approved.txt index 06d15a4e..65e5f4bd 100644 --- a/projects/SelfTest/Baselines/xml.sw.approved.txt +++ b/projects/SelfTest/Baselines/xml.sw.approved.txt @@ -3630,15 +3630,7 @@ Nor would this WithinULP(1., 0) - - - WithinULP(1., -1), std::domain_error - - - WithinULP(1., -1), std::domain_error - - - + @@ -3815,10 +3807,10 @@ Nor would this - WithinULP(1.f, -1), std::domain_error + WithinULP(1.f, static_cast<uint64_t>(-1)), std::domain_error - WithinULP(1.f, -1), std::domain_error + WithinULP(1.f, static_cast<uint64_t>(-1)), std::domain_error @@ -14880,7 +14872,7 @@ loose text artifact - + - + diff --git a/projects/SelfTest/UsageTests/Matchers.tests.cpp b/projects/SelfTest/UsageTests/Matchers.tests.cpp index 6f9c3cb4..8e9148b3 100644 --- a/projects/SelfTest/UsageTests/Matchers.tests.cpp +++ b/projects/SelfTest/UsageTests/Matchers.tests.cpp @@ -372,7 +372,7 @@ namespace { namespace MatchersTests { REQUIRE_THROWS_AS(WithinAbs(1.f, -1.f), std::domain_error); REQUIRE_NOTHROW(WithinULP(1.f, 0)); - REQUIRE_THROWS_AS(WithinULP(1.f, -1), std::domain_error); + REQUIRE_THROWS_AS(WithinULP(1.f, static_cast(-1)), std::domain_error); } } @@ -408,7 +408,6 @@ namespace { namespace MatchersTests { REQUIRE_THROWS_AS(WithinAbs(1., -1.), std::domain_error); REQUIRE_NOTHROW(WithinULP(1., 0)); - REQUIRE_THROWS_AS(WithinULP(1., -1), std::domain_error); } }