From 7afe17bbdb961df3a7163f4d725bedc1c008571f Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Fri, 16 Aug 2024 15:13:39 -0400 Subject: [PATCH] [release-branch.go1.23] go/types, types2: use max(fileVersion, go1.21) if fileVersion present Change the rules for how //go:build "file versions" are applied: instead of considering whether a file version is an upgrade or downgrade from the -lang version, always use max(fileVersion, go1.21). This prevents file versions from downgrading the version below go1.21. Before Go 1.21 the //go:build version did not have the meaning of setting the file's langage version. This fixes an issue that was appearing in GOPATH builds: Go 1.23.0 started providing -lang versions to the compiler in GOPATH mode (among other places) which it wasn't doing before, and it set -lang to the toolchain version (1.23). Because the -lang version was greater than go1.21, language version used to compile the file would be set to the //go:build file version. //go:build file versions below 1.21 could cause files that could previously build to stop building. For example, take a Go file with a //go:build line specifying go1.10. If that file used a 1.18 feature, that use would compile fine with a Go 1.22 toolchain. But it would produce an error when compiling with the 1.23.0 toolchain because it set the language version to 1.10 and disallowed the 1.18 feature. This breaks backwards compatibility: when the build tag was added, it did not have the meaning of restricting the language version. For #68658 Fixes #69094 Change-Id: I6cedda81a55bcccffaa3501eef9e2be6541b6ece Reviewed-on: https://go-review.googlesource.com/c/go/+/607955 LUCI-TryBot-Result: Go LUCI Reviewed-by: Robert Griesemer (cherry picked from commit aeac0b6cbfb42bc9c9301913a191bb09454d316a) Reviewed-on: https://go-review.googlesource.com/c/go/+/608935 --- src/cmd/compile/internal/types2/api_test.go | 50 ++++++++++++++----- src/cmd/compile/internal/types2/check.go | 47 +++++++---------- src/go/types/api_test.go | 50 ++++++++++++++----- src/go/types/check.go | 48 ++++++++---------- .../types/testdata/check/go1_20_19.go | 2 +- .../types/testdata/check/go1_21_19.go | 2 +- .../types/testdata/check/go1_21_22.go | 16 ++++++ .../types/testdata/check/go1_22_21.go | 16 ++++++ .../types/testdata/fixedbugs/issue66285.go | 7 +-- test/fixedbugs/issue63489a.go | 20 +++++--- test/fixedbugs/issue63489b.go | 15 ++++-- 11 files changed, 174 insertions(+), 99 deletions(-) create mode 100644 src/internal/types/testdata/check/go1_21_22.go create mode 100644 src/internal/types/testdata/check/go1_22_21.go diff --git a/src/cmd/compile/internal/types2/api_test.go b/src/cmd/compile/internal/types2/api_test.go index 5126ac51116cd..a6b105ace5cc3 100644 --- a/src/cmd/compile/internal/types2/api_test.go +++ b/src/cmd/compile/internal/types2/api_test.go @@ -2898,22 +2898,48 @@ func TestFileVersions(t *testing.T) { fileVersion string wantVersion string }{ - {"", "", ""}, // no versions specified - {"go1.19", "", "go1.19"}, // module version specified - {"", "go1.20", ""}, // file upgrade ignored - {"go1.19", "go1.20", "go1.20"}, // file upgrade permitted - {"go1.20", "go1.19", "go1.20"}, // file downgrade not permitted - {"go1.21", "go1.19", "go1.19"}, // file downgrade permitted (module version is >= go1.21) + {"", "", ""}, // no versions specified + {"go1.19", "", "go1.19"}, // module version specified + {"", "go1.20", "go1.21"}, // file version specified below minimum of 1.21 + {"go1", "", "go1"}, // no file version specified + {"go1", "goo1.22", "go1"}, // invalid file version specified + {"go1", "go1.19", "go1.21"}, // file version specified below minimum of 1.21 + {"go1", "go1.20", "go1.21"}, // file version specified below minimum of 1.21 + {"go1", "go1.21", "go1.21"}, // file version specified at 1.21 + {"go1", "go1.22", "go1.22"}, // file version specified above 1.21 + {"go1.19", "", "go1.19"}, // no file version specified + {"go1.19", "goo1.22", "go1.19"}, // invalid file version specified + {"go1.19", "go1.20", "go1.21"}, // file version specified below minimum of 1.21 + {"go1.19", "go1.21", "go1.21"}, // file version specified at 1.21 + {"go1.19", "go1.22", "go1.22"}, // file version specified above 1.21 + {"go1.20", "", "go1.20"}, // no file version specified + {"go1.20", "goo1.22", "go1.20"}, // invalid file version specified + {"go1.20", "go1.19", "go1.21"}, // file version specified below minimum of 1.21 + {"go1.20", "go1.20", "go1.21"}, // file version specified below minimum of 1.21 + {"go1.20", "go1.21", "go1.21"}, // file version specified at 1.21 + {"go1.20", "go1.22", "go1.22"}, // file version specified above 1.21 + {"go1.21", "", "go1.21"}, // no file version specified + {"go1.21", "goo1.22", "go1.21"}, // invalid file version specified + {"go1.21", "go1.19", "go1.21"}, // file version specified below minimum of 1.21 + {"go1.21", "go1.20", "go1.21"}, // file version specified below minimum of 1.21 + {"go1.21", "go1.21", "go1.21"}, // file version specified at 1.21 + {"go1.21", "go1.22", "go1.22"}, // file version specified above 1.21 + {"go1.22", "", "go1.22"}, // no file version specified + {"go1.22", "goo1.22", "go1.22"}, // invalid file version specified + {"go1.22", "go1.19", "go1.21"}, // file version specified below minimum of 1.21 + {"go1.22", "go1.20", "go1.21"}, // file version specified below minimum of 1.21 + {"go1.22", "go1.21", "go1.21"}, // file version specified at 1.21 + {"go1.22", "go1.22", "go1.22"}, // file version specified above 1.21 // versions containing release numbers // (file versions containing release numbers are considered invalid) {"go1.19.0", "", "go1.19.0"}, // no file version specified - {"go1.20", "go1.20.1", "go1.20"}, // file upgrade ignored - {"go1.20.1", "go1.20", "go1.20.1"}, // file upgrade ignored - {"go1.20.1", "go1.21", "go1.21"}, // file upgrade permitted - {"go1.20.1", "go1.19", "go1.20.1"}, // file downgrade not permitted - {"go1.21.1", "go1.19.1", "go1.21.1"}, // file downgrade not permitted (invalid file version) - {"go1.21.1", "go1.19", "go1.19"}, // file downgrade permitted (module version is >= go1.21) + {"go1.20.1", "go1.19.1", "go1.20.1"}, // invalid file version + {"go1.20.1", "go1.21.1", "go1.20.1"}, // invalid file version + {"go1.21.1", "go1.19.1", "go1.21.1"}, // invalid file version + {"go1.21.1", "go1.21.1", "go1.21.1"}, // invalid file version + {"go1.22.1", "go1.19.1", "go1.22.1"}, // invalid file version + {"go1.22.1", "go1.21.1", "go1.22.1"}, // invalid file version } { var src string if test.fileVersion != "" { diff --git a/src/cmd/compile/internal/types2/check.go b/src/cmd/compile/internal/types2/check.go index 91ad474e9df31..ada421ba939ed 100644 --- a/src/cmd/compile/internal/types2/check.go +++ b/src/cmd/compile/internal/types2/check.go @@ -327,7 +327,6 @@ func (check *Checker) initFiles(files []*syntax.File) { check.errorf(files[0], TooNew, "package requires newer Go version %v (application built with %v)", check.version, go_current) } - downgradeOk := check.version.cmp(go1_21) >= 0 // determine Go version for each file for _, file := range check.files { @@ -336,33 +335,18 @@ func (check *Checker) initFiles(files []*syntax.File) { // unlike file versions which are Go language versions only, if valid.) v := check.conf.GoVersion - fileVersion := asGoVersion(file.GoVersion) - if fileVersion.isValid() { - // use the file version, if applicable - // (file versions are either the empty string or of the form go1.dd) - if pkgVersionOk { - cmp := fileVersion.cmp(check.version) - // Go 1.21 introduced the feature of setting the go.mod - // go line to an early version of Go and allowing //go:build lines - // to “upgrade” (cmp > 0) the Go version in a given file. - // We can do that backwards compatibly. - // - // Go 1.21 also introduced the feature of allowing //go:build lines - // to “downgrade” (cmp < 0) the Go version in a given file. - // That can't be done compatibly in general, since before the - // build lines were ignored and code got the module's Go version. - // To work around this, downgrades are only allowed when the - // module's Go version is Go 1.21 or later. - // - // If there is no valid check.version, then we don't really know what - // Go version to apply. - // Legacy tools may do this, and they historically have accepted everything. - // Preserve that behavior by ignoring //go:build constraints entirely in that - // case (!pkgVersionOk). - if cmp > 0 || cmp < 0 && downgradeOk { - v = file.GoVersion - } - } + // If the file specifies a version, use max(fileVersion, go1.21). + if fileVersion := asGoVersion(file.GoVersion); fileVersion.isValid() { + // Go 1.21 introduced the feature of allowing //go:build lines + // to sometimes set the Go version in a given file. Versions Go 1.21 and later + // can be set backwards compatibly as that was the first version + // files with go1.21 or later build tags could be built with. + // + // Set the version to max(fileVersion, go1.21): That will allow a + // downgrade to a version before go1.22, where the for loop semantics + // change was made, while being backwards compatible with versions of + // go before the new //go:build semantics were introduced. + v = string(versionMax(fileVersion, go1_21)) // Report a specific error for each tagged file that's too new. // (Normally the build system will have filtered files by version, @@ -377,6 +361,13 @@ func (check *Checker) initFiles(files []*syntax.File) { } } +func versionMax(a, b goVersion) goVersion { + if a.cmp(b) > 0 { + return a + } + return b +} + // A bailout panic is used for early termination. type bailout struct{} diff --git a/src/go/types/api_test.go b/src/go/types/api_test.go index beed94f355799..a7aa6488028ec 100644 --- a/src/go/types/api_test.go +++ b/src/go/types/api_test.go @@ -2904,22 +2904,48 @@ func TestFileVersions(t *testing.T) { fileVersion string wantVersion string }{ - {"", "", ""}, // no versions specified - {"go1.19", "", "go1.19"}, // module version specified - {"", "go1.20", ""}, // file upgrade ignored - {"go1.19", "go1.20", "go1.20"}, // file upgrade permitted - {"go1.20", "go1.19", "go1.20"}, // file downgrade not permitted - {"go1.21", "go1.19", "go1.19"}, // file downgrade permitted (module version is >= go1.21) + {"", "", ""}, // no versions specified + {"go1.19", "", "go1.19"}, // module version specified + {"", "go1.20", "go1.21"}, // file version specified below minimum of 1.21 + {"go1", "", "go1"}, // no file version specified + {"go1", "goo1.22", "go1"}, // invalid file version specified + {"go1", "go1.19", "go1.21"}, // file version specified below minimum of 1.21 + {"go1", "go1.20", "go1.21"}, // file version specified below minimum of 1.21 + {"go1", "go1.21", "go1.21"}, // file version specified at 1.21 + {"go1", "go1.22", "go1.22"}, // file version specified above 1.21 + {"go1.19", "", "go1.19"}, // no file version specified + {"go1.19", "goo1.22", "go1.19"}, // invalid file version specified + {"go1.19", "go1.20", "go1.21"}, // file version specified below minimum of 1.21 + {"go1.19", "go1.21", "go1.21"}, // file version specified at 1.21 + {"go1.19", "go1.22", "go1.22"}, // file version specified above 1.21 + {"go1.20", "", "go1.20"}, // no file version specified + {"go1.20", "goo1.22", "go1.20"}, // invalid file version specified + {"go1.20", "go1.19", "go1.21"}, // file version specified below minimum of 1.21 + {"go1.20", "go1.20", "go1.21"}, // file version specified below minimum of 1.21 + {"go1.20", "go1.21", "go1.21"}, // file version specified at 1.21 + {"go1.20", "go1.22", "go1.22"}, // file version specified above 1.21 + {"go1.21", "", "go1.21"}, // no file version specified + {"go1.21", "goo1.22", "go1.21"}, // invalid file version specified + {"go1.21", "go1.19", "go1.21"}, // file version specified below minimum of 1.21 + {"go1.21", "go1.20", "go1.21"}, // file version specified below minimum of 1.21 + {"go1.21", "go1.21", "go1.21"}, // file version specified at 1.21 + {"go1.21", "go1.22", "go1.22"}, // file version specified above 1.21 + {"go1.22", "", "go1.22"}, // no file version specified + {"go1.22", "goo1.22", "go1.22"}, // invalid file version specified + {"go1.22", "go1.19", "go1.21"}, // file version specified below minimum of 1.21 + {"go1.22", "go1.20", "go1.21"}, // file version specified below minimum of 1.21 + {"go1.22", "go1.21", "go1.21"}, // file version specified at 1.21 + {"go1.22", "go1.22", "go1.22"}, // file version specified above 1.21 // versions containing release numbers // (file versions containing release numbers are considered invalid) {"go1.19.0", "", "go1.19.0"}, // no file version specified - {"go1.20", "go1.20.1", "go1.20"}, // file upgrade ignored - {"go1.20.1", "go1.20", "go1.20.1"}, // file upgrade ignored - {"go1.20.1", "go1.21", "go1.21"}, // file upgrade permitted - {"go1.20.1", "go1.19", "go1.20.1"}, // file downgrade not permitted - {"go1.21.1", "go1.19.1", "go1.21.1"}, // file downgrade not permitted (invalid file version) - {"go1.21.1", "go1.19", "go1.19"}, // file downgrade permitted (module version is >= go1.21) + {"go1.20.1", "go1.19.1", "go1.20.1"}, // invalid file version + {"go1.20.1", "go1.21.1", "go1.20.1"}, // invalid file version + {"go1.21.1", "go1.19.1", "go1.21.1"}, // invalid file version + {"go1.21.1", "go1.21.1", "go1.21.1"}, // invalid file version + {"go1.22.1", "go1.19.1", "go1.22.1"}, // invalid file version + {"go1.22.1", "go1.21.1", "go1.22.1"}, // invalid file version } { var src string if test.fileVersion != "" { diff --git a/src/go/types/check.go b/src/go/types/check.go index 1a5a41a3bb4b9..8a729094961fe 100644 --- a/src/go/types/check.go +++ b/src/go/types/check.go @@ -349,7 +349,6 @@ func (check *Checker) initFiles(files []*ast.File) { check.errorf(files[0], TooNew, "package requires newer Go version %v (application built with %v)", check.version, go_current) } - downgradeOk := check.version.cmp(go1_21) >= 0 // determine Go version for each file for _, file := range check.files { @@ -358,33 +357,19 @@ func (check *Checker) initFiles(files []*ast.File) { // unlike file versions which are Go language versions only, if valid.) v := check.conf.GoVersion - fileVersion := asGoVersion(file.GoVersion) - if fileVersion.isValid() { - // use the file version, if applicable - // (file versions are either the empty string or of the form go1.dd) - if pkgVersionOk { - cmp := fileVersion.cmp(check.version) - // Go 1.21 introduced the feature of setting the go.mod - // go line to an early version of Go and allowing //go:build lines - // to “upgrade” (cmp > 0) the Go version in a given file. - // We can do that backwards compatibly. - // - // Go 1.21 also introduced the feature of allowing //go:build lines - // to “downgrade” (cmp < 0) the Go version in a given file. - // That can't be done compatibly in general, since before the - // build lines were ignored and code got the module's Go version. - // To work around this, downgrades are only allowed when the - // module's Go version is Go 1.21 or later. - // - // If there is no valid check.version, then we don't really know what - // Go version to apply. - // Legacy tools may do this, and they historically have accepted everything. - // Preserve that behavior by ignoring //go:build constraints entirely in that - // case (!pkgVersionOk). - if cmp > 0 || cmp < 0 && downgradeOk { - v = file.GoVersion - } - } + // If the file specifies a version, use max(fileVersion, go1.21). + if fileVersion := asGoVersion(file.GoVersion); fileVersion.isValid() { + // Go 1.21 introduced the feature of setting the go.mod + // go line to an early version of Go and allowing //go:build lines + // to set the Go version in a given file. Versions Go 1.21 and later + // can be set backwards compatibly as that was the first version + // files with go1.21 or later build tags could be built with. + // + // Set the version to max(fileVersion, go1.21): That will allow a + // downgrade to a version before go1.22, where the for loop semantics + // change was made, while being backwards compatible with versions of + // go before the new //go:build semantics were introduced. + v = string(versionMax(fileVersion, go1_21)) // Report a specific error for each tagged file that's too new. // (Normally the build system will have filtered files by version, @@ -399,6 +384,13 @@ func (check *Checker) initFiles(files []*ast.File) { } } +func versionMax(a, b goVersion) goVersion { + if a.cmp(b) < 0 { + return b + } + return a +} + // A bailout panic is used for early termination. type bailout struct{} diff --git a/src/internal/types/testdata/check/go1_20_19.go b/src/internal/types/testdata/check/go1_20_19.go index 08365a7cfb564..e040d396c7808 100644 --- a/src/internal/types/testdata/check/go1_20_19.go +++ b/src/internal/types/testdata/check/go1_20_19.go @@ -14,4 +14,4 @@ type Slice []byte type Array [8]byte var s Slice -var p = (Array)(s /* ok because Go 1.20 ignored the //go:build go1.19 */) +var p = (Array)(s /* ok because file versions below go1.21 set the langage version to go1.21 */) diff --git a/src/internal/types/testdata/check/go1_21_19.go b/src/internal/types/testdata/check/go1_21_19.go index 2acd25865d4b6..5866033eafe6f 100644 --- a/src/internal/types/testdata/check/go1_21_19.go +++ b/src/internal/types/testdata/check/go1_21_19.go @@ -14,4 +14,4 @@ type Slice []byte type Array [8]byte var s Slice -var p = (Array)(s /* ERROR "requires go1.20 or later" */) +var p = (Array)(s /* ok because file versions below go1.21 set the langage version to go1.21 */) diff --git a/src/internal/types/testdata/check/go1_21_22.go b/src/internal/types/testdata/check/go1_21_22.go new file mode 100644 index 0000000000000..3939b7b1d868c --- /dev/null +++ b/src/internal/types/testdata/check/go1_21_22.go @@ -0,0 +1,16 @@ +// -lang=go1.21 + +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Check Go language version-specific errors. + +//go:build go1.22 + +package p + +func f() { + for _ = range /* ok because of upgrade to 1.22 */ 10 { + } +} diff --git a/src/internal/types/testdata/check/go1_22_21.go b/src/internal/types/testdata/check/go1_22_21.go new file mode 100644 index 0000000000000..f910ecb59cbc7 --- /dev/null +++ b/src/internal/types/testdata/check/go1_22_21.go @@ -0,0 +1,16 @@ +// -lang=go1.22 + +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Check Go language version-specific errors. + +//go:build go1.21 + +package p + +func f() { + for _ = range 10 /* ERROR "requires go1.22 or later" */ { + } +} diff --git a/src/internal/types/testdata/fixedbugs/issue66285.go b/src/internal/types/testdata/fixedbugs/issue66285.go index 9811fec3f3554..4af76f05da8e4 100644 --- a/src/internal/types/testdata/fixedbugs/issue66285.go +++ b/src/internal/types/testdata/fixedbugs/issue66285.go @@ -1,14 +1,9 @@ -// -lang=go1.21 +// -lang=go1.13 // Copyright 2024 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// Note: Downgrading to go1.13 requires at least go1.21, -// hence the need for -lang=go1.21 at the top. - -//go:build go1.13 - package p import "io" diff --git a/test/fixedbugs/issue63489a.go b/test/fixedbugs/issue63489a.go index b88120f2c045e..2b46814f9566d 100644 --- a/test/fixedbugs/issue63489a.go +++ b/test/fixedbugs/issue63489a.go @@ -1,16 +1,20 @@ -// errorcheck -lang=go1.21 +// errorcheck -lang=go1.22 // Copyright 2023 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build go1.4 +// This file has been changed from its original version as +// //go:build file versions below 1.21 set the language version to 1.21. +// The original tested a -lang version of 1.21 with a file version of +// go1.4 while this new version tests a -lang version of go1.22 +// with a file version of go1.21. -package p - -const c = 0o123 // ERROR "file declares //go:build go1.4" +//go:build go1.21 -// ERROR "file declares //go:build go1.4" +package p -//line issue63489a.go:13:1 -const d = 0o124 +func f() { + for _ = range 10 { // ERROR "file declares //go:build go1.21" + } +} diff --git a/test/fixedbugs/issue63489b.go b/test/fixedbugs/issue63489b.go index 2ad590dfc3334..fd897dea97cb8 100644 --- a/test/fixedbugs/issue63489b.go +++ b/test/fixedbugs/issue63489b.go @@ -1,11 +1,20 @@ -// errorcheck -lang=go1.4 +// errorcheck -lang=go1.21 // Copyright 2023 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build go1.4 +// This file has been changed from its original version as +// //go:build file versions below 1.21 set the language version to 1.21. +// The original tested a -lang version of 1.4 with a file version of +// go1.4 while this new version tests a -lang version of go1.1 +// with a file version of go1.21. + +//go:build go1.21 package p -const c = 0o123 // ERROR "file declares //go:build go1.4" +func f() { + for _ = range 10 { // ERROR "file declares //go:build go1.21" + } +}