Skip to content

Commit f91d298

Browse files
committed
apply pitrou suggestion
1 parent bfa359c commit f91d298

File tree

3 files changed

+10
-26
lines changed

3 files changed

+10
-26
lines changed

cpp/src/arrow/testing/gtest_util_test.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
#include "arrow/type_traits.h"
3737
#include "arrow/util/checked_cast.h"
3838
#include "arrow/util/float16.h"
39-
#include "arrow/util/logger.h"
4039

4140
namespace arrow::util {
4241

cpp/src/arrow/testing/math.cc

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
#include "arrow/util/float16.h"
2828
#include "arrow/util/logging_internal.h"
29+
#include "arrow/util/ubsan.h"
2930

3031
namespace arrow {
3132
namespace {
@@ -52,14 +53,14 @@ template <typename Float>
5253
struct UlpDistanceUtil {
5354
public:
5455
using UIntType = typename FloatToUInt<Float>::Type;
55-
static const UIntType kNumberOfBits = sizeof(Float) * 8;
56-
static const UIntType kSignMask = static_cast<UIntType>(1) << (kNumberOfBits - 1);
56+
static constexpr UIntType kNumberOfBits = sizeof(Float) * 8;
57+
static constexpr UIntType kSignMask = static_cast<UIntType>(1) << (kNumberOfBits - 1);
5758

5859
// This implementation is inspired by:
5960
// https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/
60-
static UIntType UlpDistnace(Float left, Float right) {
61-
auto unsigned_left = BitCast<UIntType>(left);
62-
auto unsigned_right = BitCast<UIntType>(right);
61+
static UIntType UlpDistance(Float left, Float right) {
62+
auto unsigned_left = util::SafeCopy<UIntType>(left);
63+
auto unsigned_right = util::SafeCopy<UIntType>(right);
6364
auto biased_left = ConvertSignAndMagnitudeToBiased(unsigned_left);
6465
auto biased_right = ConvertSignAndMagnitudeToBiased(unsigned_right);
6566
if (biased_left > biased_right) {
@@ -69,22 +70,8 @@ struct UlpDistanceUtil {
6970
}
7071

7172
private:
72-
template <typename To, typename From>
73-
union BitCastUnion {
74-
explicit BitCastUnion(From from) : from(from) {}
75-
From from;
76-
To to;
77-
};
78-
79-
template <typename To, typename From>
80-
static UIntType BitCast(From value) {
81-
assert(sizeof(To) == sizeof(From));
82-
BitCastUnion<To, From> bit_cast(value);
83-
return bit_cast.to;
84-
}
85-
8673
// Source reference (GoogleTest):
87-
// https://github.com/google/googletest/blob/085af2cc08600bdb13827ca40261abcbe5048bb5/googletest/include/gtest/internal/gtest-internal.h#L336-L342
74+
// https://github.com/google/googletest/blob/1b96fa13f549387b7549cc89e1a785cf143a1a50/googletest/include/gtest/internal/gtest-internal.h#L345-L368
8875
static UIntType ConvertSignAndMagnitudeToBiased(UIntType value) {
8976
if (value & kSignMask) {
9077
return ~value + 1;
@@ -116,7 +103,7 @@ bool WithinUlpGeneric(Float left, Float right, int n_ulps) {
116103
}
117104

118105
DCHECK_GE(n_ulps, 1);
119-
return UlpDistanceUtil<Float>::UlpDistnace(left, right) <=
106+
return UlpDistanceUtil<Float>::UlpDistance(left, right) <=
120107
static_cast<uint64_t>(n_ulps);
121108
}
122109

@@ -144,6 +131,7 @@ bool WithinUlp(double left, double right, int n_ulps) {
144131
void AssertWithinUlp(util::Float16 left, util::Float16 right, int n_ulps) {
145132
AssertWithinUlpGeneric(left, right, n_ulps);
146133
}
134+
147135
void AssertWithinUlp(float left, float right, int n_ulps) {
148136
AssertWithinUlpGeneric(left, right, n_ulps);
149137
}

cpp/src/arrow/testing/math.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,10 @@
1818
#pragma once
1919

2020
#include "arrow/testing/visibility.h"
21+
#include "arrow/type_fwd.h"
2122

2223
namespace arrow {
2324

24-
namespace util {
25-
class Float16;
26-
}
27-
2825
ARROW_TESTING_EXPORT
2926
bool WithinUlp(util::Float16 left, util::Float16 right, int n_ulps);
3027
ARROW_TESTING_EXPORT

0 commit comments

Comments
 (0)