Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/ast, go/build/constraint: support for per-file Go versions using //go:build lines #59033

Closed
rsc opened this issue Mar 14, 2023 · 16 comments
Closed
Labels
Proposal Proposal-Accepted WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Mar 14, 2023

As part of the discussion during the acceptance of #57001, we decided the following.

  1. Starting in Go 1.21, we will interpret the go line in go.mod as the minimum Go version required. For example if Go 1.21 encounters a go.mod file that says go 1.22, it will refuse to build code in that module.
  2. To allow writing a module with a low minimum Go version but that can use newer features in //go:build-tagged files, we will recognize //go:build constraints that require a newer Go version than the go.mod go line and allow newer Go features in those files. We are also going to allow a //go:build constraint to declare an older version, which will lock out newer Go features that would normally be allowed by the go.mod go version. For compatibility reasons, that “downgrading” can only apply for go.mod files saying go 1.21 or later.

To implement (2), we need a small amount of new API: go/build/constraint needs to export code to compute the implied minimum Go version from a constraint.Expr, and go/ast needs to expose the minimum Go version as a new field in ast.File.

This proposal is about that new API. I propose:

package constraint

// GoVersion returns the minimum Go version implied by a given build expression.
// If the expression can be satisfied without any Go version tags, GoVersion returns an empty string.
//
// For example:
//
//	GoVersion(linux && go1.22) = "go1.22"
//	GoVersion((linux && go1.22) || (windows && go1.20)) = "go1.20" => go1.20
//	GoVersion(linux) = ""
//	GoVersion(linux || (windows && go1.22)) = ""
//
// GoVersion assumes that any tag or negated tag may independently be true,
// so that its analysis can be purely structural, without SAT solving.
// “Impossible” subexpressions may therefore affect the result.
//
// For example:
//
//	GoVersion((linux && !linux && go1.20) || go1.21) = "go1.20"
func GoVersion(x Expr) string

And

package ast

type File struct {
	...
	GoVersion          string          // minimum Go version required by //go:build directives
}

go/parser would populate ast.File.GoVersion, and go/types would use that information to decide whether specific features are allowed in certain files.

@rsc rsc added this to the Proposal milestone Mar 14, 2023
@gopherbot
Copy link

Change https://go.dev/cl/476275 mentions this issue: go/build/constraint: add GoVersion

@gopherbot
Copy link

Change https://go.dev/cl/476276 mentions this issue: go/ast: add File.GoVersion

@gopherbot
Copy link

Change https://go.dev/cl/476277 mentions this issue: go/parser: report //go:build-derived Go version in ast.File.GoVersion

@gopherbot
Copy link

Change https://go.dev/cl/476278 mentions this issue: go/types, cmd/compile/internal/types2: use per-file Go version

@dr2chase
Copy link
Contributor

It seems like this does slightly interfere with "all developers working on a project will agree on the toolchain version"; if I am using 1.22, someone else is using 1.23, the app under development has a 1.21 go.mod minimum version but contains files with 1.23 build tags, the two of us will see different builds of the app. I still want this change, though.

@rsc
Copy link
Contributor Author

rsc commented Mar 15, 2023

All developers still get the same, reproducible build. That build just agrees on which files use which Go dialects. I don't think this conflicts with that.

@rsc
Copy link
Contributor Author

rsc commented Mar 15, 2023

I should add a ! example; GoVersion(!go1.22) = "".

@rsc
Copy link
Contributor Author

rsc commented Mar 15, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@hherman1
Copy link

hherman1 commented Mar 16, 2023

Is there a comment in the prior issue which summarizes the use case for having an older toolchain version for an individual file then the minimum go version in the module? It seems like a strange case to me.

@rsc
Copy link
Contributor Author

rsc commented Mar 29, 2023

Is there a comment in the prior issue which summarizes the use case for having an older toolchain version for an individual file then the minimum go version in the module? It seems like a strange case to me.

If you are updating and know that the older file needs older semantics, you can set the default for the module (to cover new files created there) and then special-case the few files you need older semantics in.

@rsc
Copy link
Contributor Author

rsc commented Mar 29, 2023

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc
Copy link
Contributor Author

rsc commented Apr 6, 2023

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: go/ast, go/build/constraint: support for per-file Go versions using //go:build lines go/ast, go/build/constraint: support for per-file Go versions using //go:build lines Apr 6, 2023
@rsc rsc modified the milestones: Proposal, Backlog Apr 6, 2023
gopherbot pushed a commit that referenced this issue Apr 11, 2023
For #57001, programs need to be able to deduce the Go version
implied by a given build constraint. GoVersion determines that,
by discarding all build tags other than Go versions and computing
the minimum Go version implied by the resulting expression.

For #59033.

Change-Id: Ifb1e7af2bdbdf172f82aa490c826c9b6ca5e824b
Reviewed-on: https://go-review.googlesource.com/c/go/+/476275
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Apr 11, 2023
For #57001, compilers and others tools will need to understand that
a different Go version can be used in different files in a program,
according to the //go:build lines in those files.

This CL adds a GoVersion string field to ast.File, to allow exposing this
per-file Go version information.

For #59033.

Change-Id: I3931ea86c237983d152964f48dce498bcb1f06aa
Reviewed-on: https://go-review.googlesource.com/c/go/+/476276
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
gopherbot pushed a commit that referenced this issue Apr 13, 2023
For #57001, compilers and others tools will need to understand that
a different Go version can be used in different files in a program,
according to the //go:build lines in those files.

Update go/parser to populate the new ast.File.GoVersion field.

This requires running the go/scanner in ParseComments mode
always and then implementing discarding of comments in the
parser instead of the scanner. The same work is done either way,
since the scanner was already preparing the comment result
and then looping. The loop has just moved into go/parser.

Also make the same changes to cmd/compile/internal/syntax,
both because they're necessary and to keep in sync with go/parser.

For #59033.

Change-Id: I7b867f5f9aaaccdca94af146b061d16d9a3fd07f
Reviewed-on: https://go-review.googlesource.com/c/go/+/476277
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
gopherbot pushed a commit that referenced this issue Apr 14, 2023
For #57001, compilers and others tools will need to understand that
a different Go version can be used in different files in a program,
according to the //go:build lines in those files.

Update go/types and cmd/compile/internal/types2 to track and
use per-file Go versions. The two must be updated together because
of the files in go/types that are generated from files in types2.

The effect of the //go:build go1.N line depends on the Go version
declared in the 'go 1.M' line in go.mod. If N > M, the file gets go1.N
semantics when built with a Go 1.N or later toolchain
(when built with an earlier toolchain the //go:build line will keep
the file from being built at all).
If N < M, then in general we want the file to get go1.N semantics
as well, meaning later features are disabled. However, older Go 1.M
did not apply this kind of downgrade, so for compatibility, N < M
only has an effect when M >= 21, meaning when using semantics
from Go 1.21 or later.

For #59033.

Change-Id: I93cf07e6c687d37bd37a9461dc60cc032bafd01d
Reviewed-on: https://go-review.googlesource.com/c/go/+/476278
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/499415 mentions this issue: doc/go1.21: mention directive handling in go/{ast,build}

gopherbot pushed a commit that referenced this issue May 31, 2023
For #56986
For #59033

Change-Id: I7d03fe34d418aff97a551b236b5d43506e402871
Reviewed-on: https://go-review.googlesource.com/c/go/+/499415
TryBot-Bypass: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
@bcmills
Copy link
Contributor

bcmills commented Nov 21, 2023

I think this was implemented in Go 1.21. @rsc, is there anything more to do for it?

@bcmills bcmills added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 21, 2023
@bcmills
Copy link
Contributor

bcmills commented Nov 21, 2023

@rsc
Copy link
Contributor Author

rsc commented Dec 5, 2023

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Proposal-Accepted WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
Status: Accepted
Development

No branches or pull requests

5 participants