proposal: support int(bool) conversions in Go.1.5 (language change)

alandonovan commented 9 years ago

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.)

slimsag commented 9 years ago

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.

ShawnMilo commented 9 years ago

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.

minux commented 9 years ago

I hate to say, but this has been proposed and rejected.

We should teach gc to generate better code, instead of adding features to the language.

And yes, this should be discussed on golang-nuts first.

griesemer commented 9 years ago

1) It shouldn't be too hard for a compiler to recognize

i := 0
if b {
   i = 1
}

and create the respective single instruction (or conditional bit set, depending on what happens with i, etc.)

2) It's trivial to define e.g.

func bool2int(b bool) int {
   if b {
      return 1
   }
   return 0
}

where needed.

3) Together with 1), the bool2int function should become trivial code-wise and then should be inlineable, effectively giving you int(b).

4) There may still be merit in conversions of bools to numeric values, but using a helper function for now should get you almost all the way there. The rest is optimization.

alandonovan commented 9 years ago

On 17 December 2014 at 14:38, Robert Griesemer notifications@github.com wrote:

1) It shouldn't be too hard for a compiler to recognize

i := 0 if b { i = 1 }

and create the respective single instruction (or conditional bit set, depending on what happens with i, etc.)

2) It's trivial to define e.g.

func bool2int(b bool) int { if b { return 1 } return 0 }

where needed.

3) Together with 1), the bool2int function should become trivial code-wise and then should be inlineable, effectively giving you int(b).

4) There may still be merit in conversions of bools to numeric values, but using a helper function for now should get you almost all the way there. The rest is optimization.

The question is: would you prefer to read just this "int(x.f > 0)", or four lines of statements inconveniently breaking up a long expression? My assertion is that the status quo is both less readable and less efficient.

minux commented 9 years ago

i think bool2int(x.f > 0) is just as readable. if we really need to change the language, ternary operator is more general than this proposal, and it's also trivial for the compiler to turn that into a conditional select instruction. (I'm not proposing ternary operators... it's too ugly when nested.)

mattn commented 9 years ago

FYI: https://groups.google.com/d/msg/golang-nuts/w1dPeHFSp9g/cMjgL76B7jIJ

cookieo9 commented 9 years ago

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 optionalValue if check is true.

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 optionalValue == 1 or -1 (ie: add or subtract 1 if boolean is set) could the multiplication be easily avoided.

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.

ghost commented 9 years ago

Also, the former would most likely require a multiplication instead the conditional branch. Only in the case where optionalValue == 1 or -1 (ie: add or subtract 1 if boolean is set) could the multiplication be easily avoided.

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?

minux commented 9 years ago

I think this is a dup of #6011.

011010101101 commented 9 years ago

@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.

ghost commented 9 years ago

@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.

bradfitz commented 9 years ago

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.

sirkon commented 9 years ago

I hope they didn't/won't implement this. Implicit conversion is an evil whatever pratical reasons are listed.

cespare commented 9 years ago

@DenisCheremisov this proposal is for an explicit conversion.

minux commented 9 years ago

@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.)

adg commented 9 years ago

The thread on golang-nuts raises a couple of key issues with this proposal:

It is yet another very small convenience that can be replaced with a short function,
It's not clear whether bool(0) should return false or true. There are arguments both ways.

For these reasons I am going to decline this proposal.

adonovan commented 11 months ago

@rsc and I were chatting about this earlier in the week and he suggested gathering some data, so I did. See https://go.dev/cl/550235 for the logic and its complete output.

In short, across about 19,000 modules, there are about 10K places where an if/else statement is used to simulate an int(bool) conversion. (I didn't attempt to find places where if/else was used to compute bool < bool, as is common in sort.Interface.Less implementations.) Of those, I ignored the ~5.5K in generated .pb.go files, and the ~2K in tensorflow generic_cmp files, leaving about 2700 more interesting cases. Here's a random sample of 100:

adonovan commented 11 months ago

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.

adonovan commented 11 months ago

Findings in std:

src/runtime/proc.go:5422:2: [if]: run0 = int(!iscgo && cgoHasExtraM && extraMLength.Load() > 0)
src/runtime/symtab.go:1062:2: [if]: mask = int(off == ^uint32(0))
src/reflect/abi.go:380:3: [if]: x = int(b.Get(i))
src/time/format.go:417:2: [if]: n = int(u == 0)
src/compress/flate/huffman_bit_writer.go:405:2: [if]: flag = int(isEof)
src/encoding/binary/binary.go:348:4: [if-else]: bs[0] = int(*v)
src/encoding/binary/binary.go:354:4: [if-else]: bs[0] = int(v)
src/encoding/binary/binary.go:361:5: [if-else]: bs[i] = int(x)
src/encoding/binary/binary.go:546:2: [if-else]: e.buf[e.offset] = int(x)
src/math/big/float.go:1377:2: [if]: sbit = int(len(r) > 0)
src/math/big/nat.go:235:3: [if-else]: c = int(cx < c2 || cy < c3)
src/crypto/x509/pkcs1.go:108:2: [if]: version = int(len(key.Primes) > 2)
src/internal/zstd/zstd.go:210:2: [if-else]: windowDescriptorSize = int(singleSegment)
src/go/printer/nodes.go:619:5: [if]: min = int(prev != nil && name == prev)
src/go/types/mono.go:198:3: [if]: weight = int(typ == targ)
src/internal/pkgbits/encoder.go:275:2: [if]: x = int(b)
src/compress/flate/deflate_test.go:180:3: [if-else]: b[i] = int(r.cur+int64(i) >= r.l-1<<16)
src/go/scanner/scanner_test.go:735:2: [if]: cnt = int(err != "")
src/go/types/hilbert_test.go:153:4: [if]: v = int(i == j)
src/strings/builder_test.go:105:3: [if]: wantAllocs = int(growLen == 0)
src/unicode/utf8/utf8_test.go:168:3: [if]: wantsize = int(wantsize >= len(b))

jimmyfrasche commented 11 months ago

@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)

adonovan commented 11 months ago

@adonovan should this issue be reopened?

Russ suggested I create a new issue summarizing all previous discoveries. I'll do that presently. [Done: #64825]

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)

Yeah, the pattern matcher for if cond { x = 1 } has both if-only and if-else forms; the matcher for return statements should do the same.

golang / go

proposal: support int(bool) conversions in Go.1.5 (language change) #9367