x/tools/go/packages: gotypesalias=1 does not seem to get used correctly for deps on Go 1.23

[mvdan]

$ go version
go version go1.23.3 linux/amd64
$ go env
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/mvdan/.cache/go-build'
GOENV='/home/mvdan/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/mvdan/go/pkg/mod'
GONOPROXY='github.com/cue-unity'
GONOSUMDB='github.com/cue-unity'
GOOS='linux'
GOPATH='/home/mvdan/go'
GOPRIVATE='github.com/cue-unity'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.23.3'
GODEBUG=''
GOTELEMETRY='on'
GOTELEMETRYDIR='/home/mvdan/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build977617588=/tmp/go-build -gno-record-gcc-switches'

Take the testscript below:

go run .

go run . -godebug

go run . -godebug -needsyntax

go run . -needsyntax
-- go.mod --
module test

go 1.23

require golang.org/x/tools v0.27.0

require (
	golang.org/x/mod v0.22.0 // indirect
	golang.org/x/sync v0.9.0 // indirect
)
-- go.sum --
golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4=
golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY=
golang.org/x/sync v0.9.0 h1:fEo0HyrW1GIgZdpbhCRO0PkJajUS5H9IFUztCgEo2jQ=
golang.org/x/sync v0.9.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
golang.org/x/tools v0.27.0 h1:qEKojBykQkQ4EynWy4S8Weg69NumxKdn40Fce3uc/8o=
golang.org/x/tools v0.27.0/go.mod h1:sUi0ZgbwW9ZPAq26Ekut+weQPR5eIM6GQLQ1Yjm1H0Q=
-- main.go --
package main

import (
	"flag"
	"fmt"
	"os"

	"golang.org/x/tools/go/packages"
)

var (
	godebug    = flag.Bool("godebug", false, "")
	needsyntax = flag.Bool("needsyntax", false, "")
)

func main() {
	flag.Parse()
	if *godebug {
		os.Setenv("GODEBUG", "gotypesalias=1")
	}
	mode := packages.NeedTypes
	if *needsyntax {
		mode |= packages.NeedSyntax
	}
	cfg := &packages.Config{Mode: mode}
	pkgs, err := packages.Load(cfg, "./pkg1")
	if err != nil {
		panic(err)
	}
	if packages.PrintErrors(pkgs) > 0 {
		os.Exit(1)
	}

	pkg := pkgs[0]
	scope := pkg.Types.Scope()
	fmt.Println(scope.Lookup("Var1"))
	fmt.Println(scope.Lookup("Var2"))
}
-- pkg1/pkg1.go --
package pkg1

import "test/pkg2"

type Alias1 = int32

var Var1 Alias1

var Var2 pkg2.Alias2

-- pkg2/pkg2.go --
package pkg2

type Alias2 = int64

With https://pkg.go.dev/github.com/rogpeppe/go-internal/cmd/testscript, I see:

> go run .
[stdout]
var test/pkg1.Var1 test/pkg1.Alias1
var test/pkg1.Var2 int64

> go run . -godebug
[stdout]
var test/pkg1.Var1 test/pkg1.Alias1
var test/pkg1.Var2 int64

> go run . -godebug -needsyntax
[stdout]
var test/pkg1.Var1 test/pkg1.Alias1
var test/pkg1.Var2 test/pkg2.Alias2

> go run . -needsyntax
[stdout]
var test/pkg1.Var1 test/pkg1.Alias1
var test/pkg1.Var2 test/pkg2.Alias2

The first two results seem wrong; alias tracking is lost for the transitive dependency, given that we see the alias target int64 rather than the alias declared in the transitive dependency, pkg2.Alias2. This happens whether or not I set the GODEBUG flag, which should already be on by default in Go 1.23.

The issue only goes away once I set NeedSyntax, which presumably forces the current process to parse and typecheck all packages directly, rather than relying on the Go toolchain to typecheck transitive dependencies and store the results in GOCACHE, for the current process to load from a warm cache.

This bug seems to go away as of go version devel go1.24-493edb2973 2024-11-16 15:10:05 +0000 linux/amd64:

> go run .
[stdout]
var test/pkg1.Var1 test/pkg1.Alias1
var test/pkg1.Var2 test/pkg2.Alias2

> go run . -godebug
[stdout]
var test/pkg1.Var1 test/pkg1.Alias1
var test/pkg1.Var2 test/pkg2.Alias2

> go run . -godebug -needsyntax
[stdout]
var test/pkg1.Var1 test/pkg1.Alias1
var test/pkg1.Var2 test/pkg2.Alias2

> go run . -needsyntax
[stdout]
var test/pkg1.Var1 test/pkg1.Alias1
var test/pkg1.Var2 test/pkg2.Alias2

[gabyhelp]

Related Issues

