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.

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

cmd/compile: incorrectly reports x - minimumValue overflows for integers #68628

Closed
Jorropo opened this issue Jul 29, 2024 · 5 comments
Closed
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@Jorropo
Copy link
Member

Jorropo commented Jul 29, 2024

Go version

go version devel go1.24-0fe775e9f3 Mon Jul 29 05:12:05 2024 +0000 X:rangefunc linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/tmp/go-build'
GOENV='/home/hugo/.config/go/env'
GOEXE=''
GOEXPERIMENT='rangefunc'
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/hugo/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/hugo/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/hugo/k/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/home/hugo/k/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.24-0fe775e9f3 Mon Jul 29 05:12:05 2024 +0000 X:rangefunc'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/hugo/.config/go/telemetry'
GCCGO='/usr/bin/gccgo'
GOAMD64='v3'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/hugo/k/OpenAirways/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build671918439=/tmp/go-build -gno-record-gcc-switches'

What did you do?

compile the following code:

const oneTurn = 1 << 16

type Rot16 uint16

func (x Rot16) Alignement(y Rot16) (alignement int16, reversed bool) {
	alignement = int16(x - y)
	reversed = abs(alignement) > abs(alignement-oneTurn/2)
	if reversed {
		alignement -= oneTurn / 2
	}
	return
}

func abs[T ~int | ~int8 | ~int16 | ~int32 | ~int64 | ~float32 | ~float64](a T) T {
	if a < 0 {
		a = -a
	}
	return a
}

What did you see happen?

./prog.go:9:46: oneTurn / 2 (untyped int constant 32768) overflows int16
./prog.go:11:17: oneTurn / 2 (untyped int constant 32768) overflows int16

What did you expect to see?

Nothing (compiles successfully).

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 29, 2024
@Jorropo
Copy link
Member Author

Jorropo commented Jul 29, 2024

I use a+-b as a workaround.

This might be working as expected as far as the go spec goes, but then I think the language should be changed, reading a+-b feels wrong.

@mknyszek
Copy link
Contributor

CC @golang/compiler

@mknyszek mknyszek added this to the Backlog milestone Jul 29, 2024
@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 29, 2024
@cherrymui
Copy link
Member

As you mentioned, this is working as intended. It is not reporting the result of the subtraction overflows, but the conversion of untyped int 32768 to int16 (which happens before the subtraction) overflows.

a+-b feels unnatural, but for int16, when b=32768, a+b and a-b are actually same. You may as well just write a+b.

@griesemer
Copy link
Contributor

Working as expected as @cherrymui already explained. The - is crucial because it brings 32768 (which is not a valid int16 number) to -32768 (which is a valid int16 number) so the conversion can succeed.

This is just a typical edge case of fixed-width 2's complement arithmetic that one has to be aware of. We're not going to change the language for that. For one, making adjustments in the spec to comprehensively cover such cases would amount to a lot of complexity for very little benefit.

Closing as working as intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants