x/tools/go/analysis/passes/stdversion: spurious reports in files tagged //go:build 1.<old>

[adonovan]

The stdversion analyzer, which will soon become part of the vet suite (see https://go.dev/cl/582556), gives false positives when run on files that, although part of modules with recent Go versions, have old build tags. In this example from x/build, the file is tagged 1.16 even though the module requires go1.21.

stdversion should only honor the file version if it is greater than the module version. (And perhaps some other tool should warn when a file's build tag is so old that it becomes redundant with the module version.)

x/build$ go vet -c=0 ./cmd/coordinator
cmd/coordinator/buildstatus.go:1005:37: strings.Cut requires go1.18 or later (file is go1.16)
1005            switch pkg, variant, _ := strings.Cut(new, ":"); {

[gopherbot]

Change https://go.dev/cl/582595 mentions this issue: go/analysis/passes/stdversion: old file doesn't trump new module

[findleyr]

As long as the module version is go1.21 or later, this is working as intended. Starting with 1.21, //go:build directives can downgrade the language semantics.

If the module version was 1.20 or earlier, this should not be an error (we should write a test).

[adonovan]

Got it; sorry for the confusion.

There is still something strange about the meaning of tags. A build constraint of go1.16 has two meanings--select the file only when using a toolchain of at least go1.16, and downgrade the effective language semantics of that toolchain (go1.21, say) to the go1.16 spec. But now consider the constraint go1.16 || go1.17 || ... || go1.22: this also means select the file only when using a toolchain of at least go1.16, but it does not cause the language spec to change because its form is not a conjunction, it's a disjunction. Is that right?

[timothy-king]

I don't believe so. go1.16 || go1.17 || ... || go1.22 will be recorded as go1.16 for the *ast.File. This is what go/types can pick up.

go/parser/parser.go does the following:

            if p.top && strings.HasPrefix(p.lit, "//go:build") {
                if x, err := constraint.Parse(p.lit); err == nil {
                    p.goVersion = constraint.GoVersion(x)
                }
            }

go/build/constraint's doc:

// 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(!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

(The level of complexity here is kinda frustrating.)

[adonovan]

You're absolutely right: https://go.dev/play/p/Ifets4t1wMk. (I never studied the constraint evaluation logic closely before.) It seems like the only thing needed here is a test of the 1.20 case. Thanks, all.

[adonovan]

Considering the original issue: if stdversion is working as intended, does that mean cmd/compile has a bug? Should it reject an attempt to call strings.Cut (go1.18) when compiling the file with go1.16 semantics?

[gopherbot]

Change https://go.dev/cl/582958 mentions this issue: cmd/coordinator: remove obsolete build tag

[gopherbot]

Change https://go.dev/cl/582936 mentions this issue: go/analysis/passes/stdversion: test *.go < go.mod version

[gopherbot]

Change https://go.dev/cl/582959 mentions this issue: all: modernize build constraints