Go: proposal: Go 2: integer comparison casting

[Hucaru]

When comparing two integers of differing type the following if statement is required:

var i int32 = 1
var j int16 = 2
if int32(j) > i {
}

My proposal is to implicitly cast the 'smaller' integer (j) to the 'larger' integer (i) type. This would not be allowed between signed and unsigned types. This allows the above to be re-written as:

var i int32 = 1
var j int16 = 2
if j > i {
}

I feel the above has the following advantages over the former:

1. If integer types change then the statement still holds, whereas the former might not
2. Helps removes human error, for example the following if statement evaluates to true:

var i int32 = 2147483647
var j int16 = 2
if j > int16(i) {
}

1. Compatible with current Go and if a user wants to be explicit they still can.

[ianlancetaylor]

This is a FAQ: https://golang.org/doc/faq#conversions .

Experience teaches us that explicit is better than implicit.

[Hucaru]

I think the points it makes are valid and I wouldn't like to see implicit casting for assignments but for comparison I think it's better as long as you keep it between the same numerical representations - can only implicitly cast when comparing a uint with a uint; an int with an int.

To allow bugs (like in the 3rd example) to arise when we could mitigate/prevent them because of a rule stating never allow any numerical casting to be implicit seems rather unfortunate to me.

[bradfitz]

One argument in favor of this is that it would likely result in more correct code in the wild.

With the current situation today, I often see people doing a comparison between an int and an int64 and incorrectly truncating the int64 down to an int, rather than the int up to an int64. It works out fine in practice (on 64-bit arches) but is a bug sitting and waiting to bite 32-bit users.

If we made this language change we'd do the right thing automatically, and the code would look prettier. (fewer conversions).

Edit: durr, this was exactly what you said about "human error" above in bullet (2).

[griesemer]

cc: @robpike

[magical]

Rather than define it in terms of casting to a larger type, I would simply say that integer comparisons are done according to the mathematical value of the two integers, as if they were both infinite-precision signed values. Then you could drop the restriction about not comparing unsigned and signed types.

That said, this proposal could lead to subtle bugs involving overflowing counters.
For example, this loop

var limit int64
for i := 0; i < limit; i++ {
  // do stuff
}

might never terminate if it was run on a system where int is 32 bits and limit > (1<<32)

[bcmills]

this proposal could lead to subtle bugs involving overflowing counters

Note that those bugs are already possible (albeit with fewer non-terminating boundary conditions) today:
https://play.golang.org/p/2a2qTx5xu_a

The only way I can see to _reliably_ avoid subtle overflow bugs is to disallow implicit overflow (#30209 or related proposals).

[robpike]

Brad makes a good point. It's an interesting idea. The spec would get trickier, but maybe not too much. On the fence but interested.

[quasilyte]

@Hucaru I feel like this task can be solved with static analysis. If someone does this kind of a cast inside comparison, linters can warn about it.

Introducing implicit casting into a language where external static analyzers will be enough seems sub-optimal to me.

(The case pointer out by @bcmills is much harder to determine in general, but whole-program analyzers like PVS for C++ do that with quite good results.)

[bradfitz]

Paging @dominikh for static checkers. (Maybe one already exists?)

[cristaloleg]

@bradfitz looks like it already exists https://github.com/go-critic/go-critic/pull/878 😉

[griesemer]

Today, the rules for comparison operators are slightly different from other operators, but for numeric types the rules end up effectively being the same: For comparisons the spec says that one of the operands must be assignable to the type of the other. For integer types this means that the operands have to be effectively the same (excluding untyped operands and shifts for which we have special rules). This is the rule we have for other binary operators: both operands must have the same type.

Since comparisons have special rules already, we can perhaps (but see below) tweak those rules w/o affecting anything else.

That said, I think it's a slippery slope: Once we allow say i16 < i32 (with i16, i32 operands of type int16 and int32), it's quite natural to ask why i16 + i32 should not be permitted. And by extension, why i32 = i16 should not be permitted as well. In fact, if we allow such assignments, the rules for comparison don't have to be changed at all, since changing the assignment rule would change the comparison rule (which is based on assignability) and it would just fall out naturally.

So I think another way of asking this question is: Should (by way of example) i32 = i16 be permitted (and we let the rest follow)?

Perhaps that's a step too far and only comparison rules should be relaxed as proposed. But even that is not so simple. For instance, the code:

var x interface{} = byte(0)
y := 0
result := x == y

is valid and result is false because the dynamic type of x (byte) and the type of y (int) don't match. We couldn't change this behavior without loosing backward-compatibility, but arguably this comparison would then work differently if we allowed byte(0) == int(0) to be ok.

One final observation: Let's assume i32 = i16 were permitted, this would have some consequences on our current contracts (generic) proposal, specifically type inference for parameterized functions: Currently, the proposed inference mechanism is extremely straight-forward. If "widening assignments" are permitted, one would also expect them to be used in type inference which will make the type inference algorithm less straight-forward (an inferred parameter type T is going to be the "smallest (numeric) type large enough" to cover all arguments assigned to parameters of type T).

Just to be clear, I am not for or against this proposal at this point. Just trying to explore the lay of the land.

[dominikh]

@bradfitz looks like it already exists go-critic/go-critic#878 wink

You wake up late once and go-critic steals your thunder :-)

