-
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: support int(bool) conversions in Go.1.5 (language change) #9367
Comments
I'm no such authority to say, but I think things like this are best discussed on the golang-nuts mailing list first. Most people don't review each Go issue, in contrast most do read the mailing list and could provide useful insight before such a change is made. |
And on the list you'll be asked what problem this solves and how it solves it better than the existing method. And then they'll say that taking something completely unambiguous and impossible to misunderstand and adding the ability to do it in a redundant way which could be confusing and will cause bugs due to unintentional casting is not beneficial for the language. When it comes to Go, think minimalism. If you have a use-case where your proposal is demonstrably better than the status quo, please provide it in the list for discussion, because without it the conversation will be a non-starter. |
I hate to say, but this has been proposed and rejected. We should teach gc to generate better code, instead of adding features to And yes, this should be discussed on golang-nuts first. |
i := 0
if b {
i = 1
} and create the respective single instruction (or conditional bit set, depending on what happens with i, etc.)
func bool2int(b bool) int {
if b {
return 1
}
return 0
} where needed.
|
On 17 December 2014 at 14:38, Robert Griesemer notifications@github.com
The question is: would you prefer to read just this "int(x.f > 0)", or four |
i think bool2int(x.f > 0) is just as readable. |
In my experience the most beneficial of this feature would be as follows: var check bool = something
// ...
value := requiredValue + int(check) * optionalValue IE: only add This would normally be expressed in go as: var check bool = something
// ...
value := requiredValue
if check {
value += optionalValue
} In this situation I would argue that the later is more clear about what is happening. Also, the former would most likely require a multiplication instead the conditional branch. Only in the case where If it is the case that value := required + int(check) would produce better code than value := required
if check {
value++
} then the optimizer should be improved. IMHO the latter is still clearer about what is happening, albeit not as much as the original example. |
It's been a couple of decades since a paid attention to CPU architecture, but at that time conditional branching was significantly more expensive than multiplication. Have things changed that much? |
I think this is a dup of #6011. |
@serussell No, branching is still more expensive. Modern CPUs have to guess which way a branch is going to go. If they guess wrong, they will realize much later that they have to go back and redo a lot of prior computation. That is very slow: something like 20 instructions wasted on a modern Intel CPU. See https://en.wikipedia.org/wiki/Branch_predictor For speed, then, it would be better for the compilers to avoid conditional jumps for bool-to-int conversion. |
@BThomson, hmmm. Then I don't understand what @cookieo9 was suggesting would be better. It's generally good (for performance) to avoid branching, except that branching code is often more clear. In any case, I suspect it's moot because I can't think of an implementation of int() that wouldn't involve a branch somewhere under the hood. int() just hides the branch, and it is less clear... although the brevity is admirable. |
Removed Go 1.5. This is either a dup of #6011 or should go through the proposal process (write a doc) if this is actually a language change proposal. |
I hope they didn't/won't implement this. Implicit conversion is an evil whatever pratical reasons are listed. |
@DenisCheremisov this proposal is for an explicit conversion. |
@serussell, compiling int(bool) won't require branches on most
modern architecture because they provide conditional set and/or
conditional move instruction.
(I'm not advocating int(bool) though, just clarify that it can be
implemented without conditional branches.)
|
The thread on golang-nuts raises a couple of key issues with this proposal:
For these reasons I am going to decline this proposal. |
Given the adjacent dups in the above, seems like the measurement system runs every package twice (perhaps p and p_test?). The boolconv.txt file shrinks by 35% upon sorting, so scale my above figures by that amount too. |
Findings in std:
|
@adonovan should this issue be reopened? It looks like this pattern did not get detected: https://cs.opensource.google/go/go/+/refs/tags/go1.21.5:src/sync/atomic/type.go;l=31 (which is then called 4 times in the lines above it) |
Russ suggested I create a new issue summarizing all previous discoveries. I'll do that presently. [Done: #64825]
Yeah, the pattern matcher for |
I propose the following language change for Go 1.5:
An explicit conversion int(b) where b has a boolean type would become legal, and would yield 1 if b is true, 0 otherwise. Similar conversions would be legal for all numeric types.
Rationale: currently this function cannot be computed in a single expression, despite it being often useful, hard to misuse, efficiently implemented by modern ALUs, present in nearly all other languages, and syntactically natural in Go.
While I appreciate the value of avoiding implicit int/bool conversions, forbidding explicit ones seems slightly obtuse, and as a result of its omission, one must write four extra lines of code:
var i int
if b {
i = 1
}
... i ...
which the gc compiler does not optimize this into the obvious single instruction.
The reverse operation bool(i), is not essential since i != 0 has the same effect and can be used in an expression context, but could be added for symmetry. (I have no strong opinion.)
The text was updated successfully, but these errors were encountered: