Skip to content

Commit

Permalink
Get ieee754 total ordering right in compareTo
Browse files Browse the repository at this point in the history
  • Loading branch information
nielsenko committed Apr 17, 2023
1 parent 08cc103 commit 05466c3
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 23 deletions.
14 changes: 2 additions & 12 deletions lib/src/decimal128.dart
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class Decimal128 extends Comparable<Decimal128> {
}

/// Negates `this` and returns a new [Decimal128].
Decimal128 operator -() => zero - this;
Decimal128 operator -() => Decimal128._(lib.realm_dart_decimal128_negate(_value));

/// Returns the absolute value of `this`.
Decimal128 abs() => this < zero ? -this : this;
Expand Down Expand Up @@ -144,16 +144,6 @@ class Decimal128 extends Comparable<Decimal128> {
}

/// Compares `this` to `other`.
/// Note that comparisons of `nan` values follow the IEEE 754 standard.
/// In particular, `nan == nan` is `false`.
@override
int compareTo(Decimal128 other) {
if (this < other) {
return -1;
} else if (this == other) {
return 0;
} else {
return 1;
}
}
int compareTo(Decimal128 other) => lib.realm_dart_decimal128_compare_to(_value, other._value);
}
41 changes: 41 additions & 0 deletions lib/src/native/realm_bindings.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3201,6 +3201,24 @@ class RealmLibrary {
realm_decimal128_t Function(
realm_decimal128_t, realm_decimal128_t)>();

int realm_dart_decimal128_compare_to(
realm_decimal128_t x,
realm_decimal128_t y,
) {
return _realm_dart_decimal128_compare_to(
x,
y,
);
}

late final _realm_dart_decimal128_compare_toPtr = _lookup<
ffi.NativeFunction<
ffi.Int Function(realm_decimal128_t,
realm_decimal128_t)>>('realm_dart_decimal128_compare_to');
late final _realm_dart_decimal128_compare_to =
_realm_dart_decimal128_compare_toPtr
.asFunction<int Function(realm_decimal128_t, realm_decimal128_t)>();

realm_decimal128_t realm_dart_decimal128_divide(
realm_decimal128_t x,
realm_decimal128_t y,
Expand Down Expand Up @@ -3347,6 +3365,20 @@ class RealmLibrary {
late final _realm_dart_decimal128_nan =
_realm_dart_decimal128_nanPtr.asFunction<realm_decimal128_t Function()>();

realm_decimal128_t realm_dart_decimal128_negate(
realm_decimal128_t decimal,
) {
return _realm_dart_decimal128_negate(
decimal,
);
}

late final _realm_dart_decimal128_negatePtr = _lookup<
ffi.NativeFunction<realm_decimal128_t Function(realm_decimal128_t)>>(
'realm_dart_decimal128_negate');
late final _realm_dart_decimal128_negate = _realm_dart_decimal128_negatePtr
.asFunction<realm_decimal128_t Function(realm_decimal128_t)>();

realm_decimal128_t realm_dart_decimal128_subtract(
realm_decimal128_t x,
realm_decimal128_t y,
Expand Down Expand Up @@ -10685,6 +10717,11 @@ class _SymbolAddresses {
realm_decimal128_t Function(
realm_decimal128_t, realm_decimal128_t)>>
get realm_dart_decimal128_add => _library._realm_dart_decimal128_addPtr;
ffi.Pointer<
ffi.NativeFunction<
ffi.Int Function(realm_decimal128_t, realm_decimal128_t)>>
get realm_dart_decimal128_compare_to =>
_library._realm_dart_decimal128_compare_toPtr;
ffi.Pointer<
ffi.NativeFunction<
realm_decimal128_t Function(
Expand Down Expand Up @@ -10725,6 +10762,10 @@ class _SymbolAddresses {
_library._realm_dart_decimal128_multiplyPtr;
ffi.Pointer<ffi.NativeFunction<realm_decimal128_t Function()>>
get realm_dart_decimal128_nan => _library._realm_dart_decimal128_nanPtr;
ffi.Pointer<
ffi.NativeFunction<realm_decimal128_t Function(realm_decimal128_t)>>
get realm_dart_decimal128_negate =>
_library._realm_dart_decimal128_negatePtr;
ffi.Pointer<
ffi.NativeFunction<
realm_decimal128_t Function(
Expand Down
2 changes: 1 addition & 1 deletion src/realm-core
25 changes: 17 additions & 8 deletions src/realm_dart.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ using namespace realm::c_api;

class WeakHandle {
public:
WeakHandle(Dart_Handle handle) : m_weakHandle(Dart_NewWeakPersistentHandle_DL(handle, this, 1, finalize_handle)) {
WeakHandle(Dart_Handle handle): m_weakHandle(Dart_NewWeakPersistentHandle_DL(handle, this, 1, finalize_handle)) {
}

Dart_Handle value() {
Expand Down Expand Up @@ -136,19 +136,19 @@ RLM_API const char* realm_dart_library_version() { return "1.0.3"; }
// }

void handle_finalizer(void* isolate_callback_data, void* realmPtr) {
realm_release(realmPtr);
realm_release(realmPtr);
}

RLM_API void* realm_attach_finalizer(Dart_Handle handle, void* realmPtr, int size) {
return Dart_NewFinalizableHandle_DL(handle, realmPtr, size, handle_finalizer);
return Dart_NewFinalizableHandle_DL(handle, realmPtr, size, handle_finalizer);
}

RLM_API void realm_dettach_finalizer(void* finalizableHandle, Dart_Handle handle) {
Dart_FinalizableHandle finalHandle = reinterpret_cast<Dart_FinalizableHandle>(finalizableHandle);
return Dart_DeleteFinalizableHandle_DL(finalHandle, handle);
Dart_FinalizableHandle finalHandle = reinterpret_cast<Dart_FinalizableHandle>(finalizableHandle);
return Dart_DeleteFinalizableHandle_DL(finalHandle, handle);
}

RLM_API void realm_set_auto_refresh(realm_t* realm, bool enable){
RLM_API void realm_set_auto_refresh(realm_t* realm, bool enable) {
(*realm)->set_auto_refresh(enable);
}

Expand All @@ -166,7 +166,7 @@ RLM_API realm_string_t realm_dart_decimal128_to_string(realm_decimal128_t x) {
static char buffer[34]; // 34 bytes is the maximum length of a string representation of a decimal128
unsigned int flags = 0;
__bid128_to_string(buffer, (BID_UINT128*)&x, &flags);
return realm_string_t{buffer, strlen(buffer)};
return realm_string_t{ buffer, strlen(buffer) };
}

RLM_API realm_decimal128_t realm_dart_decimal128_nan() {
Expand All @@ -181,12 +181,16 @@ RLM_API realm_decimal128_t realm_dart_decimal128_from_int64(int64_t x) {
return to_capi(Decimal128(x));
}

RLM_API int64_t realm_dart_decimal128_to_int64(realm_decimal128_t decimal) {
RLM_API int64_t realm_dart_decimal128_to_int64(realm_decimal128_t decimal) {
int64_t result;
from_capi(decimal).to_int(result);
return result;
}

RLM_API realm_decimal128_t realm_dart_decimal128_negate(realm_decimal128_t decimal) {
return to_capi(-from_capi(decimal));
}

RLM_API realm_decimal128_t realm_dart_decimal128_add(realm_decimal128_t x, realm_decimal128_t y) {
return to_capi(from_capi(x) + from_capi(y));
}
Expand Down Expand Up @@ -220,3 +224,8 @@ RLM_API bool realm_dart_decimal128_greater_than(realm_decimal128_t x, realm_deci
auto y_decimal = from_capi(y);
return !x_decimal.is_nan() && !y_decimal.is_nan() && x_decimal > y_decimal;
}

RLM_API int realm_dart_decimal128_compare_to(realm_decimal128_t x, realm_decimal128_t y) {
return from_capi(x).compareTotalOrdering(from_capi(y));
}

2 changes: 2 additions & 0 deletions src/realm_dart.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,14 @@ RLM_API realm_decimal128_t realm_dart_decimal128_nan();
RLM_API bool realm_dart_decimal128_is_nan(realm_decimal128_t decimal);
RLM_API realm_decimal128_t realm_dart_decimal128_from_int64(int64_t low);
RLM_API int64_t realm_dart_decimal128_to_int64(realm_decimal128_t decimal);
RLM_API realm_decimal128_t realm_dart_decimal128_negate(realm_decimal128_t decimal);
RLM_API realm_decimal128_t realm_dart_decimal128_add(realm_decimal128_t x, realm_decimal128_t y);
RLM_API realm_decimal128_t realm_dart_decimal128_subtract(realm_decimal128_t x, realm_decimal128_t y);
RLM_API realm_decimal128_t realm_dart_decimal128_multiply(realm_decimal128_t x, realm_decimal128_t y);
RLM_API realm_decimal128_t realm_dart_decimal128_divide(realm_decimal128_t x, realm_decimal128_t y);
RLM_API bool realm_dart_decimal128_equal(realm_decimal128_t x, realm_decimal128_t y);
RLM_API bool realm_dart_decimal128_less_than(realm_decimal128_t x, realm_decimal128_t y);
RLM_API bool realm_dart_decimal128_greater_than(realm_decimal128_t x, realm_decimal128_t y);
RLM_API int realm_dart_decimal128_compare_to(realm_decimal128_t x, realm_decimal128_t y);

#endif // REALM_DART_H
83 changes: 81 additions & 2 deletions test/decimal128_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
//
////////////////////////////////////////////////////////////////////////////////
import 'dart:io';
import 'dart:math';

import 'package:meta/meta.dart';
Expand All @@ -24,13 +25,13 @@ import 'package:test/test.dart';

const int defaultTimes = 100000;

@isTest
void repeat(dynamic Function() body, [int times = defaultTimes]) {
for (var i = 0; i < times; ++i) {
body();
}
}

@isTest
void repeatTest(String description, dynamic Function(Decimal128 x, int xInt, Decimal128 y, int yInt) body, [int times = defaultTimes]) {
final r = Random(42); // use a fixed seed to make tests deterministic
test('$description ($times variations)', () {
Expand All @@ -51,11 +52,55 @@ void repeatTest(String description, dynamic Function(Decimal128 x, int xInt, Dec

Future<void> main([List<String>? args]) async {
test('Decimal128.nan', () {
// Test that we mimic the behavior of Dart's double wrt. NaN and
// <, <=, >, >=. Unlike compareTo (which define a total order) these
// operators return false for NaN.
expect(0.0, isNot(double.nan));
expect(0.0, isNot(lessThan(double.nan)));
expect(0.0, isNot(lessThanOrEqualTo(double.nan)));
expect(0.0, isNot(greaterThan(double.nan)));
expect(0.0, isNot(greaterThanOrEqualTo(double.nan)));

expect(double.nan, isNot(0.0));
expect(double.nan, isNot(lessThan(0.0)));
expect(double.nan, isNot(lessThanOrEqualTo(0.0)));
expect(double.nan, isNot(greaterThan(0.0)));
expect(double.nan, isNot(greaterThanOrEqualTo(0.0)));

expect(double.nan, isNot(double.nan));
expect(double.nan, isNot(lessThan(double.nan)));
expect(double.nan, isNot(lessThanOrEqualTo(double.nan)));
expect(double.nan, isNot(greaterThan(double.nan)));
expect(double.nan, isNot(greaterThanOrEqualTo(double.nan)));

expect(double.nan.isNaN, isTrue);
expect((0.0).isNaN, isFalse);
expect((0.0 / 0.0).isNaN, isTrue);
expect((1.0).isNaN, isFalse);
expect((10.0).isNaN, isFalse);
expect(double.infinity.isNaN, isFalse);

// NaN != NaN so compare as strings
expect(Decimal128.tryParse(Decimal128.nan.toString()).toString(), Decimal128.nan.toString());

expect(Decimal128.zero, isNot(Decimal128.nan));
expect(Decimal128.zero, isNot(lessThan(Decimal128.nan)));
expect(Decimal128.zero, isNot(lessThanOrEqualTo(Decimal128.nan)));
expect(Decimal128.zero, isNot(greaterThan(Decimal128.nan)));
expect(Decimal128.zero, isNot(greaterThanOrEqualTo(Decimal128.nan)));

expect(Decimal128.nan, isNot(Decimal128.zero));
expect(Decimal128.nan, isNot(lessThan(Decimal128.zero)));
expect(Decimal128.nan, isNot(lessThanOrEqualTo(Decimal128.zero)));
expect(Decimal128.nan, isNot(greaterThan(Decimal128.zero)));
expect(Decimal128.nan, isNot(greaterThanOrEqualTo(Decimal128.zero)));

expect(Decimal128.nan, isNot(Decimal128.nan));
expect(Decimal128.nan, isNot(lessThan(Decimal128.nan)));
expect(Decimal128.nan, isNot(lessThanOrEqualTo(Decimal128.nan)));
expect(Decimal128.nan, isNot(greaterThan(Decimal128.nan)));
expect(Decimal128.nan, isNot(greaterThanOrEqualTo(Decimal128.nan)));

expect(Decimal128.nan.isNaN, isTrue);
expect(Decimal128.zero.isNaN, isFalse);
expect((Decimal128.zero / Decimal128.zero).isNaN, isTrue);
Expand Down Expand Up @@ -159,8 +204,42 @@ Future<void> main([List<String>? args]) async {
expect((Decimal128.infinity * Decimal128.zero).isNaN, isTrue);
});

test('Decimal128.compareTo is a total ordering', () {
// Test that we mimic the behavior of Dart's double wrt. compareTo in relation
// to IEEE754-2019, ie. +/- NaN, +/- infinity and +/- zero and define a total
// ordering.
expect((-0.0).compareTo(0.0), -1);
expect((0.0).compareTo(-0.0), 1);
expect((0.0).compareTo(0.0), 0);
expect((-0.0).compareTo(-0.0), 0);

expect(double.negativeInfinity.compareTo(double.infinity), -1);
expect(double.infinity.compareTo(double.negativeInfinity), 1);
expect(double.infinity.compareTo(double.infinity), 0);
expect(double.negativeInfinity.compareTo(double.negativeInfinity), 0);

expect((1.0).compareTo(double.nan), -1);
expect(double.nan.compareTo(double.nan), 0);
expect(double.nan.compareTo(1.0), 1);

// .. and now for Decimal128
expect((-Decimal128.zero).compareTo(Decimal128.zero), -1);
expect(Decimal128.zero.compareTo(-Decimal128.zero), 1);
expect(Decimal128.zero.compareTo(Decimal128.zero), 0);
expect((-Decimal128.zero).compareTo(-Decimal128.zero), 0);

expect(Decimal128.negativeInfinity.compareTo(Decimal128.infinity), -1);
expect(Decimal128.infinity.compareTo(Decimal128.negativeInfinity), 1);
expect(Decimal128.infinity.compareTo(Decimal128.infinity), 0);
expect(Decimal128.negativeInfinity.compareTo(Decimal128.negativeInfinity), 0);

expect(Decimal128.one.compareTo(Decimal128.nan), -1);
expect(Decimal128.nan.compareTo(Decimal128.nan), 0);
expect(Decimal128.nan.compareTo(Decimal128.one), 1);
});

repeatTest('Decimal128.compareTo + <, <=, ==, !=, >=, >', (x, xInt, y, yInt) {
expect(x.compareTo(y), -y.compareTo(x));
expect(x.compareTo(y), -(y.compareTo(x)));
expect(x.compareTo(y), xInt.compareTo(yInt));
expect(x == y, y == x);
expect(x == y, xInt == yInt);
Expand Down

0 comments on commit 05466c3

Please sign in to comment.