proposal: reflect: add Overflows method on reflect.Value

[katiehockman]

The proposal is to add reflect.Value.Overflows(reflect.Type) bool to the reflect package. This method on reflect.Value types would basically be a more strict version of reflect.Type.ConvertibleTo(reflect.Type), returning false if an overflow or precision loss would occur when converting reflect.Value to the given reflect.Type.

For example:

var y int32
x := reflect.Value(math.MaxInt64)
fmt.Println(x.Overflows(reflect.TypeOf(y))) // prints true
x = reflect.Value(5)
fmt.Println(x.Overflows(reflect.TypeOf(y))) // prints false

or

var y uint
x := reflect.Value(-1)
fmt.Println(x.Overflows(reflect.TypeOf(y))) // prints true
x = reflect.Value(1)
fmt.Println(x.Overflows(reflect.TypeOf(y))) // prints false

Why is reflect.Type.ConvertibleTo(reflect.Type) not enough?
reflect.Type.ConvertibleTo(reflect.Type) is helpful in understanding whether a type can be converted to another, but the value is required to know whether there will be precision loss or overflow. It's also been noted that ConvertibleTo alone can't be used to determine whether a type is safe to convert to another, as reflect.Value.Convert(reflect.Type) can still panic in some circumstances.

The real world use case came up during discussion of https://golang.org/cl/325702

credit to @rolandshoemaker who came up with the API

[robpike]

Perhaps this should not be in reflect, as the concept is important and often relevant to tight code.

[ianlancetaylor]

See also #46746.

[ianlancetaylor]

You suggest checking for loss of precision. What should happen when converting 1.1 to int? Should this return false for any value that is not precisely representable in the destination type? If yes, perhaps Overflows is not the best name. If no, do we ever care about such cases? And how should we handle 2147483647.1 converted to int32?

[josharian]

The usual Go idiom for this is doing a round trip conversion and then checking for equality: T(U(x)) == x. This might be a good way to define the semantics of Overflow, perhaps with a different name. This also has a natural (if verbose and expensive) translation into package reflect, after #46746.

(That is for concrete values. For types, you'd define it whether the round trip succeeds for all possible values.)

[dsnet]

That doesn't catch cases where the semantic value has changed:

x := int8(-1)
int8(uint64(x)) == x // reports true even though x would be converted to math.MaxUint64, which is not semantically equivalent to -1

I think part of the challenge with this API is that people are going to reasonably disagree about what is considered an "overflow" or "loss of precision".

[katiehockman]

What should happen when converting 1.1 to int? Should this return false for any value that is not precisely representable in the destination type? If yes, perhaps Overflows is not the best name.

That's a good question. I would lean towards returning false. I agree that Overflows might not be the best name in that case though.

Alternatively, we could add a new method on reflect.Value that can check "semantic" equivalence, which can be used to see whether the conversion was successful. Something like reflect.Value.EquivalentTo(reflect.Value) which returns true if the two values are semantically equivalent, i.e. int(5) is semantically equivalent to float64(5) but not to float64(5.5).

For example:

var y int

x := reflect.Value(1.1)
converted := x.Convert(reflect.TypeOf(y))
fmt.Println(x.EquivalentTo(converted) // prints false

x = reflect.Value(1)
converted = x.Convert(reflect.TypeOf(y))
fmt.Println(x.EquivalentTo(converted) // prints true

That gets a little trickier for things like string("5") and int(5), which I would argue should return false, though at least that wouldn't pass ConvertibleTo checks.

[bcmills]

I'm going to say basically the same thing here that I said on #48218. To me, reflect should provide only (and exactly) the operations that can be performed in ordinary Go code, with just enough extra API for callers to detect when the operation they're about to perform wouldn't be allowed in ordinary Go code.

The existing reflect.Value.OverflowInt method can be used to detect what would normally be a compile-time error, like var x int8 = 512.

In contrast, I think that the Overflows method proposed here would detect semantic changes that would not be rejected at run-time, and cannot not be detected in a generic way at run-time in an ordinary Go program. I think we should add this API to reflect if (and only if) we add an equivalent operation to the language proper (such as the _, ok form described in part 2 of #30209).

I do think that the proposed overflow-check function would be a useful library — I just don't think it belongs in the reflect package proper at this point.

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[rsc]

It sounds like there are enough subtleties here that we should probably not add this. Do I have that right?

[rsc]

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

[rsc]

No change in consensus, so declined.
— rsc for the proposal review group