Skip to content

Commit

Permalink
Assert on NaN, -NaN, Inf, -Inf (#39)
Browse files Browse the repository at this point in the history
* Patch: Fix a bug where one could assign Nan, Inf or -Inf to a float64 metric.
  • Loading branch information
loglund authored Jan 10, 2024
1 parent 76680ec commit 3a6ff2e
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 1 deletion.
2 changes: 1 addition & 1 deletion NEWS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----
Expand Down
29 changes: 29 additions & 0 deletions src/abacus/metric.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#pragma once

#include <cassert>
#include <cmath>
#include <cstdint>

#include "type.hpp"
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
43 changes: 43 additions & 0 deletions test/src/test_metric.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Check warning on line 8 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / MacOS-mkspecs / Apple Big Sur (ARM)

unknown pragma ignored [-Wunknown-pragmas]

Check warning on line 8 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Linux-mkspecs / Clang 15

unknown pragma ignored [-Wunknown-pragmas]

Check warning on line 8 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Linux-mkspecs / Clang 15 Address Sanitizer

unknown pragma ignored [-Wunknown-pragmas]

Check warning on line 8 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Linux-mkspecs / Clang 15 Thread Sanitizer

unknown pragma ignored [-Wunknown-pragmas]

#include <cmath>
#include <cstdint>
#include <cstring>
#include <gtest/gtest.h>
Expand All @@ -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 warning on line 46 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Windows-mkspecs / MSVC 17 32-bit

potential divide by 0
// Check that that assignment to -NaN is not allowed
EXPECT_DEATH(double_metric = -0.0 / 0.0, "");

Check warning on line 48 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Windows-mkspecs / MSVC 17 32-bit

potential divide by 0
// Check that that assignment to +Inf is not allowed
EXPECT_DEATH(double_metric = 1 / 0.0, "");

Check warning on line 50 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Windows-mkspecs / MSVC 17 32-bit

potential divide by 0

Check warning on line 50 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Windows-mkspecs / MSVC 17 64-bit

potential divide by 0
// Check that that assignment to -Inf is not allowed
EXPECT_DEATH(double_metric = 1 / -0.0, "");

Check warning on line 52 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Windows-mkspecs / MSVC 17 32-bit

potential divide by 0

Check warning on line 52 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Windows-mkspecs / MSVC 17 64-bit

potential divide by 0

// Add and Assign
// Check that assignment to NaN is not allowed
EXPECT_DEATH(double_metric += 0.0 / 0.0, "");

Check warning on line 56 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Windows-mkspecs / MSVC 17 64-bit

potential divide by 0
// Check that that assignment to -NaN is not allowed
EXPECT_DEATH(double_metric += -0.0 / 0.0, "");

Check warning on line 58 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Windows-mkspecs / MSVC 17 64-bit

potential divide by 0
// Check that that assignment to +Inf is not allowed
EXPECT_DEATH(double_metric += 1 / 0.0, "");

Check warning on line 60 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Windows-mkspecs / MSVC 17 64-bit

potential divide by 0
// Check that that assignment to -Inf is not allowed
EXPECT_DEATH(double_metric += 1 / -0.0, "");

Check warning on line 62 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Windows-mkspecs / MSVC 17 64-bit

potential divide by 0

// Subtract and Assign
// Check that assignment to NaN is not allowed
EXPECT_DEATH(double_metric -= 0.0 / 0.0, "");

Check warning on line 66 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Windows-mkspecs / MSVC 17 64-bit

potential divide by 0
// Check that that assignment to -NaN is not allowed
EXPECT_DEATH(double_metric -= -0.0 / 0.0, "");

Check warning on line 68 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Windows-mkspecs / MSVC 17 64-bit

potential divide by 0
// Check that that assignment to +Inf is not allowed
EXPECT_DEATH(double_metric -= 1 / 0.0, "");

Check warning on line 70 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Windows-mkspecs / MSVC 17 64-bit

potential divide by 0
// Check that that assignment to -Inf is not allowed
EXPECT_DEATH(double_metric -= 1 / -0.0, "");

Check warning on line 72 in test/src/test_metric.cpp

View workflow job for this annotation

GitHub Actions / Windows-mkspecs / MSVC 17 64-bit

potential divide by 0
}

0 comments on commit 3a6ff2e

Please sign in to comment.