-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/compile: untyped types reaching backend #23834
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
Comments
Fixing this might be tricky, but an easy way to help would be to find some minimal test cases that reproduce failures.
|
@mdempsky I don't know if I can solve this, but since I am interested, I'll try to investigate it. |
@namusyaka That would be great, thanks! Feel free to ask here if you have any questions. |
The example above is caused by the generated .eq.[2]interface{} method. |
@fraenkel Thanks! So just
repros that issue. Further steps for folks interested in helping debug/fix this:
|
Fixed the OIF and OFOR. Do you want separate CLs? |
Separate CLs is great. Thanks for working on this! |
Change https://golang.org/cl/94475 mentions this issue: |
Change https://golang.org/cl/94495 mentions this issue: |
Updates #23834. Change-Id: I92aca9108590a0c7de774f4fad7ded97105e3cb8 Reviewed-on: https://go-review.googlesource.com/94475 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Change https://golang.org/cl/97016 mentions this issue: |
Change https://golang.org/cl/97035 mentions this issue: |
Change https://golang.org/cl/97001 mentions this issue: |
With these last 3 CLs, there are I had changed the original CL to issue Warnl instead of Fatalf so I could see all the issues. |
Previously, finishcompare just used SetTypecheck, but this didn't recursively update any untyped bool typed subexpressions. This CL changes it to call typecheck, which correctly handles this. Also cleaned up outdated code for simplifying logic. Updates #23834 Change-Id: Ic7f92d2a77c2eb74024ee97815205371761c1c90 Reviewed-on: https://go-review.googlesource.com/97035 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Updates #23834. Change-Id: I1789525a992d37aae9e9b69c1e9d91437d3d0d3b Reviewed-on: https://go-review.googlesource.com/97001 Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Updates #23834 Change-Id: If05001f9fd6b97d72069f440102eec6e371908dd Reviewed-on: https://go-review.googlesource.com/97016 Run-TryBot: Kunpei Sakai <namusyaka@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Change https://golang.org/cl/97856 mentions this issue: |
A bit stumped on how to fix the "last" issue I can see. But I cannot seem to easily identify what the generated code is. |
I know where the last fix needs to go and I have an ugly hack to make it all work but just seems wrong. |
You can try adding ssa.Func's Name field to the logging output. It's probably code generated by alg.go (which makes me think the issue is the ORANGE->OFOR loop lowering that I mentioned in CL 97856).
If you're able to identify minimized test cases, I can probably help identify where the missing defaultlits are. |
Its the eq func for pipe using the following: type atomicError struct{ v interface{} } type pipe struct { |
Ohh, I think the issue is OCONVNOP doesn't do type propagation like I thought it does. If you change finishcompare to just:
it looks to handle that test case. (As an actual fix, I'd suggest adding a convnop helper function that does the same thing as conv, but makes sure typecheck turns the newly introduced OCONV into OCONVNOP or OLITERAL.) Does that handle all remaining cases? |
Well, that just ends badly....
|
Added a defaultlit and all looks better. |
When recursively calling walkexpr, r.Type is still the untyped value. It then sometimes recursively calls finishcompare, which complains that you can't compare the resulting expression to that untyped value. Updates #23834. Change-Id: I6b7acd3970ceaff8da9216bfa0ae24aca5dee828 Reviewed-on: https://go-review.googlesource.com/97856 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Change https://golang.org/cl/98295 mentions this issue: |
My last CL will get rid of untyped bools in all sample codes. |
Change https://golang.org/cl/98337 mentions this issue: |
I created CL 94027, which validates that the SSA backend never sees untyped types. This should be safe, because the compiler frontend should eliminate all untyped types during type checking.
However, the CL doesn't currently work, suggesting the frontend is doing something (at least technically) wrong.
The text was updated successfully, but these errors were encountered: