-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
cmd/compile: importing generic alias types is not supported in 1.23 #68526
Comments
In playground instead of a zip file: https://go.dev/play/p/MwWuoC7fDFg?v=gotip |
neat (inlining files) |
same only without import (works as)... |
The failure stacktrace is
cc @golang/compiler @griesemer |
While digging in I found another bug. I'll file a new bug in a bit. https://go.dev/play/p/arcCVWSA8AL?v=gotip
|
Here is a simplified reproducer https://go.dev/play/p/aNAwYF7zDZ-?v=gotip |
It seems to me this is the problem with lazy importing, go/types ureader works well. |
Change https://go.dev/cl/599915 mentions this issue: |
AFAICT export data does not yet have support for type parameterized aliases. That would mean no support for multiple Go package with GOEXPERIMENT=aliastypeparams yet. (Apologies if I just missed it.) I have a WiP CL that fixes the crash https://go.dev/cl/599915. I know I don't yet have reflection working yet. Put together these changes seem fairly risky to me given how late in the release cycle we are for 1.23. Three options I am seeing for addressing this are:
|
Yes, that's right. This was a known limitation, but in that case I think it should be clarified in the release notes. A better error message (option 3) sounds like the safest bet at this point. I'm not sure there's any reason to back out the experiment (option 2), since it's an experiment, but it is unfortunate that importing doesn't work. |
AFAICT the current patch set for https://go.dev/cl/599915 fixes the issue. It still needs a much more thorough review and a roll out plan. Ideally changes like this would have a couple of months to soak before a release. I am still looking into whether/how we can do a better error message. |
Change https://go.dev/cl/600615 mentions this issue: |
Change https://go.dev/cl/601035 mentions this issue: |
Change https://go.dev/cl/600936 mentions this issue: |
For golang/go#68526. For golang/go#65614. Change-Id: I260da94ffaae0d6583e6396b0a78b46dd298313c Reviewed-on: https://go-review.googlesource.com/c/website/+/600936 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Tim King <taking@google.com> Reviewed-by: Robert Griesemer <gri@google.com> TryBot-Bypass: Robert Griesemer <gri@google.com>
The CL addresses the issue for 1.23. |
…rting generic type alias When GOEXPERIMENT=aliastypeparams is set, type aliases may have type parameters. The compiler export data doesn't export that type parameter information yet, which leads to an index-out-of-bounds panic when a client package imports a package with a general type alias and then refers to one of the missing type parameters. This CL detects this specific case and panics with a more informative panic message explaining the shortcoming. The change is only in effect if the respective GOEXPERIMENT is enabled. Manually tested. No test addded since this is just a temporary fix (Go 1.24 will have a complete implementation), and because the existing testing framework doesn't easily support testing that a compilation panics. Together with @taking and input from @rfindley. For #68526. Change-Id: I24737b153a7e2f9b705cd29a5b70b2b9e808dffc Reviewed-on: https://go-review.googlesource.com/c/go/+/601035 Reviewed-by: Tim King <taking@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Griesemer <gri@google.com>
Change https://go.dev/cl/604102 mentions this issue: |
Adds a -gomodversion flag to testdir. This sets the go version in generated go.mod files. This is just runindir tests at the moment. This is a building block so that tests can be written for exported type parameterized aliases (like reproducing #68526). This also adds a test that uses this feature. A type parameterized alias is used so aliastypeparams and gotypesalias must be enabled. gotypesalias is enabled by the go module version. The alias is not exported and will not appear in exportdata. The test shows the package containing the alias can be imported. This encapsulates the level of support of type parameterized aliases in 1.23. Updates #68526 Updates #68778 Change-Id: I8e20df6baa178e1d427d0fff627a16714d9c3b18 Reviewed-on: https://go-review.googlesource.com/c/go/+/604102 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Austin Clements <austin@google.com>
Change https://go.dev/cl/608595 mentions this issue: |
Enables V2 unified IR bitstreams when GOEXPERIMENT aliastypeparams are enabled. Allows pkgbits.NewPkgEncoder to set the output version. Reenables support for writing V0 streams. Updates #68778 Updates #68526 Change-Id: I590c494d81ab7db148232ceaba52229068d1e986 Reviewed-on: https://go-review.googlesource.com/c/go/+/608595 Reviewed-by: David Chase <drchase@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Is support for cross-package generic type aliases planned? Or would resolution of this issue entail only emission of a non-iceing error? |
Yes. Follow along here #68778.
According to #68526 (comment) this will be resolved after cross package support is working. |
…rting generic type alias When GOEXPERIMENT=aliastypeparams is set, type aliases may have type parameters. The compiler export data doesn't export that type parameter information yet, which leads to an index-out-of-bounds panic when a client package imports a package with a general type alias and then refers to one of the missing type parameters. This CL detects this specific case and panics with a more informative panic message explaining the shortcoming. The change is only in effect if the respective GOEXPERIMENT is enabled. Manually tested. No test addded since this is just a temporary fix (Go 1.24 will have a complete implementation), and because the existing testing framework doesn't easily support testing that a compilation panics. Together with @taking and input from @rfindley. For golang#68526. Change-Id: I24737b153a7e2f9b705cd29a5b70b2b9e808dffc Reviewed-on: https://go-review.googlesource.com/c/go/+/601035 Reviewed-by: Tim King <taking@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Griesemer <gri@google.com>
Go version
go1.23rc1 (+ go1.23rc2)
Output of
go env
in your module/workspace:What did you do?
GOEXPERIMENT=aliastypeparams GO111MODULE=auto ~/go/bin/go1.23rc2 run .
example code files: has relative local import
bug.zip
What did you see happen?
What did you expect to see?
same as when inside package, that is, no error.
The text was updated successfully, but these errors were encountered: