Skip to content

Commit

Permalink
[release-branch.go1.23] go/types, types2: only use fileVersion if 1.2…
Browse files Browse the repository at this point in the history
…1 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 <[email protected]>
Commit-Queue: Michael Matloob <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Michael Matloob <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/604935
  • Loading branch information
matloob authored and cagedmantis committed Aug 12, 2024
1 parent 7adb012 commit 63b0f80
Show file tree
Hide file tree
Showing 10 changed files with 87 additions and 22 deletions.
8 changes: 6 additions & 2 deletions src/cmd/compile/internal/types2/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down
15 changes: 14 additions & 1 deletion src/cmd/compile/internal/types2/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
8 changes: 6 additions & 2 deletions src/go/types/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down
15 changes: 14 additions & 1 deletion src/go/types/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
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 downgrades below 1.21 are ignored */)
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 downgrades below 1.21 are ignored */)
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 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

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"
}
}

0 comments on commit 63b0f80

Please sign in to comment.