-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: handle alias types in recursive types more consistently #50729
Comments
Change https://golang.org/cl/379916 mentions this issue: |
Change https://golang.org/cl/380056 mentions this issue: |
The type checker doesn't have a general mechanism to "use" the type of a type alias whose type depends on a recursive type declaration which is not yet completely type-checked. In some cases, the type of a type alias is needed before it is determined; the type is incorrect (invalid) in that case but no error is reported. The type-checker is happy with this (incorrect type), but the compiler may crash under some circumstances. A correct fix will likely require some form of forwarding type which is a fairly pervasive change and may also affect the type checker API. This CL introduces a simple side table, a map of broken type aliases, which is consulted before the type associated with a type alias is used. If the type alias is broken, an error is reported. This is a stop-gap solution that prevents the compiler from crashing. The reported error refers to the corresponding issue which suggests a work-around that may be applicable in some cases. Also fix a minor error related to type cycles: If we have a cycle that doesn't start with a type, don't use a compiler error message that explicitly mentions "type". Fixes #50259. Fixes #50276. Fixes #50779. For #50729. Change-Id: Ie8e38f49ef724e742e8e78625e6d4f3d4014a52c Reviewed-on: https://go-review.googlesource.com/c/go/+/379916 Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
By processing non-alias type declarations before alias type declaration, and those before everything else we can avoid some of the remaining errors which are due to alias types not being available. For #25838. For #50259. For #50276. For #50729. Change-Id: I233da2899a6d4954c239638624dfa8c08662e6b9 Reviewed-on: https://go-review.googlesource.com/c/go/+/380056 Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
This error doesn't appear anymore due to some of the (partial) fixes done with the CLs above. There's still more to do to handle aliases correctly in all cases but that's separate from this issue. We should add a test to cover this specific case and then close this issue. |
Change https://go.dev/cl/412235 mentions this issue: |
The specific error doesn't occur anymore. Add a test to prevent regressions. For #50729. Change-Id: Ibf6ef6009b3d226b4f345b5a5657939915f19633 Reviewed-on: https://go-review.googlesource.com/c/go/+/412235 Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
The type checker doesn't have a general mechanism to "use" the type of a type alias whose type depends on a recursive type declaration which is not yet completely type-checked. In some cases, the type of a type alias is needed before it is determined; the type is incorrect (invalid) in that case but no error is reported. The type-checker is happy with this (incorrect type), but the compiler may crash under some circumstances. A correct fix will likely require some form of forwarding type which is a fairly pervasive change and may also affect the type checker API. This CL introduces a simple side table, a map of broken type aliases, which is consulted before the type associated with a type alias is used. If the type alias is broken, an error is reported. This is a stop-gap solution that prevents the compiler from crashing. The reported error refers to the corresponding issue which suggests a work-around that may be applicable in some cases. Also fix a minor error related to type cycles: If we have a cycle that doesn't start with a type, don't use a compiler error message that explicitly mentions "type". Fixes golang#50259. Fixes golang#50276. Fixes golang#50779. For golang#50729. Change-Id: Ie8e38f49ef724e742e8e78625e6d4f3d4014a52c Reviewed-on: https://go-review.googlesource.com/c/go/+/379916 Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
By processing non-alias type declarations before alias type declaration, and those before everything else we can avoid some of the remaining errors which are due to alias types not being available. For golang#25838. For golang#50259. For golang#50276. For golang#50729. Change-Id: I233da2899a6d4954c239638624dfa8c08662e6b9 Reviewed-on: https://go-review.googlesource.com/c/go/+/380056 Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
Was able to reach this in 1.19b2. Tiny reproduction: https://gotipplay.golang.org/p/mpONhtxQI4a (really excellent to have an issue number in the compiler error!) |
Can we re-open this? It can still happen both in tip and in 1.19.4. |
Re-opened per previous comments and example: https://gotipplay.golang.org/p/mpONhtxQI4a |
This appears to work now with GODEBUG=gotypesalias=1. Will add test and close. |
Change https://go.dev/cl/546455 mentions this issue: |
For some reason I don't totally understand, I got a notification from "Aaron" from GH on this issue yesterday with the link #50729 (comment), but I do not see it here. Here is the message verbatim:
|
Minimized reproducer of this example https://go.dev/play/p/_XljMNVUwEd :
Commenting out cc @griesemer |
This normally means that the person added a comment and then deleted it. |
@ianlancetaylor Thanks. I've opened #70230 to focus on just on this example. Re-closing. |
The following code
reports an error when we use
B
in a recursive type definition due to the way type aliases are handled inside the type checker. In this (and possibly many other similar) cases, the error can be avoided by restructuring the code slightly. This version compiles without error (declaration ofx
moved past the type declarations):The compiler should be able to avoid this error on its own.
Root cause: Currently, the type checker doesn't have a "forwarding mechanism" for type aliases that are being "used" before their respective type is fully known. To solve this problem in general, a forwarding mechanism/type needs to be introduced. This is only an issue with some recursive type definitions involving type aliases.
cc: @findleyr
The text was updated successfully, but these errors were encountered: