go/types, cmd/go: breaking change in 1.23rc2 with version constraints in GOPATH mode [1.22 backport]

[timothy-king]

@taking requested issue #68658 to be considered for backport to the next 1.22 minor release.

@gopherbot please backport this to 1.22. It's necessary to have consistent file versions with build tags with later versions.

[gabyhelp]

Related Issues and Documentation

• cmd/go: breaking change in 1.23rc2 with version constraints in GOPATH mode [1.23 backport] #69094 (closed)
• cmd/compile: Go 1.22 changes support for modules that declare `go 1.0` [1.22 backport] #65619 (closed)
• cmd/go: module loader looks for the wrong tags if the "go-import" prefix includes the major-version suffix [1.12 backport] #30665 (closed)
• cmd/go: error if GOCACHE is set but is not an absolute path [1.12 backport] #30493 (closed)
• cmd/go: breakage with go version command and GOFLAGS environment variable [1.15 backport] #41464 (closed)
• os/exec: calling Cmd.Start after setting Cmd.Path manually to absolute path without ".exe" no longer implicitly adds ".exe" in Go 1.22 [1.22 backport] #66598 (closed)
• cmd/fix: invalid -go=go1.21.0 [1.23 backport] [1.22 backport] #68827 (closed)
• cmd/fix: invalid -go=go1.21.0 [1.23 backport] #68826 (closed)
• cmd/go: TestScript/gotoolchain_issue66175 fails on tip locally [1.22 backport] #67311 (closed)
• cmd/go: 'go get […]/[email protected]' fails when the go.mod file for […]/v2 is at the repo root [1.13 backport] #34497 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[timothy-king]

CC @griesemer .

https://go.dev/cl/615685 has some helpful context on why FileVersions from go/types has a different meaning in 1.22 and should maybe be updated.

[gabyhelp]

Related Issues and Documentation

• cmd/go: breaking change in 1.23rc2 with version constraints in GOPATH mode [1.23 backport] #69094 (closed)
• cmd/compile: Go 1.22 changes support for modules that declare `go 1.0` [1.22 backport] #65619 (closed)
• cmd/go: module loader looks for the wrong tags if the "go-import" prefix includes the major-version suffix [1.12 backport] #30665 (closed)
• cmd/go: error if GOCACHE is set but is not an absolute path [1.12 backport] #30493 (closed)
• cmd/go: breakage with go version command and GOFLAGS environment variable [1.15 backport] #41464 (closed)
• os/exec: calling Cmd.Start after setting Cmd.Path manually to absolute path without ".exe" no longer implicitly adds ".exe" in Go 1.22 [1.22 backport] #66598 (closed)
• cmd/fix: invalid -go=go1.21.0 [1.23 backport] [1.22 backport] #68827 (closed)
• cmd/fix: invalid -go=go1.21.0 [1.23 backport] #68826 (closed)
• cmd/go: TestScript/gotoolchain_issue66175 fails on tip locally [1.22 backport] #67311 (closed)
• cmd/go: 'go get […]/[email protected]' fails when the go.mod file for […]/v2 is at the repo root [1.13 backport] #34497 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[timothy-king]

Example that shows the discontinuity between 1.22 and 1.23:

package main

import (
	"fmt"
	"go/ast"
	"go/importer"
	"go/parser"
	"go/token"
	"go/types"
)

func main() {
	contents := `

//go:build go1.18

// FileVersion == "go1.18"  according to go 1.22, 1.21.
// FileVersion == "go1.21"  according to go 1.23, tip.

package p

func p[T any, S []T](x T) {}

func f() {
	var f func(int) = p // Use a 1.21 feature.
	print(f)
}
	`

	fset := token.NewFileSet()
	file, err := parser.ParseFile(fset, "p.go", contents, 0)
	if err != nil {
		panic(err)
	}

	conf := types.Config{
		Importer:  importer.Default(),
		GoVersion: "go1.21",
	}
	info := types.Info{}
	_, err = conf.Check("", fset, []*ast.File{file}, &info)

	fmt.Println(err)
}

1.23 says this type checks. 1.22 says this does not type check with the following error:

p.go:9:20: implicitly instantiated function in assignment requires go1.21 or later

[timothy-king]

https://go.dev/cl/617635 has more examples where go/types.Info.FileVersions has a different meaning in 1.22.

Back porting would mean FileVersions would have a single consistent semantics going forward. This is also not a trivial backport or an issue that impacts many users directly. It may not be worth the risk.

But THB if someone came up and asked me how to use go/types.Info.FileVersions correctly, I would at this point tell them to skip to 1.23 or to only use it for the specific check 'is this file >= 1.22 or unknown (until you stop supporting 1.22)'. This way they could skip needing to specialize for both cases.

So we need to decide if we are going to accept go/types producing different FileVersion values as Unfortunate or to back port.

[timothy-king]

Moving from cmd/go review status to 'waiting on weekly review' and removing from CLI triage. This is a go/types decision.

[cherrymui]

Discussed in the release meeting, we thought that that this is a subtle edge case that doesn’t seem to be causing problems for most users (if one wants 1.21 feature, they probably shouldn't use 1.18 build tag). So this is mostly potentially for tool authors? But we haven’t heard external reports so far. So might not be worth the risk of backporting fairly late in the 1.22 cycle.

@timothy-king what do you think? Thanks.

[timothy-king]

So with some additional prodding and admittedly very strange choices, one can lift this to a compiler problem:

-- go.mod --
module example.com/m

go 1.21
-- main.go --
//go:build go1.16

package main

func P[T any]() {} // uses a go 1.18 feature

func main() {
        println("ran")
}

This builds with go1.20.14 and go1.23.1 while it fails to build with go1.21.13 and go1.22.7:

% go1.20.14 build; ./m
ran
% go1.21.13 build 
# example.com/m
./m.go:5:8: type parameter requires go1.18 or later (-lang was set to go1.21; check go.mod)
./m.go:5:10: predeclared any requires go1.18 or later (-lang was set to go1.21; check go.mod)
% go1.22.7 build
# example.com/m
./m.go:5:8: type parameter requires go1.18 or later (file declares //go:build go1.16)
./m.go:5:10: predeclared any requires go1.18 or later (file declares //go:build go1.16)
% go1.23.1 build; ./m
ran

There are multiple odd choices in this example: a 1.16 build tag, using a 1.18 feature, and using go1.20.14 with a go1.21 module. But still not backwards compatible in the strictest sense. A working example stopped compiling even if it is an edge case.

TBH the bit I really want to fix is that I want a simpler world where FileVersions has one meaning and the current 1.22 meaning is a bug. This is an obnoxious bit of tech debt I would like to get rid of.

It may not be worth the risk though as you said. I'll let other decide.

[cherrymui]

Yes, this can leads to a compilation failure. But using 1.18 feature with a 1.16 build tag seems a bad choice, and is probably really are. It doesn't seem too bad that this doesn't compile with Go 1.22, given that this would also not compile with Go 1.21.

[prattmic]

Given the lack of user-reported issues, we've decided not to backport.