Project

Profile

Help

Bug #4754

closed

xs:decimal idiv operator (suspicious code)

Added by Michael Kay about 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Low
Assignee:
Category:
-
Sprint/Milestone:
-
Start date:
2020-09-26
Due date:
% Done:

100%

Estimated time:
Applies to JS Branch:
2
Fix Committed on JS Branch:
2
Fixed in JS Release:
SEF Generated with:
Platforms:
Company:
-
Contact person:
-
Additional contact persons:
-

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?

Actions #1

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?

Actions #2

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.

Actions #3

Updated by Community Admin almost 4 years ago

  • Applies to JS Branch 2 added
  • Applies to JS Branch deleted (2.0)
Actions #4

Updated by Community Admin almost 4 years ago

  • Fix Committed on JS Branch 2 added
  • Fix Committed on JS Branch deleted (2.0)
Actions #5

Updated by Debbie Lockett almost 4 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