• cmd/cgo: generated code uses aliases even in modules using Go older than 1.9 #67738
• cmd/compile: crash while exporting aliases (with GODEBUG=gotypesalias=1) #66550 (closed)
• x/tools/gopls: type alias rename changes aliased type #61625 (closed)
• cmd/go: `go env` does not show GODEBUG #65777 (closed)
• types2, go/types: "cannot convert to type" regression with alias to pointer types #67540 (closed)
• cmd/go2go: A type alias that references a parameterized type in another package causes a panic #39792 (closed)
• cmd/go: in 1.23rc2, TestScript fails on Windows when GOEXPERIMENT is set: "go was invoked but no counters were incremented" #68579
• cmd/compile: importing generic alias types is not supported in 1.23 #68526 (closed)

Related Code Changes

• go/types, types2: implement Alias proposal (export API)
• go/types: remove GOEXPERIMENT=aliastypeparams

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[mvdan]

I didn't spend the time bisecting why this works on Go master (1.24). I suppose it could be because alias tracking is always on at this point, or perhaps due to other changes in how export data is generated or loaded.

[gopherbot]

Change https://go.dev/cl/629997 mentions this issue: [release-branch.go1.23] cmd/compile: enable alias for pkgReader

[xieyuschen]

The issue locates inside the cmd/compile. packages.Load triggers go list with -compile and -export, and it relies on cmd/compile to generate the exported data.

Tool compile will be triggered twice in this scenario for pkg2 and pkg1 and the export data for pkg2 is generated first. However, when compile tries to read exportdata pkg2 for pkg1, the enableAlias is set false. As a result, compile won't respect the alias inside pkg2 and it generates pkg1's exportdata without alias.

go/src/cmd/compile/internal/importer/ureader.go

Lines 34 to 36 in 777f43a

	// Currently, the compiler panics when using Alias types.
	// TODO(gri) set to true once this is fixed (issue #66873)
	enableAlias: false,

Then the result is read by tools/go/packages and assign type int64 to Ver2 instead of Alias2, which is what we expected.

I'm still new to the cmd/compile part and don't know a lot of things, so it's possible I've missed something important. I would appreciate any corrections or guidance. Thanks a lot!

Regarding on the CL629997, I don't know the following things but I will try to check them tomorrow.

Updated in 21Nov

• whether it breaks the other cases we wanted to prevent before.
  It doesn't matter because the logic doesn't affect the logic of compile tool to generate exportdata. The tool could generate exportdata correctly. Current issue in go1.23.x is the reader of exportdata won't respect alias because the field of enableAlias.
• where and how to put a test case..
  done in the new patchset of CL629997

[timothy-king]

Great find. I suspected it was something like this, but narrowing it down is super helpful. I will follow up on this.

[mvdan]

That would also explain why 1.24 (master) did not have this bug, as I recall some rather large import/export changes in the compiler.

[gopherbot]

Change https://go.dev/cl/631855 mentions this issue: [release-branch.go1.23] cmd/compile/internal/importer: enable aliases

[gopherbot]

Change https://go.dev/cl/632116 mentions this issue: go/packages: add a unit test for golang/go#70394

[mvdan]

I think the milestone for this issue is not right by the way - this issue affects 1.23.x only, not 1.24, so it should probably be milestoned for the next 1.23.x release.

[dmitshur]

@mvdan This issue can indeed be marked as closed since it's resolved in Go 1.24, but the Go1.24 milestone is right. There is a child backport issue (#70517 in this case) that needs to refer to this one. We need this parent issue for tracking of any remaining development (e.g., like for CL 632116 which adds a test), even if the problem happened to be fixed before the issue was filed.

See https://go.dev/wiki/MinorReleases, which includes "As soon as an interested party thinks an issue should be considered for backport, they open one or two “child” issues titled like package: title [1.17 backport]. The issue should include a link to the original issue and a short rationale about why the backport might be needed." and more details.

[timothy-king]

For a 1.22 backport, we would need to backport most of https://go.dev/cl/579935, https://go.dev/cl/585399, and possibly other subsequent CLs that build on top of these and fix bugs. These would not be "100% clean" backports either as public APIs are involved and differ between releases.

Given the relatively low volume of user complaint, the complexity/risk of the backport, and hopefully soon availability of a 1.23 fix, I am not sure a 1.22 backport makes sense at this time.

[mvdan]

All I need is a backport to 1.23, personally.

[gopherbot]

Change https://go.dev/cl/633695 mentions this issue: all: merge master (e334696) into gopls-release-branch.0.17

[gopherbot]

Change https://go.dev/cl/633698 mentions this issue: go/packages: add a unit test for golang/go#70394

[gopherbot]

Change https://go.dev/cl/633735 mentions this issue: all: merge master (7eebab3) into gopls-release-branch.0.17