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: Go 2: add an explicit way to ensure results of arithmetic expressions are safe to use #32063

Closed
jimmyfrasche opened this issue May 15, 2019 · 10 comments
Labels
FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change
Milestone

Comments

@jimmyfrasche
Copy link
Member

This is similar to the C# checked keyword, though there are no checked blocks and it handles some more cases.

I propose a predeclared identifier checked that is used like a type conversion, but does not cause a type conversion. It returns one or two values. If one value, it returns the result of its expression or panics on failure; if two, it returns the result of its expression a bool indicating success. It can wrap arithmetical expressions, including type conversions between numeric types.

a, ok := checked(int16(n)) returns 0, false is n cannot be represented by int16

a := checked(int16(n)) panics is n cannot be represented by int16

d, ok := checked(a*b + c) returns 0, false if either a*b overflows or the addition by c carries.

d, ok := checked(checked(a*b) + c) panics if a*b overflows, and returns 0, false if not but then adding c carries. (This is not the correct way to hold it but illustrative of its compositional nature).

a := checked(uint(7) - 8) panics.

f, _ := checked((a + b)/(c - d)) returns 0 if c - d is 0. (At least for integers. Should it still return Inf for floats? There are arguments for consistency either way.) It also returns 0 for: https://play.golang.org/p/q_2Ilgid_Dt (example by @bcmills from an earlier discussion).

A NaN anywhere in a checked expression causes it to fail.

This can be insufficient if you want to know by how much something overflows, but there are functions in math/bits for that.

This can be annoying if you want a value that is always checked, but at least it is explicit. Conceivably a linter could be written that given a list of values reports whether any are used in expressions not wrapped in a checked. It does not preclude adding "always checked" types, though it would need to be made to work with them sanely (even if that just means a compiler error).

This is a bit overloaded but in all cases it has the same meaning of "I only want this if it 'works', so tell me if it doesn't".

The cases where this is useful are small and they can be worked around but the work arounds do not result in clear code and are easy to get wrong. This allows safe code to be simple.

checked isn't the best name but I couldn't think of a better one and choosing a name let me write the examples. It can be called whatever makes sense.

There is a long history of proposals in this vein, in no particular order:

Since this is most similar to #6815, @griesemer's comment #6815 (comment) is particularly applicable, but I think somewhat tempered by the large number of alternate proposals since.

@jimmyfrasche jimmyfrasche added LanguageChange v2 A language change or incompatible library change Proposal labels May 15, 2019
@gopherbot gopherbot added this to the Proposal milestone May 15, 2019
@ianlancetaylor ianlancetaylor changed the title proposal: add an explicit way to ensure results of arithmetic expressions are safe to use proposal: Go 2: add an explicit way to ensure results of arithmetic expressions are safe to use May 15, 2019
@beoran
Copy link

beoran commented May 16, 2019

While I have written a competing proposal, I think that this proposal is the simplest, easiest, and most Go-like proposal for checked arithmetic in Go I have seen. If the identifier is carefully chosen, this could also be mostly backwards compatible. If my proposal is deemed too complex, then can fully support this proposal to at least get checked arithmetic in Go.

@networkimprov
Copy link

It might help to invent an identifier that's not a word, to minimize breakage in existing code...

nof // no overflow
dno // does not overflow
saf // safe
san // sane
wfm // works for me :-)

@jimmyfrasche
Copy link
Member Author

If it's a keyword, it won't break existing code. The new go directive in .mod files is used by the build system and passed to the compiler. If you want to use a keyword introduced in go 1.x you

  • remove all uses of what will be the new keyword from your module
  • update the go directive to the version of go that introduced keyword
  • only then does the compiler treat it as a keyword, which allows "breaking" changes without breaking anything

I'm not sure it has to be a keyword. It might be able to be a predeclared identifier that is recognized as a special form when not shadowed. It's possible it'd be easier to just make it a keyword, though, especially now that those can be deployed without breaking the world.

Regardless, it could probably use a better name, though that's not terribly important at this point.

More pressing is a need for more rigorous semantics and a plan to integrate it into a language spec. I think what I provided is clear enough to express my intent, but it's still a bit hand-wavy—"arithmetical expressions" aren't defined in the spec, for instance. Once all that's worked out, a better name may present itself or at least the space of names winnowed.

@bcmills
Copy link
Contributor

bcmills commented May 16, 2019

Most of the things in Go that accept a , ok form are operators, not identifiers. (I think calls to Cgo functions are the one exception today?)

Similarly, I don't think there is a built-in function today that actually changes the evaluation semantics of its arguments.

That's not to say that we couldn't do those things, but to me they make the proposed syntax here kind of an awkward fit.

@jimmyfrasche
Copy link
Member Author

