-
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: checked integer types #30613
Comments
While this is a simple solution it doesn't seem to be the best solution to me. With my range types proposal, #30428, and use of type aliases, this simply becomes: type OInt8 = range int8[math.MinInt8:math.MaxInt8]
type OInt16 = range int16[math.MinInt16:math.MaxInt16]
type OInt32 = range int32[math.MinInt32:math.MaxInt32]
type OInt64 = range int64[math.MinInt64:math.MaxInt64]
type OUInt8 = range uint8[math.MinUint8:math.MaxUint8]
type OUInt16 = range uint16[math.MinUint16:math.MaxUint16]
type OUInt32 = range uint32[math.MinUint32:math.MaxUint32]
type OUInt64 = range uint64[math.MinUint64:math.MaxUint64] As per my proposal, panics are emitted on overflow, but the |
The “o” prefix is a bit odd, in that these are types that don’t overflow. I don’t have a better suggestion, though. “c” for checked, perhaps, although |
Yes, I considered a |
And should a Go 2 derivative of this concept permit |
Maybe I’ve been hanging out on the wrong side of the C tracks, but I read int32p as “pointer to some int32”. And if you put it as a prefix...well, seems like you’d need cup and quart as well. “b” for bounded? |
@extemporalgenome Your example |
I certainly prefer this proposal to #30209 which I felt was over-complicated. My only reservation is whether this is a better way of solving the overflow problem than the wildly popular #19623 which (at the cost of some efficiency) gets rids of it altogether and also deals with requests for larger integer types such as Anyway, to return to this proposal, I agree that the |
Kinda, but not really for Rust: https://doc.rust-lang.org/book/ch03-02-data-types.html#integer-overflow
|
@extemporalgenome suggested a suffix, which is logical, since bounding pertains to the bit count
|
A suffix makes a certain amount of sense but I think it would be too easy to miss when reading the code. |
Human perception finds words, not character strings, so a prefix letter is easily confused for identifiers with that letter sequence. Certain letters or an underscore would be hard to overlook
|
Rust has the idea that debug code will be checked and production code will not. While this may seem nice for performance, it has the downside now your debug version and production version are different. That's unacceptable for safety and/or security critical software. The debug version and the production version should be semantically the same. I think overflow should always cause a recoverable panic, and , if the I also maintain that my proposal ranged integer proposal is the far more generally useful approach to solving this problem. With it, naming of the ranged, non-overflowing types can also be left to the programmer. Also, I updated my proposal to use the minimal and maximal values of the underlying type by default so it becomes even easier to write bounds checked range types: // Let us put the types in a package
package bounded
type Int8 = range int8[:]
type Int16 = range int16[:]
type Int32 = range int32[:]
type Int64 = range int64[:]
type UInt8 = range uint8[:]
type UInt16 = range uint16[:]
type UInt32 = range uint32[:]
type UInt64 = range uint64[:]
// Or just use the range type as is.
var j bounded.Int8 = range int8[:](1 << 7) // OK
var i range int8[:] = bounded.Int8(1 << 8) // panic
const MaskBit range int8[:] = range int8[:](1 << 8) // Compile error |
Interesting highlight in #30613 (comment), could go introduce a build tag to make overflows panic? Might be useful to set in testing, canaries, etc |
@JavierZunzunegui In my opinion, that is not a feature. As others have said, it's like using a life vest when you are in a wading pool but not using one when you are in the ocean. |
Could you give some more detail on why that is the case? I think it's important that checked arithmetic be at least as concise as unchecked arithmetic, so that there is no pressure for developers to use unchecked arithmetic where it is not needed. However, since existing libraries (such as That was why I included a relaxed assignability rule in #30209, so I'd like to understand why a similar approach is considered untenable for this proposal. |
What I mean when I say "no automatic conversion" is that it is otherwise ambiguous whether an operation should overflow or wrap. Consider func PrintSum(a int, b oint) {
fmt.Println(a + b)
} I think it's essential that everyone reading Go code understand immediately whether For the record, see #31500 for an alternative proposal. |
We already permit implicit type conversion only in certain contexts: for example, the conversion from an untyped numeric constant to a defined numeric type only occurs when the value is used as a function argument, variable, or arithmetic operand. I think the logical rule for this proposal in particular would be to make the checked and unchecked types mutually-assignable, but to require arithmetic expressions to involve only checked or only unchecked types. func ident(a int) int { return a }
func oident(a oint) oint { return a }
func Example() {
var a int
var b oint
c := ident(b) // ok: int (no possibility of overflow)
x := ident(a) + ident(b) // ok: int (unambiguously unchecked)
y := oident(a) + oident(b) // ok: oint (unambiguously checked)
z := oident(a + 1) // ok: a+1 is performed unchecked, then passed as oint
w := ident(b + 1) // ok: b+1 is performed checked, then passed as int
n := len(someSlice) + b // error: expression combines checked and unchecked operands
var z oint = len(someSlice) // ok: no possibility of overflow
m := z + b // ok: unambiguously checked
bad := a + b // error: expression combines checked and unchecked operands
bad := ident(a + b) // error: expression combines checked and unchecked operands
bad := ident(a) + oident(b) // error: expression combines checked and unchecked operands
} That doesn't prevent someone from unintentionally writing an unchecked operation where they meant to write a checked one (or vice-versa), but since the builtins (particularly In particular, I would expect that many sites that should be written: foo(oint(n) + 1) will instead be written foo(oint(n + 1)) regardless of any assignability rule (much like the floating-point-to- |
That is not strictly accurate according the spec, since That minor quibble aside, I think that experience shows that people find the untyped constant rules to be confusing, and there are frequent references to https://blog.golang.org/constants. I think we should be extremely cautious about adding anything similarly confusing. |
@ianlancetaylor I see this as a similar property to the -race flag - nobody uses it in production (I believe), that doesn't mean it isn't helpful. |
On one of my previous jobs we did exactly that 😉 |
I would absolutely turn on the Overflow checking is expected to have a very low additional cost (on amd64, one correctly predicted branch per operation); of course any approach would have to be reevaluated if it turns out to be more expensive than expected. |
A simpler approach would be to consider that people using sized types want values of exactly that size, and don't want to necessarily worry about overflowing at the edges. And people using unsigned types typically want wrapping semantics. So the only type that really needs a checked variant is The hard part is picking a name for that type. Here are some possibilities, based on comments above and in other issues, none of which seem perfect:
|
I don't think this is true. I very often use And as a single anecdote, the only production issue I can recall tracking back to an integer overflow off the top of my head right now was a multiplication overflow on an |
Add |
Yep, if there's to be a single checked In fact, I've seen this name used in C++ in a similar context though as a class library rather than a built-in language type. |
I've been thinking some more about this proposal, particularly in contrast to #30209. The major difference between this proposal and part 1 of #30209 is the omission of the Separate “wrapping” types would allow tools — particularly linters and fuzzers — to distinguish between two very different cases: “I wrote this code without considering overflow behavior”, and “I've thought about it and this code really should wrap silently on overflow”. The “without considering wrapping behavior” case can arise for various reasons: because it predates the ability to write checked code, or because it was copied or derived from earlier code, or because the user just didn't put much thought into overflow behavior. It seems important for both human readers and mechanical linters to be able to distinguish that from an intentional decision to use wrapping arithmetic: for example, someone auditing a package for bugs may want to focus much more on unexpected overflow than intentional overflow. |
|
This was provided as a motivation for Dart 2 using wrap-around integers. https://github.com/dart-lang/sdk/blob/master/docs/language/informal/int64.md#wrap-around |
I thought about this, but I worry that it would introduce too many bugs to switch programs over from one behavior to another. Then again, maybe probably the majority programs with overflow issues already have the bugs, they just don't know it yet. |
@nathany Please also look at #30209. @carlmjohnson See #31500, which was rejected. While it's true that RISC-V doesn't have an overflow flag, it's still possible to detect overflow. I don't see that as a reason one way or another. |
@bcmills That's a good point but isn't it a temporary condition? Go code that exists today is a small fraction of Go code that will exist in the future. |
@ianlancetaylor, I don't think it is a temporary condition. Code that exists today — particularly as examples and tutorials in blog posts, books, and other formats unlikely to be kept up-to-date — will continue to exist for the foreseeable future, and even if we add a facility for checked arithmetic it will remain very difficult to determine whether the author of a given snippet of code was aware of it at the time the code was written (or copied). That is: given the possibility of copying code from some other example or codebase, even code marked with a Moreover, without some clear way to determine whether code has already been audited for unintended overflow, I don't see how we can expect users to reasonably migrate existing code. Some explicit marker seems necessary, whether it is a structured comment, a separate type or set of types, or some other mechanism (such as a “magic import” to annotate the auditing). |
A small point in original proposal:
I believe this is only true for signed integer types (but invariably wrap). Unsigned integers wrap, since C-89. |
I used to think that detecting integer overflow/underflow was not important, but I have been working on a finance project, and I truly wish that Go had that feature. If you are doing a simple arithmetic, you can easily validate user input to prevent overflow or underflow. However, when complex computation is involved, it is difficult to determine the valid range of values that the user can enter. If some overflow or underflow occurs, we need a way to detect it and inform the user that the result is invalid. Some people may make important decisions based on that number. It is even trickier when the overflow/underflow happens to intermediate numbers that you use to perform further computations. It becomes difficult to tell when the result is wrong unless you manually compute it and match it against the result. I see one problem with adding new types: interoperability with existing types. Since Go is strongly typed, you need to explicitly cast Rather than adding new types, I am thinking that we can borrow C#
I currently use math to detect overflow and underflow. The formula is determined as follows:
You can do this with functions, but it is hard to read. var err error
number3 := checked.Add(number1, number2, &err)
number6 := checked.Sub(number4, number5, &err)
number7 := checked.Multiply(number3, number6, &err)
if err != nil {
//error handling
} With var result int
if err := checked {
result = (number1+number2) * (number4-number5)
}; err != nil {
//error handling
} which is a lot cleaner, especially when complex computation is involved. |
@henryas While enamoured with the C# Also, with risk of being labelled a pedant, you can't get underflow with integer arithmetic. You probably mean -ve overflow, or unsigned integers going -ve. |
@AndrewWPhillips Thanks for the comment. The project that I am working on involves some financial and risk calculation. It isn't simply about calculating money, but it is about the intermediate numbers and fractions used to come up for the final computation. A slight difference due to rounding can accumulate quickly and result in a significant amount of money. One of the requirements is that it must be accurate by at least eight decimal places. That means the numbers must be multiplied by at least one hundred million. They also have compounding interests and such, and so the numbers would get exceptionally large. I would think that int64 is enough and, so far, it seems to be, but I prefer to be on the safe side and try to detect the upper and the lower bounds. It is not a life-critical application, but some people may bet their life saving on it. If the result is invalid, I need to tell the users not to rely on the number. Due to the complexity of the formula and the number of variables involved, it is difficult to produce a good validation of user inputs so that the result does not overflow. It would be really great if Go has a feature for detecting this, whether it is C#-like checked facility or Ian's new checked types. I prefer that Go returns an error rather than panic. It is easier to work with errors than panic. |
@henryas Sorry, I did not make this clear. If int64 is insufficient then there is a type available providing integers that will not overflow in the "math/big" package (https://pkg.go.dev/math/big). So you can use big.Int (not BigInt that I mentioned above). There is also a big.Rat for fractions. Using these you should never have an overflow or rounding problem. (BTW like all std lib packages this is high quality with good performance.) |
@AndrewWPhillips The team considered that but later determined that int64 is enough. Int64 is more predictable and more natural to work with. Some worry about the possibility of formula bugs when working with numbers in an unnatural way. It is also easier to work with int64 for data persistence. For the sample data they gave us, int64 is enough. However, we still implement the bound checking in case if overflow occurs. Some people are extraordinarily rich, and when you accumulate the wealth of enough affluent people, something like that may happen. Anyhow, I am just highlighting the importance of integer overflow detection and why some people may need it. |
In some languages it is the operations that are checked, wrapping, or saturating. In this proposal, the types determine the behaviour. The operations Checked typesPanic on overflow (in both debug and release builds). If we want the checked types to be the default, what if they were kept short with no explicit suffix or prefix? For example, we could lift the names from Rust.
Would it be possible to check the carry/overflow/etc. flag just once after several operations (add, sub, mul, div, etc.) in an expression? I'm referring to examples like in this other issue: #24853 (comment) My CPU register knowledge is lacking, but it seems like this shouldn't be a huge performance hit?
Yeah. I don't really like the possibility of an unhandled panic crashing the application (or even just the web request). It reminds me of Arianne 5 🙃 . Perhaps that is better than unintentional wrapping, but I'm imagining a lot of defer func() {
if err := recover(); err != nil {
// do something about the error
}
}()
// every time we do some basic math on checked types, unless there's "no way" it could overflow What sort of overflow error would we expect in the panic? Wrapping types
Would these new wrapped types be aliases like The wrapping types could have a suffix/prefix. If these aren't used as frequently, perhaps the names could be somewhat longer? Wondering how it would read.
var a, b wu8 = math.MaxUint8, 1 // or `math.MaxU8` to match
fmt.Println(a + b) // =0, just like now, but wrapping is explicit in the types
var x, y u8 := math.MaxUint8, 1
fmt.Println(wu8(x) + wu8(y)) // explicit type conversions
// would the existing uint/int be aliased with implicit conversions to wrapped types?
func Op(a, b wu8) { ... }
var i, j uint8 := math.MaxUint8, 1
Op(i,j)
Legacy typesIf we introduce new explicitly wrapping types, we'd have all the existing types that are implicitly wrapping. Would we eventually want CaveatsAre there concerns around introducing new identifiers and shadowing? (esp. with short names like This proposal could result in a lot of types, especially if we add wrapping and maybe even saturating types. At least we have generics now. |
Maybe the plan could be:
|
As a simpler alternative to #30209, I propose that we add a new set of integer types that panic on overflow.
First a quick summary of some other languages, to the best of my knowledge:
&+
,&-
,&*
.checked
andunchecked
keywords.Issues #19624 and #30209 argue that in Go integer overflow should panic. But today integer overflow wraps, and we cannot break existing code.
I propose that we add a new set of checked integer types. These will use a prefix
o
, for overflow. The types will beoint
,oint8
,oint16
,oint32
,oint64
,ouint
,ouint8
,ouint16
,ouint32
,ouint64
,ouintptr
. We can also consider adding the type aliasesobyte
(=ouint8
) andorune
(=ouint32
) although I'm not sure they are very important.These new types act exactly like the corresponding types without the
o
prefix, except that if an addition, subtraction, multiplication, or division operation overflows, the result is a run-time panic.Issue #30209 suggests adding a comma-ok form to detect integer overflow in an expression, as is also suggested in #6815. The main advantage of comma-ok for simple expressions is for a simpler implementation of multi-precision arithmetic, but for that use we have instead chosen to provide functions in the math/bits package. The main advantage of comma-ok for complex expressions is to permit switching between checked and wrapping arithmetic, but we already have wrapping arithmetic in the
int
type, and there is no realistic prospect of removing that from the language. So I do not think we need a comma-ok form. If there are other uses of comma-ok, this proposal still supports them, awkwardly, via a deferred function that callsrecover
.There will as usual be no automatic conversion between existing types and the new types. This requirement of Go is the only way that this approach can work.
The text was updated successfully, but these errors were encountered: