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

Jump to bottom

cmd/compile: handle alias types in recursive types more consistently #50729

Closed
griesemer opened this issue Jan 21, 2022 · 10 comments
Closed

cmd/compile: handle alias types in recursive types more consistently #50729

griesemer opened this issue Jan 21, 2022 · 10 comments
Assignees
@griesemer
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.22

Comments

@griesemer
Copy link
Contributor

The following code

package p

var x T[B]

type T[_ any] struct{}
type A T[B /* ERROR invalid use of type alias */ ]
type B = T[A]

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 of x moved past the type declarations):

package p

type T[_ any] struct{}
type A T[B]
type B = T[A]

var x T[B]

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
@griesemer griesemer added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 21, 2022
@griesemer griesemer added this to the Go1.19 milestone Jan 21, 2022
@griesemer griesemer self-assigned this Jan 21, 2022
@griesemer
Copy link
Contributor Author

See #50259 and #50276.

@gopherbot
Copy link

Change https://golang.org/cl/379916 mentions this issue: cmd/compile/internal/types2: report an error when using a broken alias

@gopherbot
Copy link

Change https://golang.org/cl/380056 mentions this issue: cmd/compile/internal/types2: reorder object processing to avoid broken aliases

gopherbot pushed a commit that referenced this issue Jan 24, 2022
@griesemer
go/types, types2: report an error when using a broken alias
fe85c24 
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>
gopherbot pushed a commit that referenced this issue Jan 24, 2022
@griesemer
go/types, types2: reorder object processing to avoid broken aliases
671e115 
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>
@ianlancetaylor ianlancetaylor mentioned this issue Jan 31, 2022
Closed
@mengzhuo mengzhuo mentioned this issue Mar 7, 2022
Closed
@reddyhc reddyhc mentioned this issue May 9, 2022
Closed
@griesemer
Copy link
Contributor Author

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.

@gopherbot
Copy link

Change https://go.dev/cl/412235 mentions this issue: go/types, types2: add test case for issue for coverage

gopherbot pushed a commit that referenced this issue Jun 15, 2022
@griesemer
go/types, types2: add test case for issue for coverage
74bf90c 
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>
jproberts pushed a commit to jproberts/go that referenced this issue Jun 21, 2022
@griesemer @jproberts
go/types, types2: report an error when using a broken alias
87d1df8 
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>
jproberts pushed a commit to jproberts/go that referenced this issue Jun 21, 2022
@griesemer @jproberts
go/types, types2: reorder object processing to avoid broken aliases
b24be06 
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>
@AndrewHarrisSPU
Copy link

AndrewHarrisSPU commented Jul 19, 2022 •
edited

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!)

@ajwerner
Copy link

Can we re-open this? It can still happen both in tip and in 1.19.4.

@griesemer
Copy link
Contributor Author

Re-opened per previous comments and example: https://gotipplay.golang.org/p/mpONhtxQI4a

@griesemer griesemer reopened this Dec 20, 2022
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Dec 20, 2022
@griesemer griesemer self-assigned this Dec 20, 2022
@griesemer griesemer added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed NeedsFix The path to resolution is known, but the work has not been done. labels Dec 20, 2022
@griesemer griesemer modified the milestones: Go1.19, Go1.21 Dec 20, 2022
@griesemer griesemer modified the milestones: Go1.21, Go1.22 May 25, 2023
@griesemer
Copy link
Contributor Author

This appears to work now with GODEBUG=gotypesalias=1. Will add test and close.

@gopherbot
Copy link

Change https://go.dev/cl/546455 mentions this issue: go/types, types2: add testcase for alias issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@griesemer griesemer

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Release Status & Release Blockers
Status: Done
Milestone
Go1.22
Development

No branches or pull requests
4 participants
@ajwerner @griesemer @gopherbot @AndrewHarrisSPU