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
go/types, cmd/compile: "invalid type loop" depending on declaration order #51244
Comments
Will investigate. I suspect that the fix in https://go.dev/cl/361922 may be incomplete. |
Ok, I looked into it and the fix from CL 361922 was indeed incomplete. I have experimented with a couple solutions, and mailed one in https://go.dev/cl/386718. Reproducing the commit message here for discussion:
That CL implements option #2, which feels more correct but is also riskier. We should perhaps implement option #1 for 1.18 (if anything). |
Change https://go.dev/cl/386718 mentions this issue: |
@griesemer I think we decided not to fix this for 1.18, correct? Shall I move to the 1.19 milestone? |
Yes, I think pending CL may have subtle repercussions; any change in the cycle detection algorithm may have non-local effects. We decided to wait for 1.19 as this is not a release blocker. Moving to 1.19. |
Just grooming issues, this may have fallen through the cracks. @griesemer let's discuss whether to bump this to 1.20, or land a fix for 1.19. |
This issue is currently labeled as early-in-cycle for Go 1.20. |
@griesemer do we want to fix this for 1.20? I can revisit my CL. |
@findleyr We still have some time, so yes, let's see if we can address this. |
This was a complicated one, and we had higher priorities (even higher priority fixes to validType). I think we should aim for early-in-cycle 1.21 (but actually early, this time). |
This issue is currently labeled as early-in-cycle for Go 1.21. |
I thought about this more today, and while I think the fix in https://go.dev/cl/386718 is OK, I actually think we could go one step further: why can't we just allow cycles through constraints? If a cycle occurs through a constraint, that constraint is still well-defined, and we only need to verify instances that occur in the source, which is by definition a finite number of instances. I think the guard was originally added to avoid infinite recursion, but we have since developed better mechanisms for this. |
Not happening for 1.21, but I think we know what to do. Should land this early in 1.22 cycle. |
This issue is currently labeled as early-in-cycle for Go 1.22. |
types2 currently rejects the test case below with "invalid recursive type A". Swapping the order of declarations for A and B makes the error go away.
/cc @findleyr @griesemer @ianlancetaylor
The text was updated successfully, but these errors were encountered: