From 63b0f805cd83f97c43a45e9558d00513c2399fbf Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Wed, 7 Aug 2024 13:09:18 -0400 Subject: [PATCH] [release-branch.go1.23] go/types, types2: only use fileVersion if 1.21 or greater Only honor //go:build language version downgrades if the version specified is 1.21 or greater. Before 1.21 the version in //go:build lines didn't have the meaning of setting the file's language version. This fixes an issue that was appearing in GOPATH builds: Go 1.23 started providing -lang versions to the compiler in GOPATH mode (among other places) which it wasn't doing before. 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, before this change, it would produce an error when compiling with the 1.23 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. Fixes #68658 Change-Id: I4ac2b45a981cd019183d52ba324ba8f0fed93a8e Reviewed-on: https://go-review.googlesource.com/c/go/+/603895 Reviewed-by: Robert Griesemer Commit-Queue: Michael Matloob LUCI-TryBot-Result: Go LUCI Auto-Submit: Michael Matloob Reviewed-on: https://go-review.googlesource.com/c/go/+/604935 --- src/cmd/compile/internal/types2/api_test.go | 8 ++++++-- src/cmd/compile/internal/types2/check.go | 15 +++++++++++++- src/go/types/api_test.go | 8 ++++++-- src/go/types/check.go | 15 +++++++++++++- .../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 +++++++++++-------- 10 files changed, 87 insertions(+), 22 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 5126ac51116..a9dcac333b5 100644 --- a/src/cmd/compile/internal/types2/api_test.go +++ b/src/cmd/compile/internal/types2/api_test.go @@ -2903,17 +2903,21 @@ func TestFileVersions(t *testing.T) { {"", "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) + {"go1.21", "go1.20", "go1.21"}, // file downgrade not permitted + {"go1.22", "go1.21", "go1.21"}, // file downgrade permitted (file and module version are >= go1.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.21.0", "go1.21.1", "go1.21.0"}, // file upgrade ignored + {"go1.21", "go1.21.1", "go1.21"}, // file upgrade ignored {"go1.20.1", "go1.21", "go1.21"}, // file upgrade permitted + {"go1.21.1", "go1.21", "go1.21.1"}, // file downgrade ignored {"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.22.1", "go1.21", "go1.21"}, // file downgrade permitted (file and module version is >= go1.21) } { 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 91ad474e9df..482d888e99d 100644 --- a/src/cmd/compile/internal/types2/check.go +++ b/src/cmd/compile/internal/types2/check.go @@ -354,12 +354,25 @@ func (check *Checker) initFiles(files []*syntax.File) { // To work around this, downgrades are only allowed when the // module's Go version is Go 1.21 or later. // + // Downgrades are also only allowed to Go versions Go 1.21 or later. + // In GOPATH mode, there's no way to set a module version and the + // -lang is set to the local toolchain version to allow the use of + // new features in GOPATH mode. But //go:build lines added before go1.21 + // weren't intended to downgrade, so code with //go:build lines for + // go versions earlier than 1.21 may use language features added + // in later versions and compile. + // + // We should probably change the downgradeOk condition to capture this + // instead of adding an extra condition, but to make the change simpler, + // we've tried to limit it to one line. + // TODO(gri): simplify this code after 1.23 has shipped + // // 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 { + if cmp > 0 || cmp < 0 && downgradeOk && fileVersion.cmp(go1_21) >= 0 { v = file.GoVersion } } diff --git a/src/go/types/api_test.go b/src/go/types/api_test.go index beed94f3557..828cd5d2119 100644 --- a/src/go/types/api_test.go +++ b/src/go/types/api_test.go @@ -2909,17 +2909,21 @@ func TestFileVersions(t *testing.T) { {"", "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) + {"go1.21", "go1.20", "go1.21"}, // file downgrade not permitted + {"go1.22", "go1.21", "go1.21"}, // file downgrade permitted (file and module version are >= go1.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.21.0", "go1.21.1", "go1.21.0"}, // file upgrade ignored + {"go1.21", "go1.21.1", "go1.21"}, // file upgrade ignored {"go1.20.1", "go1.21", "go1.21"}, // file upgrade permitted + {"go1.21.1", "go1.21", "go1.21.1"}, // file downgrade ignored {"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.22.1", "go1.21", "go1.21"}, // file downgrade permitted (file and module version is >= go1.21) } { var src string if test.fileVersion != "" { diff --git a/src/go/types/check.go b/src/go/types/check.go index 1a5a41a3bb4..15504eed408 100644 --- a/src/go/types/check.go +++ b/src/go/types/check.go @@ -376,12 +376,25 @@ func (check *Checker) initFiles(files []*ast.File) { // To work around this, downgrades are only allowed when the // module's Go version is Go 1.21 or later. // + // Downgrades are also only allowed to Go versions Go 1.21 or later. + // In GOPATH mode, there's no way to set a module version and the + // -lang is set to the local toolchain version to allow the use of + // new features in GOPATH mode. But //go:build lines added before go1.21 + // weren't intended to downgrade, so code with //go:build lines for + // go versions earlier than 1.21 may use language features added + // in later versions and compile. + // + // We should probably change the downgradeOk condition to capture this + // instead of adding an extra condition, but to make the change simpler, + // we've tried to limit it to one line. + // TODO(gri): simplify this code after 1.23 has shipped + // // 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 { + if cmp > 0 || cmp < 0 && downgradeOk && fileVersion.cmp(go1_21) >= 0 { v = file.GoVersion } } diff --git a/src/internal/types/testdata/check/go1_20_19.go b/src/internal/types/testdata/check/go1_20_19.go index 08365a7cfb5..ba1c4543323 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 downgrades below 1.21 are ignored */) diff --git a/src/internal/types/testdata/check/go1_21_19.go b/src/internal/types/testdata/check/go1_21_19.go index 2acd25865d4..6c0900d0272 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 downgrades below 1.21 are ignored */) 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 00000000000..695503ace17 --- /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 { + } +} \ No newline at end of file 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 00000000000..79e21a7de82 --- /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" */ { + } +} \ No newline at end of file diff --git a/src/internal/types/testdata/fixedbugs/issue66285.go b/src/internal/types/testdata/fixedbugs/issue66285.go index 9811fec3f35..4af76f05da8 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 b88120f2c04..9b06d949bfa 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 language downgrades below go1.21 are no longer +// supported. The original tested a downgrade from go1.21 to +// go1.4 while this new version tests a downgrade from go1.22 +// to go1.21 + +//go:build go1.21 package p -const c = 0o123 // ERROR "file declares //go:build go1.4" - -// ERROR "file declares //go:build go1.4" - -//line issue63489a.go:13:1 -const d = 0o124 +func f() { + for _ = range 10 { // ERROR "file declares //go:build go1.21" + } +}