[Hucaru]

@griesemer Excellent points. Your example demonstrates perfectly why this is a potentially precarious language change. @quasilyte raises a very good point that the majority of the benefits that would arise from implementing the proposal could be achieved with static analysis. However, I would like to prevent the situation where bug prevention/mitigation is offloaded to external tools (that could start to make them feel mandatory) when it could be implemented at a language level - I appreciate that this viewpoint could very well be at odds with the direction the Go team wishes to go down.

[bcmills]

So I think another way of asking this question is: Should (by way of example) i32 = i16 be permitted (and we let the rest follow)?

My 2¢: clearly it should be. The destination can exactly represent all of the values of the source without rounding. The requirement for an explicit conversion in such an assignment adds no new information, and prevents no errors.

it's quite natural to ask why i16 + i32 should not be permitted

That one is also (IMO) clear: it should not be permitted unless we also remove silent overflow, because the result of i16 + i32 may today be _a different number_ from i16 + i16 with the exact same numerical values as operands.

[ianlancetaylor]

We can't permit i32 = i16 unless we change the rules for defined types or we decide that int32 and int16 are not defined types.

[alanfo]

To my mind, the arguments in the FAQ about not allowing implicit numeric conversions remain clear and convincing.

Although this proposal (and the extended idea of allowing implicit widening conversions for assignments as well) would undoubtedly be convenient and might prevent the odd bug, it just doesn't seem worthwhile to me to jeopardize the simplicity and consistency of the present position.

Also, if it were allowed for integers, then no doubt people would ask why it wasn't possible for floats as well when one has a float64 and float32 either side of a comparison (or assignment) operator. So the slope may become even slippier.

[DmitriyMV]

Given the points raised by @quasilyte and @griesemer I think it would be wise to have this in go vet. Since there is a lot of working code that actually do int(int64()) maybe it would be a good idea to hide this behind flag like, like there is a flag to check the variable shadowing?

[ianlancetaylor]

There seem to be too many complexities to make this work generally.

It seems potentially confusing to permit mixing-and-matching different defined types. Right now Go does not permit comparing a value of type Temperature to a value of type Money. We don't want to start permitting that just because Temperature is defined as int16 where Money is defined as int64. Since int16 and int64 are themselves defined types, there is no obvious way to permit int16 < int64 but not Temperature < Money. Unless we start treating the builtin types as having special properties, which we currently do not (except that they are predeclared).

There is also the interface issue mentioned above. We currently permit comparing interface values using == and !=, and that tests whether the types of the values stored in the interfaces are the same. If we make this change then logically we should extend that comparison to work for comparing values of integer types stored in interface variables, but that also seems complex.

The core idea here can likely be captured in a vet check and apparently go-critic already has that check.

For these reasons this is a likely decline. Leaving open for four weeks for final comments.

-- for @golang/proposal-review

[ianlancetaylor]

No final comments. Closing.