For a forthcoming article in xDev I had to use the Mod function and came across an interesting problem:
From the Xojo documentation:
result = number1 Mod number2
The Mod operator divides number1 by number2 and returns the remainder as result. If either number1 or number2 is a floating-point type, it is first coerced to Int32. If either number1 or number2 is an Int64, then both values are promoted to Int64.
What happens with result = doubleValue mod intValue?
result and intValue are Integer, and should be a 64 bit integer on a 64 bit system. Testing revealed that the coercing to 32bit Integer rule takes precedence over promoting to 64bit Integer on the right side.
But why would the double be coerced into a 32bit integer on 64 bit systems? That just screams overflow error! Have Xojo forgotten to update this when they moved to 64 bit?
Basically you are running into 32 bit limitations on a 64 bit system.
But it is MUCH worse than that: the same seems to apply to a number of other Math functions - it seems the underlying structure never got updated to 64 bit, and therefore with larger numbers you run into 32 bit limitations because of a systematic problems in Xojo.
The biggest problem is that your code might seem to work just fine - until it hits a not very large number that Int64 should easily handle and instead it fails. Which it would not if Xojo used Int64 on 64bit systems instead of coercing to Int32…
And now Xojo don’t seem to have a compiler guy anymore to fix it?
What kind of programming language has a systematic Maths error like this?
Marcus, why do you keep saying this about our team? We’ve done several compiler updates over the last few years since Joe Ranieri left. In the future, please check your sources before making statements like that.
Int64 works. It’s just that double is truncated to Int32 for doing mod (and others).
Travis, Greg or William may be perfectly able to do a fix here buy checking where the Double to Int promotion is in the compiler’s frontend for creating the code and change it to use Int64 instead of Int32.
Whether your answer is correct depends a LOT on what you define as “compiler” (eg whether you count fixes in the pre-processing steps as “compiler fixes” or not).
But I’m glad to hear that Xojo can do compiler fixes so it should be easy to fix this …
The basic problem is that under a 32 bit system a Double can represent a MUCH larger range of integers accurately than an Integer can. So a LOT of code used Double instead of Integer to avoid integer overflows:
The maximum value of “whole numbers” that can be accurately represented:
On 32 bit systems it made some sense to cast to a 32 bit integer even though I would argue it would have been better (and more future proof) to cast to a 64 bit integer (and yes, you CAN use 64 bit integers on 32 bit systems though it will probably slow down the calculation).
However on 64 bit systems it is simply not acceptable to cast a Double to 32 bit Integers.
Of course. I’m not a theoretical physicist …
Something that causes int32 overflow errors where none should be is a bug, not a feature.
Basic functionality should be rock solid. Especially where computers and Maths are concerned.
Xojo’s Maths implementation leaves a LOT to be desired.
Btw the two Boeings that fell out of the air because their engines stalled? That was due to a 32bit Integer overflow error.
P.S. d.TotalSeconds is a double with current value 3,686,809,196, so performing a mathematical operation with it might cause it to be cast to a 32bit Integer and consequently a 32 bit integer overflow without users being aware of it.
I tried your code. At first I thought I could not repeat the problem but then I realized that I needed to add another column to the Listbox and I wonder if the reviewers had the same problem - it is better to submit an example project!
Now your code SHOULD better be
Dim date1 As xojo.core.date = xojo.core.date.now
Dim date2 As xojo.core.date = xojo.core.date.now
Dim date3 As xojo.core.date = xojo.core.date.now
Dim interval2 As New Xojo.Core.DateInterval
Dim interval3 As New Xojo.Core.DateInterval
interval2.NanoSeconds = 2147483647
interval3.NanoSeconds = 2147483648
Listbox1.AddRow("Date Now", date1.ToText)
Listbox1.AddRow("Date 1", date2.ToText)
Listbox1.AddRow("Date 2", date3.ToText)
date3 = date2 + interval2
Listbox1.AddRow("Add 2147483647 nanosec to date 2", date3.ToText)
date3 = date2 + interval3
Listbox1.AddRow("Add 2147483648 nanosec to date 2", date3.ToText)
and I do see it: adding interval3 leads to a subtraction:
So YES, that seems to be the same “cast to Int32 overflow error”
The case is old, the forum post is newer (I didn’t know that my case was closed for several months).
You can use seconds instead of nanoseconds with DateTime to see the big difference:
Var d1 as DateTime = DateTime.Now
Var d2 as DateTime = DateTime.Now
Var d3 as DateTime = DateTime.Now
Var interval2 as new DateInterval
interval2.Seconds = 2147483647
Var interval3 as new DateInterval
interval3.Seconds = 2147483648
d2 = d2 + interval2
d3 = d3 + interval3
//d1.SQLDateTime = 2020-10-28 16:53:53
//d2.SQLDateTime = 2088-11-15 19:08:00
//d3.SQLDateTime = 1952-10-10 12:39:45
Interesting that the IDE uses different green for the seconds:
As you can see TotalSeconds / SecondsFrom1970 is a double, and adding an integer will cause it to be cast to an Int32. The sum of the two will be then done as two Int32 so interval 3 is added as a negative value.
Int64 and uint64 are present in 32bit systems, just the processing is a bit slower emulating instructions, so it should not happen anywhere. As for when to promote values to larger types and demote to smaller, it needs a better smarter expression evaluator with better type evaluation of the expression and finally promoting or demoting the results to fit the destination (when demoting, i.e. casting to a inferior type losing some information, a warning must be raised unless you have an explicit cast to say you did it in a purpose, or compiler directive to say it; a pragma in Xojo terms). The Xojo expression evaluation engine needs a full review, new types were introduced and it’s not able to work correctly with them. Also, bitwise operations should be written direct in such expressions using the CURRENT bitwise operators and not having a module with methods to emulate them in a slower fashion just because the values are in a 64bit range.