From 04e752ba881d49f2cefafbabcae4ac85581465a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20H=C3=B8jlund=20Larsen?= Date: Wed, 10 Jan 2024 12:47:04 +0100 Subject: [PATCH 01/10] now with asserts and tests thereof --- src/abacus/metric.hpp | 13 +++++++++++++ test/src/test_metric.cpp | 5 +++++ 2 files changed, 18 insertions(+) diff --git a/src/abacus/metric.hpp b/src/abacus/metric.hpp index 4081547..87e6ff7 100644 --- a/src/abacus/metric.hpp +++ b/src/abacus/metric.hpp @@ -7,6 +7,7 @@ #include #include +#include #include "type.hpp" #include "version.hpp" @@ -220,6 +221,10 @@ class metric auto operator=(double value) -> metric& { assert(is_initialized()); + assert(!std::isnan(value) && !std::isinf(value) && "Cannot assign a " + "NaN or Inf/-Inf " + "value to a " + "float metric"); *m_memory = value; return *this; } @@ -230,6 +235,10 @@ class metric auto operator+=(double value) -> metric& { assert(is_initialized()); + assert(!std::isnan(value) && !std::isinf(value) && "Cannot assign a " + "NaN or Inf/-Inf " + "value to a " + "float metric"); *m_memory += value; return *this; } @@ -239,6 +248,10 @@ class metric /// @return The result of the arithmetic auto operator-=(double value) -> metric& { + assert(!std::isnan(value) && !std::isinf(value) && "Cannot assign a " + "NaN or Inf/-Inf " + "value to a " + "float metric"); assert(is_initialized()); *m_memory -= value; return *this; diff --git a/test/src/test_metric.cpp b/test/src/test_metric.cpp index 36ed52d..8be5e96 100644 --- a/test/src/test_metric.cpp +++ b/test/src/test_metric.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -23,6 +24,10 @@ TEST(test_metric, constructor) double double_count = 1123.12; abacus::metric double_metric(&double_count); EXPECT_TRUE(double_metric.is_initialized()); + EXPECT_DEATH(double_metric = std::nan("0"), ""); + EXPECT_DEATH(double_metric = std::numeric_limits::infinity(), ""); + EXPECT_DEATH(double_metric = -std::numeric_limits::infinity(), ""); + bool bool_count = true; abacus::metric bool_metric(&bool_count); From 4151f86ff53a55bb8c67559b9f719293eb8366a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20H=C3=B8jlund=20Larsen?= Date: Wed, 10 Jan 2024 12:47:59 +0100 Subject: [PATCH 02/10] use the reprensentation we see 'in the wild' --- test/src/test_metric.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/src/test_metric.cpp b/test/src/test_metric.cpp index 8be5e96..b170601 100644 --- a/test/src/test_metric.cpp +++ b/test/src/test_metric.cpp @@ -24,9 +24,9 @@ TEST(test_metric, constructor) double double_count = 1123.12; abacus::metric double_metric(&double_count); EXPECT_TRUE(double_metric.is_initialized()); - EXPECT_DEATH(double_metric = std::nan("0"), ""); - EXPECT_DEATH(double_metric = std::numeric_limits::infinity(), ""); - EXPECT_DEATH(double_metric = -std::numeric_limits::infinity(), ""); + EXPECT_DEATH(double_metric = 0.0 / 0.0, ""); + EXPECT_DEATH(double_metric = 1 / 0.0, ""); + EXPECT_DEATH(double_metric = 1 / -0.0, ""); bool bool_count = true; From 9681196113519a5431baeec2560a02474f6910e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20H=C3=B8jlund=20Larsen?= Date: Wed, 10 Jan 2024 12:54:35 +0100 Subject: [PATCH 03/10] docs and float specific test --- src/abacus/metric.hpp | 6 +++++- test/src/test_metric.cpp | 42 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/abacus/metric.hpp b/src/abacus/metric.hpp index 87e6ff7..d27593f 100644 --- a/src/abacus/metric.hpp +++ b/src/abacus/metric.hpp @@ -221,6 +221,7 @@ class metric auto operator=(double value) -> metric& { assert(is_initialized()); + // We don't allow assignment to NaN or Inf/-Inf assert(!std::isnan(value) && !std::isinf(value) && "Cannot assign a " "NaN or Inf/-Inf " "value to a " @@ -235,6 +236,7 @@ class metric auto operator+=(double value) -> metric& { assert(is_initialized()); + // We don't allow assignment to NaN or Inf/-Inf assert(!std::isnan(value) && !std::isinf(value) && "Cannot assign a " "NaN or Inf/-Inf " "value to a " @@ -248,11 +250,13 @@ class metric /// @return The result of the arithmetic auto operator-=(double value) -> metric& { + assert(is_initialized()); + // We don't allow assignment to NaN or Inf/-Inf assert(!std::isnan(value) && !std::isinf(value) && "Cannot assign a " "NaN or Inf/-Inf " "value to a " "float metric"); - assert(is_initialized()); + *m_memory -= value; return *this; } diff --git a/test/src/test_metric.cpp b/test/src/test_metric.cpp index b170601..12c4c06 100644 --- a/test/src/test_metric.cpp +++ b/test/src/test_metric.cpp @@ -24,12 +24,46 @@ TEST(test_metric, constructor) double double_count = 1123.12; abacus::metric double_metric(&double_count); EXPECT_TRUE(double_metric.is_initialized()); - EXPECT_DEATH(double_metric = 0.0 / 0.0, ""); - EXPECT_DEATH(double_metric = 1 / 0.0, ""); - EXPECT_DEATH(double_metric = 1 / -0.0, ""); - bool bool_count = true; abacus::metric bool_metric(&bool_count); EXPECT_TRUE(bool_metric.is_initialized()); } + +TEST(test_metric, float_assignment){ + double double_count = 1123.12; + abacus::metric double_metric(&double_count); + EXPECT_TRUE(double_metric.is_initialized()); + EXPECT_DOUBLE_EQ(double_metric.value(), 1123.12); + + // Assignment + // Check that assignment to NaN is not allowed + EXPECT_DEATH(double_metric = 0.0 / 0.0, ""); + // Check that that assignment to -NaN is not allowed + EXPECT_DEATH(double_metric = -0.0 / 0.0, ""); + // Check that that assignment to +Inf is not allowed + EXPECT_DEATH(double_metric = 1 / 0.0, ""); + // Check that that assignment to -Inf is not allowed + EXPECT_DEATH(double_metric = 1 / -0.0, ""); + + // Add and Assign + // Check that assignment to NaN is not allowed + EXPECT_DEATH(double_metric += 0.0 / 0.0, ""); + // Check that that assignment to -NaN is not allowed + EXPECT_DEATH(double_metric += -0.0 / 0.0, ""); + // Check that that assignment to +Inf is not allowed + EXPECT_DEATH(double_metric += 1 / 0.0, ""); + // Check that that assignment to -Inf is not allowed + EXPECT_DEATH(double_metric += 1 / -0.0, ""); + + // Subtract and Assign + // Check that assignment to NaN is not allowed + EXPECT_DEATH(double_metric -= 0.0 / 0.0, ""); + // Check that that assignment to -NaN is not allowed + EXPECT_DEATH(double_metric -= -0.0 / 0.0, ""); + // Check that that assignment to +Inf is not allowed + EXPECT_DEATH(double_metric -= 1 / 0.0, ""); + // Check that that assignment to -Inf is not allowed + EXPECT_DEATH(double_metric -= 1 / -0.0, ""); + +} \ No newline at end of file From 408afa0af70f8ef91c89d552fcd6167da37a9b34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20H=C3=B8jlund=20Larsen?= Date: Wed, 10 Jan 2024 13:07:55 +0100 Subject: [PATCH 04/10] newline + update NEWS --- NEWS.rst | 2 +- test/src/test_metric.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/NEWS.rst b/NEWS.rst index 7cb7cc2..3bfccc5 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -6,7 +6,7 @@ every change, see the Git log. Latest ------ -* tbd +* Patch: Fix a bug where one could assign Nan, Inf or -Inf to a float64 metric. 6.0.0 ----- diff --git a/test/src/test_metric.cpp b/test/src/test_metric.cpp index 12c4c06..4fb7ef0 100644 --- a/test/src/test_metric.cpp +++ b/test/src/test_metric.cpp @@ -3,9 +3,9 @@ // // Distributed under the "BSD License". See the accompanying LICENSE.rst file. +#include #include #include -#include #include #include @@ -30,7 +30,8 @@ TEST(test_metric, constructor) EXPECT_TRUE(bool_metric.is_initialized()); } -TEST(test_metric, float_assignment){ +TEST(test_metric, float_assignment) +{ double double_count = 1123.12; abacus::metric double_metric(&double_count); EXPECT_TRUE(double_metric.is_initialized()); @@ -65,5 +66,4 @@ TEST(test_metric, float_assignment){ EXPECT_DEATH(double_metric -= 1 / 0.0, ""); // Check that that assignment to -Inf is not allowed EXPECT_DEATH(double_metric -= 1 / -0.0, ""); - -} \ No newline at end of file +} From 64ce8bbe732e41a026f060046f9d041d639a4500 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20H=C3=B8jlund=20Larsen?= Date: Wed, 10 Jan 2024 13:43:25 +0100 Subject: [PATCH 05/10] clang-formatted --- src/abacus/metric.hpp | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/abacus/metric.hpp b/src/abacus/metric.hpp index d27593f..5ca3245 100644 --- a/src/abacus/metric.hpp +++ b/src/abacus/metric.hpp @@ -6,8 +6,8 @@ #pragma once #include -#include #include +#include #include "type.hpp" #include "version.hpp" @@ -222,10 +222,11 @@ class metric { assert(is_initialized()); // We don't allow assignment to NaN or Inf/-Inf - assert(!std::isnan(value) && !std::isinf(value) && "Cannot assign a " - "NaN or Inf/-Inf " - "value to a " - "float metric"); + assert(!std::isnan(value) && !std::isinf(value) && + "Cannot assign a " + "NaN or Inf/-Inf " + "value to a " + "float metric"); *m_memory = value; return *this; } @@ -237,10 +238,11 @@ class metric { assert(is_initialized()); // We don't allow assignment to NaN or Inf/-Inf - assert(!std::isnan(value) && !std::isinf(value) && "Cannot assign a " - "NaN or Inf/-Inf " - "value to a " - "float metric"); + assert(!std::isnan(value) && !std::isinf(value) && + "Cannot assign a " + "NaN or Inf/-Inf " + "value to a " + "float metric"); *m_memory += value; return *this; } @@ -252,10 +254,11 @@ class metric { assert(is_initialized()); // We don't allow assignment to NaN or Inf/-Inf - assert(!std::isnan(value) && !std::isinf(value) && "Cannot assign a " - "NaN or Inf/-Inf " - "value to a " - "float metric"); + assert(!std::isnan(value) && !std::isinf(value) && + "Cannot assign a " + "NaN or Inf/-Inf " + "value to a " + "float metric"); *m_memory -= value; return *this; From 53bd1e5596295cee764574c2d1f6c4668d03c2eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20H=C3=B8jlund=20Larsen?= Date: Wed, 10 Jan 2024 13:54:19 +0100 Subject: [PATCH 06/10] need to stop the windows compiler from assuming if i can divide by 0 or not --- test/src/test_metric.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/src/test_metric.cpp b/test/src/test_metric.cpp index 4fb7ef0..772b246 100644 --- a/test/src/test_metric.cpp +++ b/test/src/test_metric.cpp @@ -2,6 +2,7 @@ // All Rights Reserved // // Distributed under the "BSD License". See the accompanying LICENSE.rst file. +#pragma STDC FENV_ACCESS ON #include #include From 16879a55b55da0a6bd2eeee03090657209d45127 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20H=C3=B8jlund=20Larsen?= Date: Wed, 10 Jan 2024 14:00:24 +0100 Subject: [PATCH 07/10] trying out the pragma --- test/src/test_metric.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/test_metric.cpp b/test/src/test_metric.cpp index 772b246..f65fbac 100644 --- a/test/src/test_metric.cpp +++ b/test/src/test_metric.cpp @@ -2,7 +2,7 @@ // All Rights Reserved // // Distributed under the "BSD License". See the accompanying LICENSE.rst file. -#pragma STDC FENV_ACCESS ON +#pragma fenv_access on #include #include From 1be90d2c110abe55e9dfd3c512320647b903edba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20H=C3=B8jlund=20Larsen?= Date: Wed, 10 Jan 2024 14:07:09 +0100 Subject: [PATCH 08/10] Parentheses --- test/src/test_metric.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/test_metric.cpp b/test/src/test_metric.cpp index f65fbac..8b4b9e6 100644 --- a/test/src/test_metric.cpp +++ b/test/src/test_metric.cpp @@ -2,7 +2,7 @@ // All Rights Reserved // // Distributed under the "BSD License". See the accompanying LICENSE.rst file. -#pragma fenv_access on +#pragma fenv_access(on) #include #include From 8bfbe2115947f71163fc39300638f156f2d4d571 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20H=C3=B8jlund=20Larsen?= Date: Wed, 10 Jan 2024 14:16:25 +0100 Subject: [PATCH 09/10] docs --- test/src/test_metric.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/src/test_metric.cpp b/test/src/test_metric.cpp index 8b4b9e6..f01eed6 100644 --- a/test/src/test_metric.cpp +++ b/test/src/test_metric.cpp @@ -2,6 +2,9 @@ // All Rights Reserved // // Distributed under the "BSD License". See the accompanying LICENSE.rst file. + +// Pragma needed to be able to compile code, that divides by literal zero with +// MSVC #pragma fenv_access(on) #include From 298d9ddef1c4ed398a37c2955f96b1613e61d4b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20H=C3=B8jlund=20Larsen?= Date: Wed, 10 Jan 2024 14:18:03 +0100 Subject: [PATCH 10/10] split the assert to be more explicit about which case was triggered --- src/abacus/metric.hpp | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/src/abacus/metric.hpp b/src/abacus/metric.hpp index 5ca3245..90a7494 100644 --- a/src/abacus/metric.hpp +++ b/src/abacus/metric.hpp @@ -222,11 +222,14 @@ class metric { assert(is_initialized()); // We don't allow assignment to NaN or Inf/-Inf - assert(!std::isnan(value) && !std::isinf(value) && - "Cannot assign a " - "NaN or Inf/-Inf " - "value to a " - "float metric"); + assert(!std::isnan(value) && "Cannot assign a " + "NaN " + "value to a " + "float metric"); + assert(!std::isinf(value) && "Cannot assign an " + "Inf/-Inf " + "value to a " + "float metric"); *m_memory = value; return *this; } @@ -238,11 +241,14 @@ class metric { assert(is_initialized()); // We don't allow assignment to NaN or Inf/-Inf - assert(!std::isnan(value) && !std::isinf(value) && - "Cannot assign a " - "NaN or Inf/-Inf " - "value to a " - "float metric"); + assert(!std::isnan(value) && "Cannot assign a " + "NaN " + "value to a " + "float metric"); + assert(!std::isinf(value) && "Cannot assign an " + "Inf/-Inf " + "value to a " + "float metric"); *m_memory += value; return *this; } @@ -254,11 +260,14 @@ class metric { assert(is_initialized()); // We don't allow assignment to NaN or Inf/-Inf - assert(!std::isnan(value) && !std::isinf(value) && - "Cannot assign a " - "NaN or Inf/-Inf " - "value to a " - "float metric"); + assert(!std::isnan(value) && "Cannot assign a " + "NaN " + "value to a " + "float metric"); + assert(!std::isinf(value) && "Cannot assign an " + "Inf/-Inf " + "value to a " + "float metric"); *m_memory -= value; return *this;