@bcmills Yes to everything you said, but it doesn't have to change the evaluation semantics.

It could return the same result as if it were an "unwrapped" expression instead of 0 in , ok form. In that case, it's not doing anything different, just reporting whether something went wrong along the way. Without the , ok it's discarding the result when it panics but still doesn't alter how it's computing anything.

I can't think of a case where you'd want the result of a calculation that you know is scare-quotes wrong, though—it seems like an either/or situation. If you need to know the overflow/carry/borrow, you'd be using the functions in math/bits instead.

If it does change the semantics and returns 0 when !ok, it can shortcircuit in complicated expressions. That's kind of micro-optimizey. It might be a savings in a tight loop, especially if the loop just bails when !ok, but that might be detectable even in the semantics-preserving variant allowing the compiler to emit shortcircuiting code because it knows the result is unused, though that seems to be entering into the "a sufficiently smart compiler" territory. Regardless, it could shortcircuit whenever there isn't a , ok since it can just immediately panic.

Linguistic purity aside, in my opinion, this would be less awkward to use than many of the alternatives, except in the case where you wanted a value that is always "checked". In all the cases where I've wanted something like this, it usually just to make sure some calculation or conversion doesn't hit any edge cases like wrapping or truncation and to return an error otherwise.

@griesemer
Copy link
Contributor

Regarding backward-compatibility: The proposal suggests a predeclared identifier (which in Go are not keywords). Since no checked predeclared identifier exists today, this would be trivially backward-compatible (if code defines a different checked, that one will simply shadow the predeclared one).

@networkimprov
Copy link

I think not literally breaking existing code (instead rendering it ambiguous) isn't a good reason to choose an ordinary word for this feature :-)

Presumably you'd vet-flag code that redefined this identifier?

@ianlancetaylor
Copy link
Contributor

I would worry about scattering checked all over Go programs. That seems like it would tend to obscure what is really going on.

It also might be a bit obscure to new adopters of Go. It might seem that checked protects against all kinds of potential errors, although I think this proposal is only that it checks for overflow.

Can I write checked(0)? If not, why not? What about checked(f())?

@jimmyfrasche
Copy link
Member Author

I would worry about scattering checked all over Go programs. That seems like it would tend to obscure what is really going on.

That is a legitimate concern. I would think that it would not be used a lot but when it was it would be a few lines and less of an obfuscation than equivalent workarounds. I don't have any empirical evidence to offer for that, just intuition based on personal experience.

Can I write checked(0)? If not, why not? What about checked(f())?

checked(f()) over promises since it cannot change how f() is computed. Allowing it makes it easier to write code but not easier to read it or reason about it.

I think it should be limited to arithmetic expressions (I'm counting type conversions).

Otherwise something like checked(a[i] + c) becomes ambiguous—did it fail because a[i] is out of bounds or a nil map or because the sum carried? Even limited to arithmetic expressions, there's still an ambiguity about what failed when there's more than one thing that can fail but at least it's the same kind of error.

That adds to the concern that it can obscure what will go on since r := checked(a[i] + f()) has to become

t0, t1 := a[i], f()
r := checked(t0 + t1)

But it keeps it clear about what it can and can't do and keeps it from sprouting null coalescing or what have you.

checked(0) is fine though. It might come up naturally in code generation and elsewhere linters can complain about the no-op.

It also might be a bit obscure to new adopters of Go. It might seem that checked protects against all kinds of potential errors, although I think this proposal is only that it checks for overflow.

I think the above covers most of that.

As to obscurity, it's not something most languages have, but it's not unprecedented, so it depends somewhat on the prior experience of the new adopter. Even if it's entirely new to them, I think the concept is simple enough, especially compared with semi-novel features Go already has like channels.

@ianlancetaylor
Copy link
Contributor

My guess is that most programs most of the time want arithmetic expressions to panic on overflow. Unfortunately we didn't write Go that way originally. But if it's correct that most programs want to avoid overflow, then this proposal would lead to checked scattered over large chunks of Go code. Or, it wouldn't do any good for most programs. Neither seems like the best path to follow.

It's also troubling that although checked is a very general word, in this proposal it only applies to arithmetic overflow and underflow. Why not checked(err)?

This proposal doesn't discuss inferred types of untyped constants. For example

var x int8 = checked(100 + 100)

should presumably panic. But this means that type inference needs to apply to the arguments of checked somehow (we already do this for shift operations and the like, but not for function calls). Or perhaps we need to write

var x  = checked(int8(100 + 100))

but that somewhat obscures the type of x.

This proposal looks more like a stopgap than a comprehensive approach to the problem of integer overflow.

Writing for @golang/proposal-review .

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

No branches or pull requests

7 participants