golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
120.1k stars 17.24k forks source link

proposal: strconv: add generic integer variants #67110

Open Jorropo opened 2 weeks ago

Jorropo commented 2 weeks ago

Proposal Details

I had an argument with @ldemailly on what is the least ugly to stringify a ~uint8 (typed iota enum) to a base ten number. strconv.FormatUint(uint64(i), 10) is verbose so it seems people would enjoy using strconv.Itoa more, however Itoa is easy to use incorrectly, it may truncate on 32 bits platforms and is signed which might or might not be what you want to do. (search 13k results, this is popular)

I propose theses new functions:

// if new parse / format functions for new integers types are added, they would also be added there.
type integers interface{ uint | uint8 | uint16 | uint32 | uint64 | uintptr | int | int8 | int16 | int32 | int64 }

// ParseInteger is like [ParseInt] and [ParseUint], but it will error if s can't be parsed perfectly into T.
// It accept 2 <= base <= 36 or 0, see [ParseInt] for base 0 documentation.
func ParseInteger[T ~integers](s string, base int) (T, error)

// FormatInteger is like [FormatInt] and [FormatUint], it accept 2 <= base <= 36.
func FormatInteger[T ~integers](v T, base int) string

The idea is you don't bother thinking about what the best function is, what type conversion you need to do or what bitsize should be set to. You just call ParseInteger or FormatInteger with the type you want, and let the type checker handles the tricky bits for you.

zephyrtronium commented 2 weeks ago

See also #57975 which proposed Parse[T] for a larger set of types.

Jorropo commented 2 weeks ago

Thx, I've updated the proposal to only include ParseInteger and FormatInteger because the list of types usable in Format and Parse is arbitrary and some similar list has been refused in #57975.

I don't see rational behind the useless generics argument here. Generics can be way faster than using reflect, performance will likely not improve over current strconv, but I don't want a tradeoff between readability plus « pit of success » API however it gains an extra heap allocation.

Finally implementing it can be do with foldable types checking tricks thx there can only be integers, no need for reflect.

gophun commented 2 weeks ago

it can be do with foldable types checking tricks

What are "foldable" types and what are their tricks? I have never heard this term in the context of Go.

Jorropo commented 2 weeks ago

@gophun for example see signTest in https://go-review.googlesource.com/c/exp/+/461016

earthboundkid commented 2 weeks ago

I still feel like I did on #57975: parsing any type is too much, but a narrowly tailored integer parser would be useful. I prefer a pointer-based API because you can omit [T], so

// if new parse / format functions for new integers types are added, they would also be added there.
type integers interface{ uint | uint8 | uint16 | uint32 | uint64 | uintptr | int | int8 | int16 | int32 | int64 }

// Integer is like calling the matching Parse${T} with bitSize being the type's size.
func Integer[T ~integers](ptr *T, v string, base int) error

// FormatInteger is like calling the matching Format${T}, it accept 2 <= base <= 36.
func FormatInteger[T ~integers](v T, base int) string

// Usage
n := defaultValue
strconv.Integer(&n, s, 10) // Can ignore the return value if your usecase just falls back to defaultValue
Jorropo commented 2 weeks ago

@earthboundkid maybe, looks subjective to me, FWIW it's more tokens. But as long it's not a runtime any I don't care.

gopherbot commented 2 weeks ago

Change https://go.dev/cl/582575 mentions this issue: strconv: implement ParseInteger and FormatInteger

Jorropo commented 2 weeks ago

I implemented this in https://go-review.googlesource.com/c/go/+/582575 to prove this completely dodge questions about import cycles and reflect.

earthboundkid commented 2 weeks ago

@earthboundkid maybe, looks subjective to me, FWIW it's more tokens. But as long it's not a runtime any I don't care.

The version without a pointer comes out significantly longer for the not uncommon case of not caring about the error:

n := defaultValue // let's say this is an int32
if v, err := strconv.Integer[int32](s, 10); err == nil {
    n = v
}
earthboundkid commented 2 weeks ago

I implemented this in https://go-review.googlesource.com/c/go/+/582575 to prove this completely dodge questions about import cycles and reflect.

Another way to do it would be to get the unsafe.Sizeof, no?

Edit: Playground

Jorropo commented 2 weeks ago

You can use unsafe but it has bad press and is a new import. We can decide on implementation later this aimed to prove you only need the go spec to do this.

jimmyfrasche commented 2 weeks ago

60274 would be useful here.

The pointer version of

n := defaultValue // let's say this is an int32
if v, err := strconv.Integer[int32](s, 10); err == nil {
    n = v
}

is

n := defaultValue
var v int32
if strconv.Integer(&v, s, 10) == nil {
    n = v
}

so you're not saving a great deal unless v is already defined (struct field, etc.)

The first looks clearer to me and the second makes it easier to write

strconv.Integer(&v, "oops, always zero!", 10)
earthboundkid commented 2 weeks ago

If it’s using a pointer and there’s an error, it shouldn’t write a zero into the pointer. It should just leave it alone. So then you can drop the whole if statement and assignment.

earthboundkid commented 2 weeks ago

You can use unsafe but it has bad press and is a new import.

See https://github.com/golang/go/issues/29982 which would move Sizeof out of unsafe.