Skip to content
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: disallow assigning >32bit untyped constant to variable of type int #20616

Open
bradfitz opened this issue Jun 8, 2017 · 10 comments
Labels
LanguageChange NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal v2 A language change or incompatible library change
Milestone

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Jun 8, 2017

I propose that the Go language and implementations be changed to reject a constant assignment to a variable of type int (or uint) if the constant would overflow a 32-bit int (or uint), regardless of the size of int on the current GOOS.

That is, on amd64, this would be rejected:

const MaxInt64 = 1<<63 - 1

var x int = MaxInt64

Currently that passes on 64-bit platforms but fails when people cross-compile it to Raspberry Pis or run it on the playground (https://play.golang.org/p/4PK8z_WBKi).

This bites people like in googleapis/google-cloud-go#648 where it lurks unnoticed by users & continous builds for a long time, until somebody builds the code for the increasingly rare 32-bit machines.

/cc @griesemer

@mvdan
Copy link
Member

mvdan commented Jun 8, 2017

Have you thought about making this a go vet error instead? Or is "breaking" these programs on 64-bit acceptable, given that they're not valid programs on all architectures?

@cznic
Copy link
Contributor

cznic commented Jun 8, 2017

I don't think the lack of proper release testing of some projects for more platforms/architectures justifies bending the language to "solve" the problem. (And I was bitten this way by my own code more than once.)

@bradfitz
Copy link
Contributor Author

bradfitz commented Jun 8, 2017

@mvdan, a vet error would be a good first step (especially if #18084 happens), but I still consider these invalid programs in all cases.

@cznic, I would normally agree with you on that (that the answer is testing), but in this case I think the language just shouldn't permit it in the first place. I don't think it should be possible to write a portable Go program (no syscall, unsafe, etc) program that runs on some machines but not others.

@rsc
Copy link
Contributor

rsc commented Jun 12, 2017

Based on discussion with @bradfitz, this is only about assigning an untyped constant to an int/uint/uintptr. The following are OK because the constants are typed:

const X int = 1<<63-1
var x int = X

var y uintptr = ^uintptr(0)

@rsc rsc added the v2 A language change or incompatible library change label Jun 12, 2017
@davecheney
Copy link
Contributor

davecheney commented Jun 12, 2017 via email

@rsc
Copy link
Contributor

rsc commented Jun 13, 2017

@davecheney, so then I can't write ^uintptr(0) at all?

@davecheney
Copy link
Contributor

davecheney commented Jun 13, 2017 via email

@ianlancetaylor
Copy link
Contributor

It's currently confusing in Go that the expression uint(-1) does not compile. This proposal would seem to extend that to saying that the expression int64(0x100000000) would not compile. Russ's comment above suggests that we only want to give an error on an attempt to assign an untyped constant to a variable of type int, but that isn't something we have in the language right now, so that would need to be more clearly specified.

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 20, 2018
@drakmaniso
Copy link

As long as int is part of the language, it will be a potential failure point for cross-platform development. This proposal would solve compile-time failures, but leave the much harder to spot (and to test for) run-time bugs (i.e. when an int variable suddenly overflows). Note that 32 bit platforms are still the norm for embedded development, and TinyGo has shown that Go has a good potential there.

IMHO the right solution would be to remove int altogether, and use int32 as the default integer type. This is the approach taken by Rust. (see RFC 212)

I don't think there is any good reason to choose int64 as the default, even on 64 bit platforms: 32 bit integers already provide a reasonable range of numbers, are most of the time faster, and safe to convert to the default float type.

Another solution would be proposal #19623

@beoran
Copy link

beoran commented Sep 26, 2019

Yes, I think this proposal only plasters over the problem, so I am not in favor of it. #19623 would probably be the better solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LanguageChange NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests

9 participants