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

doc: document closure inlining affects function PC comparison #45781

Open
rski opened this issue Apr 26, 2021 · 4 comments
Open

doc: document closure inlining affects function PC comparison #45781

rski opened this issue Apr 26, 2021 · 4 comments

Comments

@rski
Copy link

@rski rski commented Apr 26, 2021

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

$ go version
go version devel go1.17-d5d24dbe41 Mon Apr 26 17:13:36 2021 +0000 linux/amd64

Does this issue reproduce with the latest release?

only master

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

go env Output
$ go env
~/go/src/playground/at-2021-04-26-204402 $ ~/Code/go/bin/go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/rski/.cache/go-build"
GOENV="/home/rski/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/rski/go/pkg/mod"
GONOPROXY=""
GONOSUMDB="="
GOOS="linux"
GOPATH="/home/rski/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/rski/Code/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/rski/Code/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel go1.17-d5d24dbe41 Mon Apr 26 17:13:36 2021 +0000"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="0"
GOMOD="/home/rski/go/src/playground/at-2021-04-26-204402/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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2782832869=/tmp/go-build -gno-record-gcc-switches"

What did you do?

https://play.golang.org/p/Yw1lgRph4Kd

What did you expect to see?

This should print main.WithFoo.func1, as with go 16.3

What did you see instead?

After the regabi branch merge, this prints

main.init.func1

This was temporarily fixed by 1129a60, which disabled inlining of functions that contain closures.

@rski rski changed the title dev.regabi merged changed reflect behaviour in the presense of inlined functions dev.regabi merge changed reflect behaviour in the presense of inlined functions Apr 26, 2021
@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Apr 26, 2021

cc @danscales for inlining closures.

This is working as intended. We probably need to document it better.

@rski
Copy link
Author

@rski rski commented Apr 26, 2021

This is working as intended. interesting. The reason I figured out this happened is because it broke some code. It uses reflect like this to figure out if a test was passed a specific opt function, and the inlined version no longer compares equal with the non inlined version. Is this use pushing it and not covered by the compatibility guarantee?

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Apr 26, 2021

Comparing PCs of functions is never intended to work (e.g. func values are incomparable). It may happen to work in some cases, but not guaranteed to work.

@rski
Copy link
Author

@rski rski commented Apr 26, 2021

heh, I'm surprised we got away with it for so many go versions then. Thanks for the quick response, I'll replace the code with something less brittle.

@cherrymui cherrymui changed the title dev.regabi merge changed reflect behaviour in the presense of inlined functions doc: document closure inlining affects function PC comparison Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants