Fix denominator of roundto result

Multiprecision rational created from a double value may have large power
of 2 denominator since fractional decimal numbers can not be represented
as binary floating point numbers. It leads to failed assertion when
result is compared to a value converted directly from strings.
Use integer multiprecision arithmetics to round numbers to ensure
proper denominator. Inspired by python gmpy2 package
<https://github.com/aleaxit/gmpy/blob/3e4564ae9d/src/gmpy2_mpq_misc.c#L315>

The change makes `roundto` symmetric for positive/negative arguments.

Halves are rounded to nearest even. Rounded away from zero are discussed
in #1663 and it may be achieved with minimal modification.

- See #2329
- Closes #1983

5 files changed, 145 insertions, 3 deletions

diff --git a/doc/ledger3.texi b/doc/ledger3.texi
index e9cadefb..02680caa 100644
--- a/doc/ledger3.texi
+++ b/doc/ledger3.texi
@@ -8660,7 +8660,7 @@ $ ledger -f expr.dat --format "%(account) %(roundto(amount, 1))\n" reg
 @end smallexample
 @smallexample @c output:B4DFB9F
 Assets:Cash ¤ -123,40
-Expenses:Office Supplies ¤ 123,50
+Expenses:Office Supplies ¤ 123,40
 @end smallexample
 @end defun
 
diff --git a/src/amount.cc b/src/amount.cc
index 484c3099..604519df 100644
--- a/src/amount.cc
+++ b/src/amount.cc
@@ -684,8 +684,45 @@ void amount_t::in_place_roundto(int places)
 {
   if (! quantity)
     throw_(amount_error, _("Cannot round an uninitialized amount"));
-    double x=ceil(mpq_get_d(MP(quantity))*pow(10, places) - 0.49999999) / pow(10, places);
-    mpq_set_d(MP(quantity), x);
+
+  // <https://github.com/ledger/ledger/issues/2362>
+  // _dup() call (see in_place_ceiling and in_place_floor)
+  // leads to 2 failures in the balance/testRound test,
+  // however in its absence this method affects copies of amount_t instances.
+  // Remove expected_failures from amount/testRound
+  // and update balance/testRound
+  // when uncommenting the following line.
+  // _dup();
+
+  mpz_t& scale(temp);
+  if (places)
+    mpz_ui_pow_ui(scale, 10, labs(places));
+
+  if (places > 0) {
+    mpz_mul(mpq_numref(MP(quantity)), mpq_numref(MP(quantity)), scale);
+  } else if (places < 0) {
+    mpz_mul(mpq_denref(MP(quantity)), mpq_denref(MP(quantity)), scale);
+  }
+
+  auto whole(mpq_numref(tempq));
+  auto reminder(mpq_denref(tempq));
+  mpz_fdiv_qr(whole, reminder, mpq_numref(MP(quantity)), mpq_denref(MP(quantity)));
+  mpz_mul_2exp(reminder, reminder, 1);
+  const int rem_denom_cmp = mpz_cmp(reminder, mpq_denref(MP(quantity)));
+  if (rem_denom_cmp > 0
+      || (rem_denom_cmp == 0 && mpz_odd_p(whole)))
+    mpz_add_ui(whole, whole, 1);
+
+  if (places > 0) {
+    mpq_set_num(MP(quantity), whole);
+    mpq_set_den(MP(quantity), scale);
+    mpq_canonicalize(MP(quantity));
+  } else if (places == 0)
+    mpq_set_z(MP(quantity), whole);
+  else {
+    mpq_set_ui(MP(quantity), 0, 1);
+    mpz_mul(mpq_numref(MP(quantity)), whole, scale);
+  }
 }
 
 void amount_t::in_place_unround()
