gnucash maint: Multiple changes pushed
John Ralls
jralls at code.gnucash.org
Mon May 13 18:19:29 EDT 2019
Updated via https://github.com/Gnucash/gnucash/commit/e6c50357 (commit)
via https://github.com/Gnucash/gnucash/commit/8738644a (commit)
via https://github.com/Gnucash/gnucash/commit/aab89065 (commit)
from https://github.com/Gnucash/gnucash/commit/d4c524a9 (commit)
commit e6c50357bbff398cdba76213d7d1620ca8f6bd34
Author: John Ralls <jralls at ceridwen.us>
Date: Mon May 13 15:19:19 2019 -0700
Fix test error string to match actual error.
diff --git a/libgnucash/engine/test/utest-gnc-pricedb.c b/libgnucash/engine/test/utest-gnc-pricedb.c
index 670bf6f5e..5d1aec75f 100644
--- a/libgnucash/engine/test/utest-gnc-pricedb.c
+++ b/libgnucash/engine/test/utest-gnc-pricedb.c
@@ -1071,7 +1071,7 @@ gnc_pricedb_lookup_day_t64(GNCPriceDB *db,// C: 4 in 2 SCM: 2 in 1 Local: 1:0:0
static void
test_gnc_pricedb_lookup_day_t64 (PriceDBFixture *fixture, gconstpointer pData)
{
- gchar *msg1 = "[gnc_dmy2timespec_internal()] Date computation error from Y-M-D 12-11-18: Year is out of valid range: 1400..10000";
+ gchar *msg1 = "[gnc_dmy2time64_internal()] Date computation error from Y-M-D 12-11-18: Year is out of valid range: 1400..9999";
gint loglevel = G_LOG_LEVEL_WARNING | G_LOG_FLAG_FATAL;
gchar *logdomain = "qof.engine";
TestErrorStruct check = {loglevel, logdomain, msg1, 0};
commit 8738644af0dc70914c2f128ee985246a7e068f8d
Merge: d4c524a9e aab89065d
Author: John Ralls <jralls at ceridwen.us>
Date: Mon May 13 15:13:52 2019 -0700
Merge David Palma's fix-division into maint.
commit aab89065da63db13d003dc75a1f63a2840928667
Author: David Palma <dbpalma9 at gmail.com>
Date: Sat May 4 15:02:02 2019 +0100
Bug 796949 - Fix division and rounding of zero.
Fix division of 128-bit integers so that the remainder inherits the dividend's sign.
Fix rounding for truncated zero.
diff --git a/libgnucash/engine/gnc-int128.cpp b/libgnucash/engine/gnc-int128.cpp
index e545649f7..8ec77f5ce 100644
--- a/libgnucash/engine/gnc-int128.cpp
+++ b/libgnucash/engine/gnc-int128.cpp
@@ -71,7 +71,7 @@ GncInt128::GncInt128 (int64_t upper, int64_t lower, uint8_t flags) :
m_lo {static_cast<uint64_t>(lower < 0 ? -lower : lower)}
{
if ((upper < 0 && lower > 0) || (upper > 0 && lower < 0))
- m_lo = (m_hi << 63) - m_lo;
+ m_lo = (m_hi << 63) - m_lo;
else
m_lo += (m_hi << 63);
@@ -142,7 +142,7 @@ GncInt128::operator int64_t() const
GncInt128::operator uint64_t() const
{
auto flags = get_flags(m_hi);
- if (flags & neg)
+ if ((flags & neg) && !isZero()) // exclude negative zero
throw std::underflow_error ("Can't represent negative value as uint64_t");
if ((flags & (overflow | NaN)) || (m_hi || m_lo > UINT64_MAX))
throw std::overflow_error ("Value to large to represent as uint64_t");
@@ -160,14 +160,15 @@ GncInt128::cmp (const GncInt128& b) const noexcept
return 1;
auto hi = get_num(m_hi);
auto bhi = get_num(b.m_hi);
+ if (isZero() && b.isZero()) return 0;
if (flags & neg)
{
- if (!b.isNeg()) return -1;
- if (hi > bhi) return -1;
- if (hi < bhi) return 1;
- if (m_lo > b.m_lo) return -1;
- if (m_lo < b.m_lo) return 1;
- return 0;
+ if (!b.isNeg()) return -1;
+ if (hi > bhi) return -1;
+ if (hi < bhi) return 1;
+ if (m_lo > b.m_lo) return -1;
+ if (m_lo < b.m_lo) return 1;
+ return 0;
}
if (b.isNeg()) return 1;
if (hi < bhi) return -1;
@@ -309,9 +310,9 @@ GncInt128::operator-() const noexcept
auto retval = *this;
auto flags = get_flags(retval.m_hi);
if (isNeg())
- flags ^= neg;
+ flags ^= neg;
else
- flags |= neg;
+ flags |= neg;
retval.m_hi = set_flags(retval.m_hi, flags);
return retval;
}
@@ -357,7 +358,7 @@ GncInt128::operator+= (const GncInt128& b) noexcept
if (isOverflow() || isNan())
return *this;
if ((isNeg () && !b.isNeg ()) || (!isNeg () && b.isNeg ()))
- return this->operator-= (-b);
+ return this->operator-= (-b);
uint64_t result = m_lo + b.m_lo;
uint64_t carry = static_cast<int64_t>(result < m_lo); //Wrapping
m_lo = result;
@@ -365,7 +366,7 @@ GncInt128::operator+= (const GncInt128& b) noexcept
auto bhi = get_num(b.m_hi);
result = hi + bhi + carry;
if (result < hi || result & flagmask)
- flags |= overflow;
+ flags |= overflow;
m_hi = set_flags(result, flags);
return *this;
}
@@ -439,7 +440,7 @@ GncInt128::operator-= (const GncInt128& b) noexcept
return *this;
if ((!isNeg() && b.isNeg()) || (isNeg() && !b.isNeg()))
- return this->operator+= (-b);
+ return this->operator+= (-b);
bool operand_bigger {abs().cmp (b.abs()) < 0};
auto hi = get_num(m_hi);
auto far_hi = get_num(b.m_hi);
@@ -478,6 +479,8 @@ GncInt128::operator*= (const GncInt128& b) noexcept
{
/* Test for 0 first */
auto flags = get_flags(m_hi);
+ /* Handle the sign; ^ flips if b is negative */
+ flags ^= (get_flags(b.m_hi) & neg);
if (isZero() || b.isZero())
{
m_lo = 0;
@@ -514,8 +517,7 @@ GncInt128::operator*= (const GncInt128& b) noexcept
m_hi = set_flags(m_hi, flags);
return *this;
}
- /* Handle the sign; ^ flips if b is negative */
- flags ^= (get_flags(b.m_hi) & neg);
+
/* The trivial case */
if (abits + bbits <= legbits)
{
@@ -602,6 +604,7 @@ div_multi_leg (uint64_t* u, size_t m, uint64_t* v, size_t n,
uint64_t d {(UINT64_C(1) << sublegbits)/(v[n - 1] + UINT64_C(1))};
uint64_t carry {UINT64_C(0)};
bool negative {q.isNeg()};
+ bool rnegative {r.isNeg()};
for (size_t i = 0; i < m; ++i)
{
u[i] = u[i] * d + carry;
@@ -689,6 +692,7 @@ div_multi_leg (uint64_t* u, size_t m, uint64_t* v, size_t n,
r = GncInt128 ((u[3] << sublegbits) + u[2], (u[1] << sublegbits) + u[0]);
r /= d;
if (negative) q = -q;
+ if (rnegative) r = -r;
}
void
@@ -698,6 +702,7 @@ div_single_leg (uint64_t* u, size_t m, uint64_t v,
uint64_t qv[sublegs] {};
uint64_t carry {};
bool negative {q.isNeg()};
+ bool rnegative {r.isNeg()};
for (int i = m - 1; i >= 0; --i)
{
qv[i] = u[i] / v;
@@ -713,13 +718,18 @@ div_single_leg (uint64_t* u, size_t m, uint64_t v,
q = GncInt128 ((qv[3] << sublegbits) + qv[2], (qv[1] << sublegbits) + qv[0]);
r = GncInt128 ((u[3] << sublegbits) + u[2], (u[1] << sublegbits) + u[0]);
if (negative) q = -q;
+ if (rnegative) r = -r;
}
}// namespace
- void
+void
GncInt128::div (const GncInt128& b, GncInt128& q, GncInt128& r) const noexcept
{
+ // clear remainder and quotient
+ r = GncInt128(0);
+ q = GncInt128(0);
+
auto qflags = get_flags(q.m_hi);
auto rflags = get_flags(r.m_hi);
if (isOverflow() || b.isOverflow())
@@ -745,6 +755,7 @@ GncInt128::div (const GncInt128& b, GncInt128& q, GncInt128& r) const noexcept
assert (&r != &b);
q.zero(), r.zero();
+ qflags = rflags = 0;
if (b.isZero())
{
qflags |= NaN;
@@ -755,11 +766,17 @@ GncInt128::div (const GncInt128& b, GncInt128& q, GncInt128& r) const noexcept
}
if (isNeg())
+ {
qflags |= neg;
+ rflags |= neg; // the remainder inherits the dividend's sign
+ }
if (b.isNeg())
qflags ^= neg;
+ q.m_hi = set_flags(q.m_hi, qflags);
+ r.m_hi = set_flags(r.m_hi, rflags);
+
if (abs() < b.abs())
{
r = *this;
@@ -767,7 +784,7 @@ GncInt128::div (const GncInt128& b, GncInt128& q, GncInt128& r) const noexcept
}
auto hi = get_num(m_hi);
auto bhi = get_num(b.m_hi);
- q.m_hi = set_flags(hi, qflags);
+
if (hi == 0 && bhi == 0) //let the hardware do it
{
assert (b.m_lo != 0); // b.m_hi is 0 but b isn't or we didn't get here.
@@ -914,6 +931,11 @@ GncInt128::asCharBufR(char* buf) const noexcept
sprintf (buf, "%s", "NaN");
return buf;
}
+ if (isZero())
+ {
+ sprintf (buf, "%d", 0);
+ return buf;
+ }
uint64_t d[dec_array_size] {};
decimal_from_binary(d, get_num(m_hi), m_lo);
char* next = buf;
diff --git a/libgnucash/engine/gnc-rational-rounding.hpp b/libgnucash/engine/gnc-rational-rounding.hpp
index 692301701..11987b6cb 100644
--- a/libgnucash/engine/gnc-rational-rounding.hpp
+++ b/libgnucash/engine/gnc-rational-rounding.hpp
@@ -25,6 +25,12 @@
#include "gnc-numeric.h"
#include "gnc-int128.hpp"
+template <typename T> inline bool
+quotient_is_positive(T dividend, T divisor)
+{
+ return (dividend > 0 && divisor > 0) || (dividend < 0 && divisor < 0);
+}
+
enum class RoundType
{
floor = GNC_HOW_RND_FLOOR,
@@ -72,8 +78,23 @@ round(T num, T den, T rem, RT2T<RoundType::floor>)
// << ", and rem " << rem << ".\n";
if (rem == 0)
return num;
- if (num < 0)
- return num + 1;
+ // floor num==0 that is the quotient of two numbers with opposite signs
+ if (num < 0 || (num == 0 && !quotient_is_positive(rem, den)))
+ return num - 1;
+ return num;
+}
+
+
+template <> inline GncInt128
+round<GncInt128>(GncInt128 num, GncInt128 den, GncInt128 rem,
+ RT2T<RoundType::floor>)
+{
+// std::cout << "Rounding to floor with num " << num << " den " << den
+// << ", and rem " << rem << ".\n";
+ if (rem == 0)
+ return num;
+ if (num.isNeg())
+ return num - 1;
return num;
}
@@ -82,7 +103,18 @@ round(T num, T den, T rem, RT2T<RoundType::ceiling>)
{
if (rem == 0)
return num;
- if (num > 0)
+ if (num > 0 || (num == 0 && quotient_is_positive(rem, den)))
+ return num + 1;
+ return num;
+}
+
+template <> inline GncInt128
+round<GncInt128>(GncInt128 num, GncInt128 den, GncInt128 rem,
+ RT2T<RoundType::ceiling>)
+{
+ if (rem == 0)
+ return num;
+ if (!num.isNeg())
return num + 1;
return num;
}
@@ -98,16 +130,31 @@ round(T num, T den, T rem, RT2T<RoundType::promote>)
{
if (rem == 0)
return num;
+ if (num == 0)
+ return (!quotient_is_positive(rem, den) ? -1 : 1);
return num + (num < 0 ? -1 : 1);
}
+template <> inline GncInt128
+round<GncInt128>(GncInt128 num, GncInt128 den, GncInt128 rem,
+ RT2T<RoundType::promote>)
+{
+ if (rem == 0)
+ return num;
+ return num + (num.isNeg() ? -1 : 1);
+}
+
template <typename T> inline T
round(T num, T den, T rem, RT2T<RoundType::half_down>)
{
if (rem == 0)
return num;
if (std::abs(rem * 2) > std::abs(den))
+ {
+ if (num == 0)
+ return (!quotient_is_positive(rem, den) ? -1 : 1);
return num + (num < 0 ? -1 : 1);
+ }
return num;
}
@@ -118,7 +165,7 @@ round<GncInt128>(GncInt128 num, GncInt128 den, GncInt128 rem,
if (rem == 0)
return num;
if (rem.abs() * 2 > den.abs())
- return num + (num < 0 ? -1 : 1);
+ return num + (num.isNeg() ? -1 : 1);
return num;
}
@@ -128,7 +175,11 @@ round(T num, T den, T rem, RT2T<RoundType::half_up>)
if (rem == 0)
return num;
if (std::abs(rem) * 2 >= std::abs(den))
+ {
+ if (num == 0)
+ return (!quotient_is_positive(rem, den) ? -1 : 1);
return num + (num < 0 ? -1 : 1);
+ }
return num;
}
@@ -139,7 +190,7 @@ round<GncInt128>(GncInt128 num, GncInt128 den, GncInt128 rem,
if (rem == 0)
return num;
if (rem.abs() * 2 >= den.abs())
- return num + (num < 0 ? -1 : 1);
+ return num + (num.isNeg() ? -1 : 1);
return num;
}
@@ -150,11 +201,15 @@ round(T num, T den, T rem, RT2T<RoundType::bankers>)
return num;
if (std::abs(rem * 2) > std::abs(den) ||
(std::abs(rem * 2) == std::abs(den) && num % 2))
- return num += (num < 0 ? -1 : 1);
+ {
+ if (num == 0)
+ return (!quotient_is_positive(rem, den) ? -1 : 1);
+ return num + (num < 0 ? -1 : 1);
+ }
return num;
}
-template<> inline GncInt128
+template <> inline GncInt128
round<GncInt128>(GncInt128 num, GncInt128 den, GncInt128 rem,
RT2T<RoundType::bankers>)
{
@@ -162,7 +217,7 @@ round<GncInt128>(GncInt128 num, GncInt128 den, GncInt128 rem,
return num;
if (rem.abs() * 2 > den.abs() ||
(rem.abs() * 2 == den.abs() && num % 2))
- return num += (num < 0 ? -1 : 1);
+ return num + (num.isNeg() ? -1 : 1);
return num;
}
#endif //__GNC_RATIONAL_ROUNDING_HPP__
diff --git a/libgnucash/engine/test/gtest-gnc-int128.cpp b/libgnucash/engine/test/gtest-gnc-int128.cpp
index 5aa04f8f6..82ed98ba0 100644
--- a/libgnucash/engine/test/gtest-gnc-int128.cpp
+++ b/libgnucash/engine/test/gtest-gnc-int128.cpp
@@ -432,7 +432,7 @@ TEST(GncInt128_functions, divide)
GncInt128 big (sarg, barg);
GncInt128 bigger (static_cast<uint64_t>(sarg), uarg);
GncInt128 nsmall = -small;
- GncInt128 nbig = -bigger;
+ GncInt128 nbigger = -bigger;
EXPECT_EQ (GncInt128(INT64_C(0)), zero /= smallest);
EXPECT_EQ (GncInt128(INT64_C(0)), zero %= smallest);
@@ -451,11 +451,20 @@ TEST(GncInt128_functions, divide)
nsmall.div (smaller, q, r);
EXPECT_EQ (-two, q);
- EXPECT_EQ (GncInt128(INT64_C(3810195028972355394)), r);
+ EXPECT_EQ (GncInt128(INT64_C(-3810195028972355394)), r);
nsmall.div (-smaller, q, r);
EXPECT_EQ (two, q);
- EXPECT_EQ (GncInt128(INT64_C(3810195028972355394)), r);
+ EXPECT_EQ (GncInt128(INT64_C(-3810195028972355394)), r);
+
+ smaller.div (small, q, r);
+ EXPECT_EQ (zero, q);
+ EXPECT_EQ (smaller, r);
+
+ smaller.div (nsmall, q, r);
+ EXPECT_EQ (zero, q);
+ EXPECT_TRUE (q.isNeg());
+ EXPECT_EQ (smaller, r);
bigger.div (bigger, q, r);
EXPECT_EQ (one, q);
@@ -477,18 +486,23 @@ TEST(GncInt128_functions, divide)
EXPECT_EQ (-two, q);
EXPECT_EQ (GncInt128(UINT64_C(3810195028972355394)), r);
- nbig.div (-big, q, r);
+ nbigger.div (-big, q, r);
EXPECT_EQ (two, q);
- EXPECT_EQ (GncInt128(UINT64_C(3810195028972355394)), r);
+ EXPECT_EQ (GncInt128(0, UINT64_C(3810195028972355394), GncInt128::neg), r);
- nbig.div (-big, q, r);
+ nbigger.div (-big, q, r);
EXPECT_EQ (two, q);
- EXPECT_EQ (GncInt128(UINT64_C(3810195028972355394)), r);
+ EXPECT_EQ (GncInt128(0, UINT64_C(3810195028972355394), GncInt128::neg), r);
big.div (bigger, q, r);
EXPECT_EQ (zero, q);
EXPECT_EQ (big, r);
+ big.div (nbigger, q, r);
+ EXPECT_EQ (zero, q);
+ EXPECT_TRUE (q.isNeg());
+ EXPECT_EQ (big, r);
+
big.div (big - 1, q, r);
EXPECT_EQ(one, q);
EXPECT_EQ(one, r);
diff --git a/libgnucash/engine/test/gtest-gnc-numeric.cpp b/libgnucash/engine/test/gtest-gnc-numeric.cpp
index f6ba3f114..680b6d82d 100644
--- a/libgnucash/engine/test/gtest-gnc-numeric.cpp
+++ b/libgnucash/engine/test/gtest-gnc-numeric.cpp
@@ -411,7 +411,7 @@ TEST(gncnumeric_functions, test_convert)
EXPECT_EQ(3465453, c.num());
EXPECT_EQ(128, c.denom());
ASSERT_NO_THROW(c = b.convert<RoundType::floor>(128));
- EXPECT_EQ(-3465452, c.num());
+ EXPECT_EQ(-3465454, c.num());
EXPECT_EQ(128, c.denom());
ASSERT_NO_THROW(c = a.convert<RoundType::ceiling>(128));
EXPECT_EQ(3465454, c.num());
Summary of changes:
libgnucash/engine/gnc-int128.cpp | 56 +++++++++++++++-------
libgnucash/engine/gnc-rational-rounding.hpp | 71 ++++++++++++++++++++++++----
libgnucash/engine/test/gtest-gnc-int128.cpp | 28 ++++++++---
libgnucash/engine/test/gtest-gnc-numeric.cpp | 2 +-
libgnucash/engine/test/utest-gnc-pricedb.c | 2 +-
5 files changed, 125 insertions(+), 34 deletions(-)
More information about the gnucash-changes
mailing list