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/go: per-file Go versions via //go:build do not play well with goexperiment #66399

Open
mvdan opened this issue Mar 19, 2024 · 1 comment
Open
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@mvdan
Copy link
Member

mvdan commented Mar 19, 2024

Go version

go version devel go1.23-8496060870 2024-03-19 07:29:47 +0000 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/mvdan/.cache/go-build'
GOENV='/home/mvdan/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/mvdan/go/pkg/mod'
GONOPROXY='github.com/cue-unity'
GONOSUMDB='github.com/cue-unity'
GOOS='linux'
GOPATH='/home/mvdan/go'
GOPRIVATE='github.com/cue-unity'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/mvdan/tip'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/mvdan/tip/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.23-8496060870 2024-03-19 07:29:47 +0000'
GODEBUG=''
GCCGO='gccgo'
GOAMD64='v3'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
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-build4235802855=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Testscript below, run via https://pkg.go.dev/github.com/rogpeppe/go-internal@v1.12.0/cmd/testscript:

go run ./tag_lang

go run ./tag_lang_experiment

-- go.mod --
module test

go 1.22
-- tag_lang/tag.go --
//go:build go1.23

package main

func count(n int) func(yield func(i int) bool) {
	return func(yield func(i int) bool) {
		i := 0
		for i < n && yield(i) {
			i++
		}
	}
}

func main() {
	for i := range count(5) {
		println(i)
	}
}
-- tag_lang_experiment/tag.go --
//go:build go1.23 || goexperiment.rangefunc

package main

func count(n int) func(yield func(i int) bool) {
	return func(yield func(i int) bool) {
		i := 0
		for i < n && yield(i) {
			i++
		}
	}
}

func main() {
	for i := range count(5) {
		println(i)
	}
}

What did you see happen?

$ testscript -v repro.txtar 
[...]
> go run ./tag_lang
[stderr]
0
1
2
3
4

> go run ./tag_lang_experiment
[stderr]
# test/tag_lang_experiment
tag_lang_experiment/tag.go:15:17: cannot range over count(5) (value of type func(yield func(i int) bool)): requires go1.23 or later (-lang was set to go1.22; check go.mod)

[exit status 1]
FAIL: /tmp/testscript552503763/repro.txtar/script.txtar:3: unexpected go command failure
error running repro.txtar in /tmp/testscript552503763/repro.txtar

What did you expect to see?

Both should work the same.

Note that this is technically working as expected given #59033. https://pkg.go.dev/go/build/constraint#GoVersion on go1.23 gives go1.23, but for go1.23 || goexperiment.rangefunc gives no result, so cmd/go falls back to go 1.22 from go.mod.

I reckon this is a sharp edge that we should polish; cmd/go should be able to figure out that, since either build tag must be true with go1.23 || goexperiment.rangefunc, the range-over-func language feature should be enabled. I think the simplest way to solve this would be to have the go1.23 build tag imply the goexperiment.rangefunc build tag, alongside a default of GOEXPERIMENT=rangefunc being set. In the same setup, using //go:build goexperiment.rangefunc with GOEXPERIMENT=rangefunc go run works, after all.

The reason I ran into this is because we have a library that was written in a way to be compatible with iterator functions, i.e. Go funcs and methods return yield funcs today, even though we only support Go 1.21 and 1.22, where range-over-func isn't available yet. But we want to double check that range-over-func will work with those APIs once it's live, so we use a build-tagged file with go1.23 || goexperiment.rangefunc to briefly test that the range-over-func would compile and run as expected. Having the experiment build tag there is useful so it can be tested on Go 1.22 by enabling the experiment, and not just by installing Go 1.23 from master.

I reckon this is fairly niche and perhaps low priority, but given that there likely will be multiple more language changes implemented in a similar way with GOEXPERIMENT (#57001), I think polishing this sharp edge would make it a lot easier to trial the new features before they are stabilized.

@dr2chase
Copy link
Contributor

@rsc

@thanm thanm added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

3 participants