Multiplication of Int64 and Int32 may result in Int32 (risking overflows at runtime)

[philipp-classen]

Discussion

I had been debugging a problem with OverflowError in my code. Maybe the behavior is expected, but to me it was at surprising.

When multiplying Int64 and Int32, my assumption was that either it should implicitly promote the Int32 to Int64 or fail with an compile error. It turns out that the actual behavior - both compiler and interpreter - is to do a Int32 multiplication:

icr:1> x = 5_000_000_000
=> 5000000000
icr:2> typeof(x)
=> Int64
icr:3> typeof(1 * x)
=> Int32
icr:4> typeof(1_i64 * x)
=> Int64
icr:5> 1 * x
Unhandled exception: Arithmetic overflow (OverflowError)
 from :1:1 in '<top-level>'

My assumption would have been that here, especially because "1" is not typed, should not result in a Int32 multiplication.

Here is another example of two programs. In both cases, I would have expected that they print "5000000000", but it is not the case:

Example 1: outputs 5000000000

x = 5000000000
puts x

Example 2: fails with "Arithmetic overflow (OverflowError)"

x = 1 * 5000000000
puts x

Also, with variables instead of literals. For instance, this here will overflow:

one = 1
x = 5000000000
puts(one * x)

But this program will print "5000000000":

one = 1_i64
x = 5000000000
puts(one * x)

As said before, I was under the assumption that multiplying incompatible types like Int32 and Int64 should be rejected by the compiler, or that it should do the safe conversion implicitly (here Int32 to Int64). Otherwise, I feel it leads to subtle runtime errors.

Also, the operation is not symmetrical:

icr:3> typeof(5000000000 * 1)
 => Int64
icr:4> typeof(1 * 5000000000)
 => Int32

Same with the compiler:

p! typeof(5000000000 * 1)
p! typeof(1 * 5000000000)

results in this output:

typeof(5000000000_i64 * 1) # => Int64
typeof(1 * 5000000000_i64) # => Int32

Here I would also expected to see either Int64 in both cases, or that the compiler rejects it.

[straight-shoota]

Yeah, this behaviour is expected and (currently) intended. I agree that it can be confusing, though.

The underlying idea is that performing an operation on a variable should not change the type of the variable. This is particularly relevant for the assignment operators, such as a *= b (which is equivalent to a = a * b). It would be weird if the type of a was changed by this operation.
So the math operators ensure that typeof(a * b) is always equal to typeof(a).

Knowing this practical reason, I believe the behaviour is quite comprehensible. But it's not without alternatives.
It's just that this solution was chosen for Crystal and we have to live with it.

I think this could potentially be changed in the future. I would support that because it's an odd rule and confusing.
But I'm not sure how much effort it would be. However, I'm confident that we can find an alternative that is more approachable and less surprising, and probably won't break too much of existing code. It would still be a breaking change, though. So it's not an immediate concern.

[konovod]

Maybe a *= b should be made equivalent to a = a.class.new(a*b)? This would work with unions but maybe has runtime overhead (it can be optimized to old behavior when typeof(a*b) == typeof(a))

[straight-shoota]

Yeah, that could be an option.

I'd like to look into requiring identical number types for math operations. Thus you would need to be explicit about type casting when the types don't match. Auto-casting should cover some basic use cases, so it wouldn't be too much noise.

[ysbaddaden]

It goes further than just the assignment operators. See #8111 and #8872.

IMO integers are broken in Crystal, or at least not as simple as they could have been; either you live with the default Int32 everywhere or you type everything when you need explicit sizes, down to each literal value 🤷

I wish this had been fixed before 1.0 but we instead kept the rule that the left-most type is the result type, so Int64 * Int32 returns an Int64 while Int32 * Int64 returns an Int32, which means that you should always write a * 1 instead of 1 * a so you can keep the type of a... unless you want the result to be an Int32...