diff --git a/test/regress/2329.test b/test/regress/2329.test
new file mode 100644
index 00000000..643ef0d1
--- /dev/null
+++ b/test/regress/2329.test
@@ -0,0 +1,83 @@
+
+; roundto() result should not have large power 2 denominator.
+; Pairs to test positive/negative value symmetry.
+assert roundto(1.1, 1) == 1.1
+assert roundto(-1.1, 1) == -1.1
+
+; positive places
+assert roundto(1.13, 1) == 1.1
+assert roundto(1,17, 1) == 1.2
+assert roundto(1.10, 1) == 1.1
+assert roundto(-1.13, 1) == -1.1
+assert roundto(-1,17, 1) == -1.2
+assert roundto(-1.10, 1) == -1.1
+
+; zero places
+assert roundto(0.12, 0) == 0
+assert roundto(123.89, 0) == 124
+assert roundto(-0.12, 0) == 0
+assert roundto(-123.89, 0) == -124
+
+; negative places
+assert roundto(123.45, -1) == 120
+assert roundto(98765, -3) == 99000
+assert roundto(24500, -2) == 24500
+assert roundto(-123.45, -1) == -120
+assert roundto(-98765, -3) == -99000
+assert roundto(-24500, -2) == -24500
+
+; round to even
+assert roundto(0.5, 1) + roundto(1.5, 1) + roundto(2.5, 1) + roundto(3.5, 1) + roundto(4.5, 1) == roundto(0.5 + 1.5 + 2.5 + 3.5 + 4.5, 1)
+assert roundto(-0.5, 1) + roundto(-1.5, 1) + roundto(-2.5, 1) + roundto(-3.5, 1) + roundto(-4.5, 1) == roundto(-0.5 - 1.5 - 2.5 - 3.5 - 4.5, 1)
+
+assert roundto(0.12345, 4) == 0.1234
+assert roundto(0.9875, 3) == 0.988
+assert roundto(-0.12345, 4) == -0.1234
+assert roundto(-0.9875, 3) == -0.988
+assert roundto(1234.5, 0) == 1234
+assert roundto(367.5, 0) == 368
+assert roundto(-1234.5, 0) == -1234
+assert roundto(-367.5, 0) == -368
+assert roundto(29500.00, -3) == 30000
+assert roundto(72250, -2) == 72200
+assert roundto(-29500.00, -3) == -30000
+assert roundto(-72250, -2) == -72200
+
+; Assertion in transactions, no error:
+comment
+
+While parsing posting:
+  Account:NOK  0  = -700.67 NOK
+                    ^^^^^^^^^^^
+Error: Balance assertion off by -0.00 NOK (expected to see -700.67 NOK)
+end comment
+
+2024-02-01 Regular precision
+    Account:NOK   (-roundto(12.34 NOK * 56.78, 2))
+    Expenses:NOK  700.67 NOK
+2024-02-02 Assertion
+    Account:NOK  0  = -700.67 NOK
+
+; Transaction balance, no error:
+comment
+
+> 2024-02-01 Excessive precision
+>     Account:USD   (-roundto($12.34 * 56.78, 2))
+>     Expenses:USD  $700.67
+Unbalanced remainder is:
+    $0.0000000000000
+Amount to balance against:
+  $700.6700000000000
+Error: Transaction does not balance
+end comment
+
+commodity $
+    format $1000.0000000000000
+2024-02-01 Excessive precision
+    Account:USD   (-roundto($12.34 * 56.78, 2))
+    Expenses:USD  $700.67
+
+test bal --flat --no-total Account
+         -700.67 NOK  Account:NOK
+ $-700.6700000000000  Account:USD
+end test
diff --git a/test/unit/t_amount.cc b/test/unit/t_amount.cc
index b82de510..28bc56e9 100644
--- a/test/unit/t_amount.cc
+++ b/test/unit/t_amount.cc
@@ -1163,6 +1163,18 @@ BOOST_AUTO_TEST_CASE(testCommodityCeiling)
   BOOST_CHECK(x2.valid());
 }
 
+BOOST_AUTO_TEST_CASE(testRound, * boost::unit_test::expected_failures(1))
+{
+  amount_t a1("$ 123.123");
+  amount_t a2(a1);
+  a2.in_place_roundto(2);
+  // Fails due to missing _dup() call in amount_t::in_place_roundto(int).
+  // <https://github.com/ledger/ledger/issues/2362>
+  BOOST_CHECK_EQUAL(amount_t("$ 123.123"), a1);
+  // Should it be "$ 123.12"?
+  BOOST_CHECK_EQUAL(amount_t("$ 123.120"), a2);
+}
+
 #ifndef NOT_FOR_PYTHON
 #if 0
 BOOST_AUTO_TEST_CASE(testReduction)
diff --git a/test/unit/t_balance.cc b/test/unit/t_balance.cc
index b503d02a..066f066e 100644
--- a/test/unit/t_balance.cc
+++ b/test/unit/t_balance.cc
@@ -451,6 +451,9 @@ BOOST_AUTO_TEST_CASE(testRound)
   b2 += a5;
   b2 += a6;
 
+  // <https://github.com/ledger/ledger/issues/2362>
+  // This block modifies b1 and b2, likely it is a bug,
+  // but otherwise some assertions fails.
   a1.in_place_roundto(2);
   a2.in_place_roundto(2);
   a3.in_place_roundto(2);
@@ -465,11 +468,18 @@ BOOST_AUTO_TEST_CASE(testRound)
   b4 += a5;
   b4 += a6;
 
+  // After fix of #2362 rounded() and in_place_round()
+  // likely should be replaced with roundto(2) and
+  // in_place_roundto(2).
+  // It looks like rounded() and in_place_round()
+  // need some other tests, perhaps with unround().
   BOOST_CHECK_EQUAL(b0.rounded(), b0);
   BOOST_CHECK_EQUAL(b2.rounded(), b4);
+  // Relies on b1 modified by amount_t::in_place_roundto(int).
   BOOST_CHECK_EQUAL(b1.rounded(), b4);
 
   b1.in_place_round();
+  // Relies on b1 modified by amount_t::in_place_roundto(int).
   BOOST_CHECK_EQUAL(b1, b3);
 
   BOOST_CHECK(b0.valid());