catenacyber / perfsprint

Golang linter to use strconv
MIT License
19 stars 2 forks source link

Itoa is suggested when arguments formatted are int64 #10

Closed rski closed 9 months ago

rski commented 9 months ago

e.g. in:

foo := int64(math.MaxInt64)
fmt.Sprintf("%d", foo)

https://pkg.go.dev/strconv#Itoa takes an int, which is as at least as big as an int32, meaning it might not hold an int64. For int64, https://pkg.go.dev/strconv#FormatInt should be suggested instead

rski commented 9 months ago

huh, looking at the code, this shouldn't be happening. I'm confused

rski commented 9 months ago

ah, ok I understand. This is specifically for MaxInt64, Int64 variables do suggest FormatInt correctly

catenacyber commented 9 months ago

So, should I close this issue ?

For the input

package main

import (
    "fmt"
    "math"
)

func main() {
    foo := int64(math.MaxInt64)
    fmt.Sprintf("%d", foo)
}

I get fmt.Sprintf can be replaced with faster strconv.FormatInt as expected

rski commented 9 months ago

the problem is this input:

func main() {
    foo := math.MaxInt64
    fmt.Sprintf("%d", foo)
}

which suggests Itoa. in 64 bit this would work, but in 32 bit no.I think maybe untyped int constants need to be compared against maxint32 and suggest FormatInt or Itoa based on that

catenacyber commented 9 months ago

Well, golang stdlib defines math.MaxInt64 as an int It looks like program with math.MaxInt64 cannot compile/run on 32-bit architectures cf https://stackoverflow.com/questions/60392921/why-is-math-maxint64-inferred-as-int32

So perfsprint is working as expected here