proposal: cmd/vet: consider adding check for incorrect bitSize on strconv.ParseFloat (and similar functions) #42389
Comments
ianlancetaylor
changed the title
|
CC @robpike |
|
Strikes me as possibly above the bar but marginally at best, and over time I expect the frequency to drop. Instead I might advocate a single push to clean up all the public instances. Should be easy to find. |
|
@benhoyt I'm not opposed to "a single push to clean up all the public instances" (though that can be a very time-consuming process to drive: N pull requests across N repositories with N different styles of contributing). However, why do you expect the frequency of this to drop over time? I'd expect it to be constant as this seems to be a copy-n-paste type of mistake (or even grow, given with increased Go usage more newbies are copying-and-pasting stuff). That said, I don't mind scrapping the vet check, either. With the |
|
I don't see much evidence for it being a copy-and-paste error, but if you say it is I have no reason to doubt you. In any case, since passing 10 instead of 64 has no effect, is it even a bug? |
|
It being a copy and paste error was an educated guess, based (so to speak) on the fact that 10 is usually the second argument in a ParseInt call, so likely copied from there. In any case, yeah, I think now that the dust has settled and because of the tests we've added, this is a mistake but not a bug. Closing. Was fun to write a vet check though. :-) |
There is a non-trivial amount of code that calls
strconv.ParseFloatwith abitSizeof 10 (or even 0) instead of 32 or 64, presumably due to a copy-n-paste error fromParseInt, or thinking the second argument was a base instead of a bit size. We were going to return an error in these cases, but doing that would probably break existing code, so we went back on that (issue #42297 has more context and code links).Instead of returning an error, I've put up CL 267600 with a go/analysis pass (intended for "go vet") that checks for this. My questions (copied from #42297) are:
ParseFloatandParseComplex. Do we want to add checks for other functions that take abitSizeas well? It'll be easy to add:AppendFloat(32, 64),FormatComplex(64, 128),FormatFloat(32, 64),ParseInt,ParseUint(0, 8, 16, 32, 64).The text was updated successfully, but these errors were encountered: