-
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: spec: support int(bool) conversions #64825
Comments
Worth noting that in addition to the list of examples above, there are a number of constants especially in the compiler and runtime defined as both Booleans and integers for the purpose of feature detection in different contexts; see e.g. go/src/internal/goexperiment/exp_arenas_on.go Lines 7 to 8 in adec22b
Since converting a constant to a type that is not a type parameter type results in a constant, this change would allow the integer versions to go away. That's an important feature of a language change over e.g. #61915. |
In the analysis the |
The proposal mentions that float64 and complex128 not supported, but what about other integer types? Are |
I don't know about |
In response to #64825 (comment), I updated the proposal to clarify that it includes all integer types. |
The proposal text mentions writing a "less" comparison func for bools, but does not mention the increasingly-common "cmp" comparison functions (used by (Related to the declined proposal #61643.) |
With this you could write func Compare(x, y int) int {
return int(x > y) - int(x < y)
} I don't know if that's good or bad. 😆 |
This proposal has been added to the active column of the proposals project |
Would supporting the bool->int conversion mean the following is valid? var a int = x > y || b |
No, it's an explicit conversion. It would be either
or
|
Every language change carries significant constant costs to update everything - tools, books, docs, etc - and the data (2,700 potential non-machine-generated uses in 19,000 modules) seems to show that this would be used about 0.2 times per module, which doesn't seem worth it. Assuming this gets declined, then in a separate issue, we may still want to make sure the compiler will handle code like this correctly:
That way people can still write their own version and know it will be handled properly. |
I think that particular form is not compiled today as efficiently as it could be, but if you write it instead as:
...then it compiles down to |
It is already basically handled properly. |
As this proposal states, discussion of efficiency here is a distraction, just as it was a distraction in #9367. This is purely about writing clear and concise code. Right now, it's annoying to deal with bools when writing some kinds of expressions, particularly comparisons, in a way that dealing with other basic data types like integers and strings is not. This is particularly true for the I'm sure that if we look, we can find plenty of language changes that will be used by less than 20% of modules. (Not to mention existing language features. What percentage of modules use complex types?) |
As remarked in #61915 (comment) and this thread, one of the things that cannot be done now is use of bool→int in constants. That requires a language change. I would also like to reiterate that the analysis undercounts in multiple ways so the numbers provided are a floor. I'm also not sure why we're not counting machine generated modules. Use in machine generated code should be weighted less, sure, but not discarded. It's still evidence of real-world use. I'm also not sure use-per-module is the right proxy. I've written modules that don't use floating point numbers that doesn't make them less useful. |
Based on the discussion above, this proposal seems like a likely decline. |
Personally I'd be sad if we punt on this addition once again. I very much relate to Alan's original points that it feels like an omission in the language that makes it somewhat inconsistent. I've had to write this function five times in the past handful of years, which is certainly not often, but each time I'm left with the impression that it's a silly situation to be in. In that way, I see this proposal as being very similar to It's true that every language change carries its cost, but in this case, I honestly feel like the cost is tiny. We should weigh quality of life improvements for human developers in the long term over minimal tooling pain in the short term, I think. |
I filed an alternative proposal: #66062. |
One thought I had: if the solution being a language change is what is keeping this proposal from being a net positive, we could always consider a builtin function like |
The fact that this has been requested so many times by different people seems to indicate that there is a need here. It's a simple and obvious feature request, I don't understand the resistance against it. |
I agree with this 100%: currently it's possible to convert |
It may be helpful if you can share places you've done that. Examples of people actually needing int(bool) conversions in a capacity that a function cannot replace is probably the most important kind of information for the proposal process. |
One example I found in the go codebase itself: if these files were not auto-generated, I think it does make more sense for these values to be bools instead of ints from a type correctness standpoint. |
Decline doesn't mean that it will never happen, just that it won't happen now. We have limited time, and as more and more things build on Go, every change becomes more difficult to make, because it requires updating everything above it in the stack. I'm sure approximately nothing uses complex types. If Go didn't have them today, I think we would decline adding them pretty quickly. It was much easier to add things back then. The bar is always moving up, for better or worse. |
Another example from the Go tree: https://github.com/golang/go/blob/go1.22.1/src/internal/goexperiment/exp_allocheaders_on.go Having written a fair amount of code using the GOOS and GOEXPERIMENT int constants, I agree it's very annoying. :) Are there examples not from the Go tree itself? This pattern is unfortunately not captured by the ecosystem analysis (and I'm not sure how you'd do it reliably). |
No change in consensus, so declined. |
The analyzer I used is at https://go.dev/cl/550235, where it will remain in deep cryostorage until needed again. |
I'm surprised by these assessments by the proposal review group. The discussion here seems overwhelmingly in favor of adding these conversions. E.g., there are as many thumb-up "votes" on this issue already as there were on #59488, which added the Proposal #46505 to allow @ianlancetaylor argued:
Doesn't this argument hold even more strongly for allowing users to write |
Worth mentioning there are many more examples under the b2i name out in the wild than those mentioned. This pattern is especially present in embedded systems programming. Here are more examples under the b2u8 name, mostly belonging to the embedded ecosystem of Go. |
Don't forget to enable the "Exclude Go vendor" filter in the left pane. BTW this is my favorite of all of them, which uses "unsafe" to make it faster. The difference is "almost unnoticeable", but every picosecond counts! |
I wish GitHub offered "☠️" as a reaction. |
Funny that happend after two encouraging comments by well-known gophers. Really sad the Go team played this as it is.
That's exactly how I read the discussion. Now I'm stuck with being able to do string([]byte) but not int(bool).
Given that it would be great if the community could weigh in in prioritising changes instead of just singular yes/no votes? |
That's trivially |
There is no denying this would occasionally be useful. The problem is balancing overall churn against that utility. If someone is very excited about this change, the usual way to make the case (or not) would be to prepare a small stack of CLs showing (1) the spec changes, (2) the compiler and go/types changes, and (3) updating all the code in the main repo to use it as effectively as possible. (1) and (2) show that there is not too much involved (although there is always more to do in programs like staticcheck and gopls), and (3) demonstrates the benefits. I am still skeptical that the benefits will be large enough, but we definitely missed the ones @aclements pointed out in package runtime, of not having to maintain pairs of aligned constants. We can just emit the booleans and convert them in constant expressions as needed. Maybe there are others we missed too. The diffs for (3) are what matter most: demonstrate benefits justifying the cost in (1) and (2). |
Composing fmt.Printf("%s.(%s).%s", // e.g. "bytes.(*Buffer).WriteString"
packageName,
strings.Repeat("*", int(isPtr)), // optional *
typeName,
methodName) |
Too much talk... let's implement this feature already! I mean.. I do never get a technical reason for not doing this... you may peek at the summary I have made regarding the state of this, here. |
You can use nstd.Btoi at the time, though it has not the type inference convenience. |
I am in support of this. This missing feature is the one reason I would still use c. |
While I appreciate the vote of support for this late proposal, I question the wisdom of your choices. ;-) |
Proposal Details
I propose, a second time, the following language change first proposed for go1.5 in #9367:
An explicit conversion
int(b)
, whereb
has a boolean type andint
stands for any integral type, would become legal and would yield 1 ifb
is true, 0 otherwise.Rationale: currently this function cannot be computed in a single expression, despite it being often useful, hard to misuse, present in nearly all other languages, and syntactically natural in Go. As a corollary, there is currently no simple way to derive an integer constant from a boolean constant.
While I appreciate the value of avoiding implicit int-to-bool conversions, forbidding explicit ones seems slightly obtuse, and as a result of its omission, one must write four extra lines of code:
Also, consider implementations of sort.Interface.Less that define a total order over tuples. They consist of a sequence of operations, one per column (x, etc) of the tuple, of this form:
For booleans, the
<
operator must be expressed as the opaque!a[i].x && a[j].x
. It would be easier to readint(a[i].x) < int(a[j].x)
.The motive here is brevity and clarity, not efficiency. I am confident that a good compiler can avoid the branch. The concern with efficiency in the previous proposal was a distraction, and it has been dealt with separately.
We do not propose to support the reverse operation,
bool(i)
, as one can already sayi != 0
in expression context (and it's shorter). Also,i != 0
makes the semantics explicit, wherebool(i)
could potentially be misinterpreted asi > 0
,i == 1
,i == max[u]int
,i == -1
, ori & 1 == 1
, all of which are representations of "true" used in other languages; and George Boole would have argued that the bool(int) function is simply not defined for values other than zero and one.We also do not propose to support
float64(b)
andcomplex128(b)
. Though consistent, there is little need for such conversions, and they can easily be implemented in two steps, still within expression context, for examplefloat64(int(b))
.In https://go.dev/cl/550235, I measured the occurrence of code snippets like the one above. 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 copies of github.com/wzzhu/tensor, leaving about 2700 more interesting cases. Here's a random sample of a few dozen:
https://go-mod-viewer.appspot.com/[email protected]/cmd/scollector/collectors/ntp_unix.go#L76: [if]: current_source = int(fl == "")
https://go-mod-viewer.appspot.com/cosmossdk.io/client/[email protected]/internal/testpb/query.pulsar.go#L2731: [if-else]: dAtA[i] = int(x.Bools[iNdEx])
https://go-mod-viewer.appspot.com/github.com/BurntSushi/[email protected]/randr/randr.go#L3797: [if-else]: buf[b] = int(Delete)
https://go-mod-viewer.appspot.com/github.com/BurntSushi/[email protected]/xproto/xproto.go#L10065: [if-else]: buf[b] = int(Delete)
https://go-mod-viewer.appspot.com/github.com/BurntSushi/[email protected]/xproto/xproto.go#L10719: [if-else]: buf[b] = int(OwnerEvents)
https://go-mod-viewer.appspot.com/github.com/BurntSushi/[email protected]/xproto/xproto.go#L3668: [if-else]: buf[b] = int(v.OverrideRedirect)
https://go-mod-viewer.appspot.com/github.com/Hyperledger-TWGC/[email protected]/sm4/sm4_gcm.go#L77: [return]: return int(temp&0x01 == 1)
https://go-mod-viewer.appspot.com/github.com/XiaoMi/[email protected]/mysql/resultset_sort.go#L132: [return]: return int(v > s)
https://go-mod-viewer.appspot.com/github.com/andybalholm/[email protected]/brotli_bit_stream.go#L306: [if-else]: tmp = int(depths[symbols[0]] == 1)
https://go-mod-viewer.appspot.com/github.com/andybalholm/[email protected]/decode.go#L1282: [if-else]: s.should_wrap_ringbuffer = int(uint(s.pos) != 0)
https://go-mod-viewer.appspot.com/github.com/apache/arrow/go/[email protected]/arrow/compute/internal/exec/span.go#L280: [if-else]: a.Nulls = int(val.IsValid())
https://go-mod-viewer.appspot.com/github.com/apache/arrow/go/[email protected]/internal/bitutils/bitmap_generate.go#L85: [if-else]: outResults[i] = int(g())
https://go-mod-viewer.appspot.com/github.com/aviddiviner/[email protected]/docopt.go#L433: [if]: argcount = int(eq == "=")
https://go-mod-viewer.appspot.com/github.com/blevesearch/[email protected]/s2/edge_clipping.go#L148: [if]: ai = int(a.X > b.X)
https://go-mod-viewer.appspot.com/github.com/btccom/go-micro/[email protected]/api/router/static/static.go#L232: [if]: idx = int(len(req.URL.Path) > 0 && req.URL.Path != "/")
https://go-mod-viewer.appspot.com/github.com/cockroachdb/[email protected]/sstable/writer_test.go#L923: [if]: expectedLength = int(expectedIntersects)
https://go-mod-viewer.appspot.com/github.com/consensys/[email protected]/ecc/bw6-761/fr/fri/fri.go#L309: [if-else]: fullMerkleProof = int(len(pp.Rounds[0].Interactions[0][0].ProofSet) > len(pp.Rounds[0].Interactions[0][1].ProofSet))
https://go-mod-viewer.appspot.com/github.com/coreservice-io/[email protected]/version_util/version.go#L81: [return]: return int(a.tail > b.tail)
https://go-mod-viewer.appspot.com/github.com/cosmos/[email protected]/internal/testprotos/test3/test.pulsar.go#L6416: [if-else]: dAtA[i] = int(x.RepeatedBool[iNdEx])
https://go-mod-viewer.appspot.com/github.com/datastax/[email protected]/datacodec/boolean.go#L187: [if-else]: *d = int(wasNull || !val)
https://go-mod-viewer.appspot.com/github.com/dop251/[email protected]/ftoa/ftoa.go#L614: [if]: stop = int(len(buf) > 0 && buf[0] == '-')
https://go-mod-viewer.appspot.com/github.com/dotcloud/[email protected]/auto.go#L174: [if]: start = int(mtype.NumIn() > 0 && mtype.In(0).AssignableTo(reflect.TypeOf(autoHandler)))
https://go-mod-viewer.appspot.com/github.com/dubbogo/[email protected]/time/timer.go#L118: [if-else]: ret = int(first.trig > second.trig)
https://go-mod-viewer.appspot.com/github.com/fraugster/[email protected]/type_boolean.go#L91: [if]: v = int(values[i].(bool))
https://go-mod-viewer.appspot.com/github.com/gagliardetto/[email protected]/encoder.go#L189: [if]: num = int(b)
https://go-mod-viewer.appspot.com/github.com/golang-commonmark/[email protected]/blockquote.go#L160: [if]: d = int(spaceAfterMarker)
https://go-mod-viewer.appspot.com/github.com/hyperledger/[email protected]/core/FP256BN/FP4.go#L214: [if-else]: u = int(F.a.iszilch())
https://go-mod-viewer.appspot.com/github.com/intel/[email protected]/pkg/blockio/oci_test.go#L95: [if]: expectedErrorCount = int(len(tc.expectedErrorSubstrings) > 0)
https://go-mod-viewer.appspot.com/github.com/intel/[email protected]/pkg/sst/sst_if.go#L109: [if]: ReadWrite = int(doWrite)
https://go-mod-viewer.appspot.com/github.com/jezek/[email protected]/xprint/xprint.go#L639: [if-else]: buf[b] = int(Cancel)
https://go-mod-viewer.appspot.com/github.com/jezek/[email protected]/xproto/xproto.go#L7364: [if-else]: buf[b] = int(DoAcceleration)
https://go-mod-viewer.appspot.com/github.com/la5nta/[email protected]/rigcontrol/hamlib/rigctld.go#L168: [if]: bInt = int(on == true)
https://go-mod-viewer.appspot.com/github.com/m3db/[email protected]/src/dbnode/persist/fs/msgpack/stream.go#L170: [if]: unreadBytes = int(s.unreadByte != -1)
https://go-mod-viewer.appspot.com/github.com/m3db/[email protected]/src/dbnode/storage/bootstrap_instrumentation.go#L180: [if]: status = int(isBootstrapped)
https://go-mod-viewer.appspot.com/github.com/mitchellh/[email protected]/pixel_format.go#L111: [if-else]: boolByte = int(format.TrueColor)
https://go-mod-viewer.appspot.com/github.com/nicocha30/[email protected]/pkg/atomicbitops/bool.go#L34: [if]: u = int(val)
https://go-mod-viewer.appspot.com/github.com/nicocha30/[email protected]/pkg/state/wire/wire.go#L88: [if-else]: v = int(b)
https://go-mod-viewer.appspot.com/github.com/oasisprotocol/[email protected]/internal/modm/modm_64bit.go#L736: [if]: cmp = int(i == 0)
https://go-mod-viewer.appspot.com/github.com/onosproject/onos-api/[email protected]/onos/config/v3/typedvalue.go#L877: [if]: intval = int(b)
https://go-mod-viewer.appspot.com/github.com/parquet-go/[email protected]/row_buffer_test.go#L146: [if]: definitionLevel = int(!node.Required())
https://go-mod-viewer.appspot.com/github.com/peske/[email protected]/cmd/goyacc/yacc.go#L2126: [if]: nolook = int(tystate[i] != MUSTLOOKAHEAD)
https://go-mod-viewer.appspot.com/github.com/pion/webrtc/[email protected]/atomicbool.go#L27: [if]: i = int(value)
https://go-mod-viewer.appspot.com/github.com/pocketbase/[email protected]/forms/admin_upsert_test.go#L160: [if]: expectInterceptorCall = int(s.expectError)
https://go-mod-viewer.appspot.com/github.com/polarismesh/[email protected]/store/mysql/strategy.go#L80: [if]: isDefault = int(strategy.Default)
https://go-mod-viewer.appspot.com/github.com/regen-network/regen-ledger/api/[email protected]/regen/ecocredit/marketplace/v1/state.pulsar.go#L515: [if-else]: dAtA[i] = int(x.Maker)
https://go-mod-viewer.appspot.com/github.com/regen-network/regen-ledger/[email protected]/regen/ecocredit/marketplace/v1/tx.pulsar.go#L2785: [if-else]: dAtA[i] = int(x.DisableAutoRetire)
https://go-mod-viewer.appspot.com/github.com/regen-network/regen-ledger/[email protected]/regen/ecocredit/v1alpha1/types.pulsar.go#L2460: [if-else]: dAtA[i] = int(x.AllowlistEnabled)
https://go-mod-viewer.appspot.com/github.com/robotn/[email protected]/glx/glx.go#L1494: [if-else]: buf[b] = int(IsDirect)
https://go-mod-viewer.appspot.com/github.com/robotn/[email protected]/randr/randr.go#L3804: [if-else]: buf[b] = int(Pending)
https://go-mod-viewer.appspot.com/github.com/robotn/[email protected]/shape/shape.go#L130: [if-else]: buf[b] = int(v.Shaped)
https://go-mod-viewer.appspot.com/github.com/robotn/[email protected]/xproto/xproto.go#L10719: [if-else]: buf[b] = int(OwnerEvents)
https://go-mod-viewer.appspot.com/github.com/robotn/[email protected]/xproto/xproto.go#L7614: [if-else]: buf[b] = int(Exposures)
https://go-mod-viewer.appspot.com/github.com/sajari/[email protected]/fuzzy.go#L255: [if-else]: temp = int((*a)[j-1] == (*b)[i-1])
https://go-mod-viewer.appspot.com/github.com/segmentio/[email protected]/file.go#L692: [if]: f.index = int(f.dictOffset > 0)
https://go-mod-viewer.appspot.com/github.com/shakinm/[email protected]/xls/workbook.go#L107: [if-else]: grbitOffset = int(len(wb.sst.RgbSrc) == 0)
https://go-mod-viewer.appspot.com/github.com/thanos-io/[email protected]/pkg/store/cache/caching_bucket_test.go#L442: [if]: expectedHitsDiff = int(expectedCache)
https://go-mod-viewer.appspot.com/github.com/u2takey/[email protected]/strings/string.go#L225: [if]: i = int(strings.Index(str, \u) > 0)
https://go-mod-viewer.appspot.com/github.com/uadmin/[email protected]/setting.go#L522: [if]: n = int(v)
https://go-mod-viewer.appspot.com/github.com/zhangzhaojian/[email protected]/mongo/collection.go#L388: [if]: limit = int(deleteOne)
https://go-mod-viewer.appspot.com/go.etcd.io/raft/[email protected]/rawnode_test.go#L325: [if]: maybePlusOne = int(ok && autoLeave)
https://go-mod-viewer.appspot.com/go.flow.arcalot.io/[email protected]/schema/function.go#L322: [if]: expectedReturnVals = int(f.StaticOutputValue != nil || f.DynamicTypeHandler != nil)
https://go-mod-viewer.appspot.com/[email protected]/internal/compile/serial.go#L207: [return]: return int(b)
https://go-mod-viewer.appspot.com/gopkg.in/olebedev/[email protected]/api.go#L947: [if]: val = int(val)
And here are all occurrences in the standard library:
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))
The text was updated successfully, but these errors were encountered: