Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

strconv: ParseFloat silently assumes bitSize=64 for any value that is not 32 #53868

Closed
colega opened this issue Jul 14, 2022 · 2 comments
Closed

strconv: ParseFloat silently assumes bitSize=64 for any value that is not 32 #53868

colega opened this issue Jul 14, 2022 · 2 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@colega
Copy link
Contributor

colega commented Jul 14, 2022

What version of Go are you using (go version)?

$ go version 1.17.3

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN="/home/oleg/go/bin"
GOCACHE="/home/oleg/.cache/go-build"
GOENV="/home/oleg/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/oleg/go/pkg/mod"
GONOPROXY="github.com/grafana"
GONOSUMDB="github.com/grafana"
GOOS="linux"
GOPATH="/home/oleg/go"
GOPRIVATE="github.com/grafana"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.3"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/oleg/w/github.com/grafana/mimir/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1481112582=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I called strconv.ParseFloat with bitSize=10. It succeeded.

What did you expect to see?

I expected it to fail, as the documentation says it only accepts bitSize=32 and bitSize=64.

What did you see instead?

It didn't fail, but parsed my number.

Why is this an issue

strconv.ParseFloat silently assumes bitSize=64 for any bitSize value that is not 32. This means that changing a call from strconv.ParseFloat("1.1", 16) to strconv.ParseFloat("1.1", 32) actually decreases the precision instead of increasing it.

In my particular case, I made a similar change (suggested by a linter, because honestly, I didn't know about bitSize limitation), and it resulted in a flaky test because of the precision decrease.

Further thoughts

I am not brave enough to open a PR returning an error when strconv.ParseFloat() is called with bitSize values that are not 32/64, as I feel that woud break the internet at this point, however I do think that the function should either comply with it's comment, fail, or at least have this behavior documented.
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 14, 2022
@mknyszek mknyszek added this to the Backlog milestone Jul 14, 2022
@mknyszek
Copy link
Contributor

CC @rsc @griesemer @ianlancetaylor via https://dev.golang.org/owners

@ianlancetaylor
Copy link
Contributor

We tried to fix this in https://go.dev/cl/248219 for #40706. However, that led to #42297, which showed that enforcing the documented rules caused too many problems with existing code. We considered a vet check in #42389, but decided against it.

We could update the documentation, but I think that in general we should prefer that people pass the correct bit size, even though other sizes happen to work.

So, we aren't going to do anything here. Sorry. Thanks for reporting the problem.

@ianlancetaylor ianlancetaylor closed this as not planned Won't fix, can't repro, duplicate, stale Jul 14, 2022
@golang golang locked and limited conversation to collaborators Jul 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@mknyszek @colega @ianlancetaylor @gopherbot