Bug #4754
closedxs:decimal idiv operator (suspicious code)
100%
Description
I'm testing various changes in handling of Atomic.js, and hit a test failure in K-NumericIntegerDivide-1
. The test does
(xs:decimal(6) idiv xs:integer(2)) instance of xs:integer
which is supposed to return true, but it's coming out as false.
In the course of this I found code which I haven't changed and which looks wrong.
Calculate.js line 207 has
"c~c": function (lhs, rhs) {
// TODO explicit divByZero check? FOAR0002?
try {
const a = lhs.toBig(), b = rhs.toBig();
const x = a.div(b).round(0, 0);
return isSafeInteger(x) ? INT.fromNumber(x) : DEC.fromBig(x);
}
catch (e) {
divByZeroErr();
}
},
Here a
and b
are Big objects, so x
is also a Big
, but isSafeInteger
is designed to operate on a JS number.
I suspect isSafeInteger() applied to a Big is always returning false (because Math.abs() returns NaN), so we are always following the DEC.fromBig() path. With the current Atomic.js
, DEC.fromBig()
always returns an xs:decimal
, causing this test to fail. Perhaps in the released code, there was some compensating code that caused DEC.fromBig() to return an xs:integer
in this case?
Updated by Michael Kay about 4 years ago
I've changed the line
return isSafeInteger(x) ? INT.fromNumber(x) : DEC.fromBig(x);
to
return INT.fromBig(x)
and the test is now passing. But I can't see how the test ever worked in the released code. Perhaps we were inferring the type of the result statically?
Updated by Michael Kay about 4 years ago
- Status changed from New to Resolved
- Applies to JS Branch 2.0 added
- Fix Committed on JS Branch 2.0 added
Marking as resolved, though it remains a puzzle how the code was passing the tests.
Updated by Community Admin almost 4 years ago
- Applies to JS Branch 2 added
- Applies to JS Branch deleted (
2.0)
Updated by Community Admin almost 4 years ago
- Fix Committed on JS Branch 2 added
- Fix Committed on JS Branch deleted (
2.0)
Updated by Debbie Lockett over 3 years ago
- Status changed from Resolved to Closed
- % Done changed from 0 to 100
- Fixed in JS Release set to Saxon-JS 2.1
Bug fix applied in the Saxon-JS 2.1 maintenance release.
Please register to edit this issue
Also available in: Atom PDF Tracking page