Skip to content

Commit

Permalink
go/types, types2: use max(fileVersion, go1.21) if fileVersion present
Browse files Browse the repository at this point in the history
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

Change-Id: I6cedda81a55bcccffaa3501eef9e2be6541b6ece
Reviewed-on: https://go-review.googlesource.com/c/go/+/607955
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Griesemer <gri@google.com>
  • Loading branch information
matloob committed Aug 27, 2024
1 parent 54fe0fd commit aeac0b6
Show file tree
Hide file tree
Showing 11 changed files with 174 additions and 99 deletions.
50 changes: 38 additions & 12 deletions src/cmd/compile/internal/types2/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3031,22 +3031,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 != "" {
Expand Down
47 changes: 19 additions & 28 deletions src/cmd/compile/internal/types2/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,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 {
Expand All @@ -335,33 +334,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,
Expand All @@ -376,6 +360,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{}

Expand Down
50 changes: 38 additions & 12 deletions src/go/types/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3035,22 +3035,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 != "" {
Expand Down
48 changes: 20 additions & 28 deletions src/go/types/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,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 {
Expand All @@ -356,33 +355,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,
Expand All @@ -397,6 +382,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{}

Expand Down
2 changes: 1 addition & 1 deletion src/internal/types/testdata/check/go1_20_19.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 */)
2 changes: 1 addition & 1 deletion src/internal/types/testdata/check/go1_21_19.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 */)
16 changes: 16 additions & 0 deletions src/internal/types/testdata/check/go1_21_22.go
Original file line number Diff line number Diff line change
@@ -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 {
}
}
16 changes: 16 additions & 0 deletions src/internal/types/testdata/check/go1_22_21.go
Original file line number Diff line number Diff line change
@@ -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" */ {
}
}
7 changes: 1 addition & 6 deletions src/internal/types/testdata/fixedbugs/issue66285.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
20 changes: 12 additions & 8 deletions test/fixedbugs/issue63489a.go
Original file line number Diff line number Diff line change
@@ -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"
}
}
15 changes: 12 additions & 3 deletions test/fixedbugs/issue63489b.go
Original file line number Diff line number Diff line change
@@ -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"
}
}

0 comments on commit aeac0b6

Please sign in to comment.