On 2017-07-05 18:05, Max Reitz wrote: > On 2017-07-05 15:48, Max Reitz wrote: >> On 2017-07-05 09:07, Markus Armbruster wrote: >>> Max Reitz <[email protected]> writes: >>> >>>> This generic function (along with its implementations for different >>>> types) determines whether two QObjects are equal. >>>> >>>> Signed-off-by: Max Reitz <[email protected]> >>> [...] >>>> diff --git a/qobject/qnum.c b/qobject/qnum.c >>>> index 476e81c..784d061 100644 >>>> --- a/qobject/qnum.c >>>> +++ b/qobject/qnum.c >>>> @@ -213,6 +213,59 @@ QNum *qobject_to_qnum(const QObject *obj) >>>> } >>>> >>>> /** >>>> + * qnum_is_equal(): Test whether the two QNums are equal >>>> + */ >>>> +bool qnum_is_equal(const QObject *x, const QObject *y) >>>> +{ >>>> + QNum *num_x = qobject_to_qnum(x); >>>> + QNum *num_y = qobject_to_qnum(y); >>>> + >>>> + switch (num_x->kind) { >>>> + case QNUM_I64: >>>> + switch (num_y->kind) { >>>> + case QNUM_I64: >>>> + /* Comparison in native int64_t type */ >>>> + return num_x->u.i64 == num_y->u.i64; >>>> + case QNUM_U64: >>>> + /* Implicit conversion of x to uin64_t, so we have to >>>> + * check its sign before */ >>>> + return num_x->u.i64 >= 0 && num_x->u.i64 == num_y->u.u64; >>>> + case QNUM_DOUBLE: >>>> + /* Implicit conversion of x to double; no overflow >>>> + * possible */ >>>> + return num_x->u.i64 == num_y->u.dbl; >>> >>> Overflow is impossible, but loss of precision is possible: >>> >>> (double)9007199254740993ull == 9007199254740992.0 >>> >>> yields true. Is this what we want? >> >> I'd argue that yes, because the floating point value represents >> basically all of the values which are "equal" to it. >> >> But I don't have a string opinion. I guess the alternative would be to >> convert the double to an integer instead and check for overflows before? >> >>>> + } >>>> + abort(); >>>> + case QNUM_U64: >>>> + switch (num_y->kind) { >>>> + case QNUM_I64: >>>> + return qnum_is_equal(y, x); >>>> + case QNUM_U64: >>>> + /* Comparison in native uint64_t type */ >>>> + return num_x->u.u64 == num_y->u.u64; >>>> + case QNUM_DOUBLE: >>>> + /* Implicit conversion of x to double; no overflow >>>> + * possible */ >>>> + return num_x->u.u64 == num_y->u.dbl; >>> >>> Similar loss of precision. >>> >>>> + } >>>> + abort(); >>>> + case QNUM_DOUBLE: >>>> + switch (num_y->kind) { >>>> + case QNUM_I64: >>>> + return qnum_is_equal(y, x); >>>> + case QNUM_U64: >>>> + return qnum_is_equal(y, x); >>>> + case QNUM_DOUBLE: >>>> + /* Comparison in native double type */ >>>> + return num_x->u.dbl == num_y->u.dbl; >>>> + } >>>> + abort(); >>>> + } >>>> + >>>> + abort(); >>>> +} >>> >>> I think there's more than one sane interpretations of "is equal", >>> including: >>> >>> * The mathematical numbers represented by @x and @y are equal. >>> >>> * @x and @y have the same contents, i.e. same kind and u. >>> >>> * @x and @y are the same object (listed for completeness; we don't need >>> a function to compare pointers). >>> >>> Your patch implements yet another one. Which one do we want, and why? >> >> Mine is the first one, except that I think that a floating point value >> does not represent a single number but just some number in a range. >> >>> The second is easier to implement than the first. >> >> It seems much less useful, though. >> >>> If we really want the first, you need to fix the loss of precision bugs. >> >> I'm not sure, but I don't mind either, so... >> >>> I guess the obvious fix is >>> >>> return (double)x == x && x == y; >> >> Yes, that would do, too; and spares me of having to think about how well >> comparing an arbitrary double to UINT64_MAX actually works. :-) > > On second thought, this won't do, because (double)x == x is always true > if x is an integer (because this will implicitly cast the second x to a > double, too). However, (uint64_t)(double)x == x should work.
Hm, well, the nice thing with this is that (double)UINT64_MAX is actually UINT64_MAX + 1, and now (uint64_t)(UINT64_MAX + 1) is undefined... Urgs. So I guess one thing that isn't very obvious but that should *always* work (and is always well-defined) is this: For uint64_t: y < 0x1p64 && (uint64_t)y == x For int64_t: y >= -0x1p63 && y < 0x1p63 && (int64_t)y == x I hope. :-/ (But finally a chance to use binary exponents! Yay!) Max
signature.asc
Description: OpenPGP digital signature
