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: math: Go 2: use explicit types for limit constants #23087

Closed
ianlancetaylor opened this issue Dec 11, 2017 · 14 comments
Closed

proposal: math: Go 2: use explicit types for limit constants #23087

ianlancetaylor opened this issue Dec 11, 2017 · 14 comments
Labels
FrozenDueToAge Proposal v2 A language change or incompatible library change
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

math.MaxInt64 and friends are untyped constants. This causes confusion when using the constants on 32-bit systems. For example, on a 32-bit system,

var V = math.MaxInt64

does not compile: constant 9223372036854775807 overflows int.

This confusion has been reported at #19621 and #23086.

This can not be changed for Go 1 without breaking backward compatibility. For Go 2 we could use explicit types for the constants, so that the type of math.MaxInt64 is int64, and so forth. I believe that would cause less confusion in the long run.

It could, however, break currently working code that expects values like math.MaxInt8 to take on type int. In this proposal they would be type int8. So some existing code may need to be changed to add explicit conversions to int. It should be possible to automate this conversion using go fix.

@ianlancetaylor ianlancetaylor added the v2 A language change or incompatible library change label Dec 11, 2017
@gopherbot gopherbot added this to the Proposal milestone Dec 11, 2017
@dsnet
Copy link
Member

dsnet commented Dec 11, 2017

Wouldn't this change break some optimizations that rely on math.MaxInt64 being a const? If it is a mutable global variable, the compiler can no longer perform certain classes of value-range propagation optimizations, right?

@bradfitz
Copy link
Contributor

@dsnet, he's proposing making it a typed constant, not a variable.

@dsnet
Copy link
Member

dsnet commented Dec 11, 2017

Ah, my mistake. I saw the var V = math.MaxInt64 and got confused.

@cznic
Copy link
Contributor

cznic commented Dec 11, 2017

Untyped constants are valuable in computing values of expressions composed of other untyped constants and within quite large limits it's guaranteed to always work. var foo = anyUntypedConstant, OTOH, was never guaranteed nor IMHO intended to always work.

@flanglet
Copy link

What about implicit constants ?

n := 1
n &= 0xFFFF0000

@ianlancetaylor
Copy link
Contributor Author

ianlancetaylor commented Dec 11, 2017

@cznic Do you have any examples from real code?

@flanglet I'm not sure what you mean but that seems like a different problem. I'm talking very specifically about the constants defined in the math package.

@jimmyfrasche
Copy link
Member

This seems like something to be solved by a linter.

@ianlancetaylor
Copy link
Contributor Author

@jimmyfrasche Are you talking about the original problem in this issue? There is no need for a linter for that; the compiler gives an error. This proposal is a suggestion for a way to remove that error as a stumbling block for people just learning how to use Go. I can certainly understand the argument that this isn't worth changing, but a linter won't help.

@jimmyfrasche
Copy link
Member

A linter could say "this works on your computer because it's 64bit but won't compile on 32bit arches because that constant will overflow int". That would at least help with people creating accidentally 64bit only programs, at least.

Maybe that and a better error message like: constant 18446744073709551615 (requires 64 bit integer) overflows int (32 bit integer)

@ericlagergren
Copy link
Contributor

@ianlancetaylor isn't this something the compiler could produce a better error message with instead of using typed constants? It's quite nice—and IMO clearer—being able to omit the type conversions.

@jimmyfrasche
Copy link
Member

The confusion seems to come from

  • perhaps not immediately recognizing that that large string of digits maps back to a constant name
  • not recognizing that the value is an untyped constant in a position that gives it its default type
  • not being able to see immediately that a large string of digits in base 10 requires so much space in base 2

I think a better error message should

  • when possible, include the constant name (should be discoverable when it's a single constant not involved in an expression)
  • note whenever an ideal constant is given its default type implicitly
  • note the precision of int/uint for the target GOARCH when they're involved

Those are all orthogonal and each useful in its own right. In the case of var V = math.MaxInt64 they'd combine into something like

untyped constant math.MaxInt64 implicitly converted to int but overflows int (32 bit)

That makes it much clearer what's going on and why.

If you don't know all of the rules at play resulting in that error it gives you all the information you need to go look them up.

Even if you do know all that, it's still much friendlier and requires less thought to see where you went wrong. Error messages shouldn't make you think "huh?"—they should make you think "oh, duh!"

I'm sure that those would require extra bookkeeping or backtracking in the compiler and that some of those enhancements would be easier than others but they seem like they'd go a long way to making all errors involving untyped constants easier for everyone to deal with.

@bradfitz
Copy link
Contributor

Another: #23163

@bcmills
Copy link
Contributor

bcmills commented Jan 4, 2018

This proposal seems like it would break comparisons against smaller-than-64-bit constants.

For example, at the moment you can check the value of an int easily before converting it to an int32:

x, err := strconv.Atoi(…)
if err != nil {
	return …
}
if x > math.MaxInt32 || x < math.MinInt32 {
	return …
}
x32 := int32(x)

On the other hand, if math.MaxInt32 is an int32, the condition stutters on the type:

if x > int(math.MaxInt32) || x < int(math.MinInt32) {

There are some other ways to address that stutter, of course. We could allow non-narrowing integer conversions (e.g. int32 to int) to occur implicitly, or by make it easier to do narrowing range-checks by some other means (e.g. the , ok conversions proposed in #19624).

Still, I think the impact on range checks is likely to be a significant downside.

@ianlancetaylor
Copy link
Contributor Author

The arguments above seem convincing. Closing in favor of better compiler error messages.

@golang golang locked and limited conversation to collaborators Mar 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Proposal v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests

9 participants