-
Notifications
You must be signed in to change notification settings - Fork 17.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
proposal: Go 2: integer comparison casting #35010
Comments
This is a FAQ: https://golang.org/doc/faq#conversions . Experience teaches us that explicit is better than implicit. |
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. |
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 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). |
cc: @robpike |
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. var limit int64
for i := 0; i < limit; i++ {
// do stuff
} might never terminate if it was run on a system where |
Note that those bugs are already possible (albeit with fewer non-terminating boundary conditions) today: The only way I can see to reliably avoid subtle overflow bugs is to disallow implicit overflow (#30209 or related proposals). |
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. |
Checks when numeric types are compared with truncated conversion on the either side. Problematic code example: func f(x int32, y int16) bool { return int16(x) < y } Fixes code example: func f(x int32, int16) bool { return x < int32(y) } See golang/go#35010 Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>
Checks when numeric types are compared with truncated conversion on the either side. Problematic code example: ``` func f(x int32, y int16) bool { return int16(x) < y } ``` Fixed code example: ``` func f(x int32, int16) bool { return x < int32(y) } ``` See golang/go#35010 Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>
@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.) |
Checks when numeric types are compared with truncated conversion on the either side. Problematic code example: ``` func f(x int32, y int16) bool { return int16(x) < y } ``` Fixed code example: ``` func f(x int32, int16) bool { return x < int32(y) } ``` See golang/go#35010 Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>
Paging @dominikh for static checkers. (Maybe one already exists?) |
@bradfitz looks like it already exists go-critic/go-critic#878 😉 |
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 So I think another way of asking this question is: Should (by way of example) 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 One final observation: Let's assume Just to be clear, I am not for or against this proposal at this point. Just trying to explore the lay of the land. |
You wake up late once and go-critic steals your thunder :-) |
@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. |
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.
That one is also (IMO) clear: it should not be permitted unless we also remove silent overflow, because the result of |
We can't permit |
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. |
Given the points raised by @quasilyte and @griesemer I think it would be wise to have this in |
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 There is also the interface issue mentioned above. We currently permit comparing interface values using 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 |
No final comments. Closing. |
When comparing two integers of differing type the following if statement is required:
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:
I feel the above has the following advantages over the former:
The text was updated successfully, but these errors were encountered: