go: reflect: Overflow methods should be on Type

See these four methods on reflect.Value:

For example, looking at OverflowInt’s godoc:

OverflowInt reports whether the int64 x cannot be represented by v’s type. It panics if v’s Kind is not Int, Int8, Int16, Int32, or Int64.

None of that needs a value, in my opinion - a type would suffice. The implementation seems to confirm that - only the “kind” and “type” are used, not the actual value.

So I think the methods should be added to Type, with the same signature.

I noticed this because quite a few projects have code like reflect.Zero(typ).OverflowInt(x), when in fact it could just be typ.OverflowInt(x), saving the creation of a new value. For example, in encoding/json: https://cs.opensource.google/go/go/+/refs/tags/go1.20.4:src/encoding/json/decode.go;l=799

As for users who have a reflect.Value rather than a reflect.Type, they could also replace val.OverflowInt(x) with val.Type().OverflowInt(x), and we could consider deprecating the Value methods in favor of the Type methods after a few Go releases. I don’t personally see a point in keeping eight methods around; .Type() isn’t enough characters to warrant a shortcut, and in my experience, most reflect users will need to get the reflect.Type for other reasons.

About this issue

Original URL
State: open
Created a year ago
Reactions: 11
Comments: 16 (16 by maintainers)

Most upvoted comments

It may also be implemented as a single generic function:

// Overflow reports whether x cannot be represented by type t.
func Overflow[T constraints.Integer | constraints.Float | constraints.Complex](t Type, x T) bool

and used in this way:

reflect.Overflow(t, x)

gazerro on May 25, 2023

@gazerro I’d prefer reflect to stay relatively consistent. We don’t use top-level functions for this kind of stuff so far. And using generics wouldn’t add type-safety here and I’d rather four methods on Type than one package-scoped function, in terms of namespace pollution. YMMV.

Merovius on May 25, 2023

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

Adding to package reflect:

// OverflowComplex reports whether the complex128 x cannot be represented by type t. 
// It panics if t's Kind is not Complex64 or Complex128.
func (t Type) OverflowComplex(x complex128) bool

// OverflowFloat reports whether the float64 x cannot be represented by type t. 
// It panics if t's Kind is not Float32 or Float64.
func (t Type) OverflowFloat(x float64) bool

// OverflowInt reports whether the int64 x cannot be represented by type t. 
// It panics if t's Kind is not Int, Int8, Int16, Int32, or Int64.
func (t Type) OverflowInt(x int64) bool

// OverflowUint reports whether the uint64 x cannot be represented by type t. 
// It panics if t's Kind is not Uint, Uintptr, Uint8, Uint16, Uint32, or Uint64.
func (t Value) OverflowUint(x uint64) bool

rsc on Oct 26, 2023

This seems straightforward. I believe the new API is:

// OverflowComplex reports whether the complex128 x cannot be represented by type t. 
// It panics if t's Kind is not Complex64 or Complex128.
func (t Type) OverflowComplex(x complex128) bool

// OverflowFloat reports whether the float64 x cannot be represented by type t. 
// It panics if t's Kind is not Float32 or Float64.
func (t Type) OverflowFloat(x float64) bool

// OverflowInt reports whether the int64 x cannot be represented by type t. 
// It panics if t's Kind is not Int, Int8, Int16, Int32, or Int64.
func (t Type) OverflowInt(x int64) bool

// OverflowUint reports whether the uint64 x cannot be represented by type t. 
// It panics if t's Kind is not Uint, Uintptr, Uint8, Uint16, Uint32, or Uint64.
func (t Value) OverflowUint(x uint64) bool

Do I have that right?

rsc on Oct 24, 2023

I’ll add it to the list to review soonish.

ianlancetaylor on Aug 17, 2023