First time being bitten by Crystal autocasting

Well I don’t see which criteria you would apply for such an annotation. What would be a reasonable cause for excluding one method def’s parameter from autocasting?

Unnecessarily too verbose/complex.

It could avoid such bad use, due to “a bit of lack of the attention” of the user of the method, for example:

Functions that directly deal with floating-point representations should never autocast variables, because there are no floats to begin with. IMHO it makes no sense to talk about this:

x = 123_i32
Math.frexp(x)

It compiles because .frexp has a catch-all overload that calls to_f on the argument, which is deprecated by this pull request. Let’s say that this overload is eventually removed, but variable autocasting stays. Then some integers would still compile, including the above one, but not e.g. Int64 variables. This awkward result is entirely due to the variable autocasting mechanism, and has nothing to do with floating-point representations at all. (In fact I’d argue that these functions should disable literal autocasting too, so the above annotation might be useful.)

Then in the opposite direction, when autocasting is desired, both Int8 and Int32 variables autocast to Float64 variables, but Int8 | Int32 variables don’t. This cannot happen with integer literals because all literals have a non-union type.

So overall I think variable autocasting introduces more problems than it solves ones.

1 Like

I wonder if we can remove it at this point. Given that it’s already a feature, could we remove it?

I think we should probably remove it. After all literal autocasting (which I guess will stay and is fine) is probably more common.

I wonder if variable autocasting between integer types would be fine, though…

At this point: No, I don’t think we can remove it. We introduced it as a proper feature and have to maintain backwards compatibility.

Should we decide to abandon variable autocasting, we can deprecate it but have to keep it working until 2.0.

Before doing that however, we should carefully investigate the effects and implications of autocasting in order to gain a proper understanding.
And then we can develop a strategy for how it could be improved or replaced.

7 Likes

I could actually use more auto-casting.

It would be nice if I didn’t have to use #to_u16 in this function.

def xy_to_square(x : UInt8, y : UInt8, board_width : UInt8 = BoardWidth::MEDIUM) : UInt16
  (x.to_u16 % board_width) + (y.to_u16 * board_width)
end

or #to_u8 in this one

def square_to_xy(square : UInt16, board_width : UInt8 = BoardWidth::MEDIUM) : Tuple(UInt8, UInt8)
  {(square % board_width).to_u8, (square // board_width).to_u8}
end

The suggestions to use Type! or annotations to make a parameter strict (define a failure point) looks quite nice and I could see that being useful in some rare cases.

1 Like

Note that autocasting here will convert the returning UInt8 from (x % board_width) + (y * board_width) to UInt16, therefore changing the meaning: y * board_width might overflow, while y.to_u16 * board_width won’t. And a double-autocast in which the UInt16 “bubbles up” to the operation y * board_width is way too dangerous.

Interesting,
So what kind of issues would having auto-casting bubble up until it hits a type restriction cause?
Is it that it would be hard to reason about?

So with this function, I don’t actually care what types are being used internally in the function as long as it only accepts UInt8 and returns UInt16.

Well, the rule of thumb is that the more magic in the guessing/autocasting, the harder to know why a certain expression has a specific type. Making it work in a way that it keeps being intuitive and helpful is hard.

3 Likes