From 3a6ff2e15dde210bd941fb023664680b3c2f6a60 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mikkel=20H=C3=B8jlund=20Larsen?=
 <mikkelhojlundgrontmij@gmail.com>
Date: Wed, 10 Jan 2024 14:26:31 +0100
Subject: [PATCH] Assert on NaN, -NaN, Inf, -Inf (#39)

* Patch: Fix a bug where one could assign Nan, Inf or -Inf to a float64 metric.
---
 NEWS.rst                 |  2 +-
 src/abacus/metric.hpp    | 29 +++++++++++++++++++++++++++
 test/src/test_metric.cpp | 43 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 1 deletion(-)

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/src/abacus/metric.hpp b/src/abacus/metric.hpp
index 4081547..90a7494 100644
--- a/src/abacus/metric.hpp
+++ b/src/abacus/metric.hpp
@@ -6,6 +6,7 @@
 #pragma once
 
 #include <cassert>
+#include <cmath>
 #include <cstdint>
 
 #include "type.hpp"
@@ -220,6 +221,15 @@ class metric<abacus::type::float64>
     auto operator=(double value) -> metric<abacus::type::float64>&
     {
         assert(is_initialized());
+        // We don't allow assignment to NaN or Inf/-Inf
+        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;
     }
@@ -230,6 +240,15 @@ class metric<abacus::type::float64>
     auto operator+=(double value) -> metric<abacus::type::float64>&
     {
         assert(is_initialized());
+        // We don't allow assignment to NaN or Inf/-Inf
+        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;
     }
@@ -240,6 +259,16 @@ class metric<abacus::type::float64>
     auto operator-=(double value) -> metric<abacus::type::float64>&
     {
         assert(is_initialized());
+        // We don't allow assignment to NaN or Inf/-Inf
+        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;
     }
diff --git a/test/src/test_metric.cpp b/test/src/test_metric.cpp
index 36ed52d..f01eed6 100644
--- a/test/src/test_metric.cpp
+++ b/test/src/test_metric.cpp
@@ -3,6 +3,11 @@
 //
 // 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 <cmath>
 #include <cstdint>
 #include <cstring>
 #include <gtest/gtest.h>
@@ -28,3 +33,41 @@ TEST(test_metric, constructor)
     abacus::metric<abacus::type::boolean> bool_metric(&bool_count);
     EXPECT_TRUE(bool_metric.is_initialized());
 }
+
+TEST(test_metric, float_assignment)
+{
+    double double_count = 1123.12;
+    abacus::metric<abacus::type::float64> 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, "");
+}