-
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
x/tools/go/ssa: cannot convert slice to generic type that is both array and pointer to array #56849
Comments
One can trigger the same issue on Go 1.19 with array pointers and slices:
|
Thanks for the report. We can also take strings into account. So I am aware of 25 sequence conversion cases with type parameters that type check at tip:
Might still be missing something. Some of the above like |
My main point is that representing something like
is challenging with the current set of A possible solution is to have a "MagicConvert" instruction that just handles all of these cases. MagicConvert could just be a relaxation of the current Convert instruction, or something new. Another possible solutions is use a type switch of the type of D. This seems to require A less important point is 'that is a lot of sequence conversion cases'. I found this slightly surprising. |
I've considered a type switch approach, but I am worried that this will make analysis of generic functions more difficult for end users, as the SSA will be more removed from the source representation. The switch statement will introduce new phi nodes, and more (impossible) paths through the program, for example if the type switch is inside a loop. I haven't yet come up with anything better than MagicConvert (GenericConvert?). It's probably also worth keeping ChangeType in mind. |
All fair points. My gut reaction from issue is that maybe I'm not really sure which approach will be better in the long term.
This is a good name and maybe a decent way to approach scoping what the instruction would do.
+1. I don't think ChangeType should be touched. Tough maybe it is emitted in fewer places. |
In our defense, we didn't expect generics to be this generic :P And I think the instruction is still very useful in combination with instantiation, or with type sets where all types permit SliceToArrayPointer to be used. I don't think that our existing instructions are too specific, only that we may not have ones that are general enough for generic functions.
I wasn't thinking of changing it. My point was more that ChangeType is one more semantic that GenericConvert might have if we extend the type sets above, in addition to SliceToArrayPointer, SliceToArrayPointer + dereference, and Convert. |
Change https://go.dev/cl/457335 mentions this issue: |
Change https://go.dev/cl/457436 mentions this issue: |
My 2c: GenericConvert seems simpler, generates less code, and feels consistent with how other operations have been extended. E.g., append and copy on bytestring don't type switch to separately handle []byte and string. |
Adds a new MultiConvert instruction. MultiConvert instructions are a catch all for conversions involving a typeparameter that would result in multiple different types of conversion instruction [sequences]. Updates golang/go#56849 Change-Id: I6c2f53ccef1b933406096d6ca2867f1007a13bd3 Reviewed-on: https://go-review.googlesource.com/c/tools/+/457436 gopls-CI: kokoro <noreply+kokoro@google.com> Run-TryBot: Tim King <taking@google.com> Reviewed-by: Alan Donovan <adonovan@google.com> Reviewed-by: Zvonimir Pavlinovic <zpavlinovic@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
The Go compiler at tip accepts this function
but x/tools/go/ssa fails to build a generic function for it:
/cc @timothy-king
The text was updated successfully, but these errors were encountered: