cmd/compile: field reference not working for type parameter with structural constraint #50417
Comments
ianlancetaylor
changed the title
ianlancetaylor
added
the
NeedsInvestigation
|
@ianlancetaylor Thank you for fixing the title 😃 . I should have realized it was the |
|
CC @griesemer It does seem to me that this should be permitted. But even if I'm right it's quite possible that this will not be implemented for 1.18, but will have to wait for a later release. |
|
A smaller repro: https://go.dev/play/p/bJ1QGu1MU38?v=gotip I'm working on a book about generics in Go (self-promotion klaxon), and one of my beta readers sent me more or less exactly this as a bug report. When I looked at it, I was confused too. If the constraint is precisely that T is a struct with an I need to explain in the book why this apparently intuitive idea doesn't work (yet); how do you think I should frame that explanation? Should I say something like "this doesn't work due to a known compiler bug #50417"? Or is it rather a misunderstanding of the type parameters proposal which I can clarify? (Perhaps the spec should clarify it.) Or is it something like "this definitely should work, but it is difficult to implement, so we haven't done it yet—but we're working on it."? |
Oh come on, by just a few lines ;) Anyway, thank you for the follow-up on this as the more traction we get on it, hopefully the more of a chance there will be it is addressed for 1.18 :) |
|
See issue #50233 as well . We currently allow struct initialization of interface types with a single struct type, but not reading/writing of individual fields in expressions/assignments, etc. Still to be decided whether we should support both or not support both for Go 1.18, but we will aim to become consistent. We should be deciding soon. |
|
Thanks @danscales! I strongly prefer option 3 from #50233 (support setting |
|
Per #50233 (comment), I also verified it is currently possible to read a field from a structural constraint, just not set it. Thanks again Dan! |
|
As Ian and I mention in #50233 (comment) and following , reading a field of a type param that has a structural constraint does not work. The example given is not a read of a type param with a structural constraint, but a read of a non-generic type (which has been returned by a generic function call). |
|
Change https://golang.org/cl/375795 mentions this issue: |
|
Change https://golang.org/cl/375794 mentions this issue: |
|
Change https://golang.org/cl/375514 mentions this issue: |
The underlying type of a type parameter is an interface, so we don't need a special case for type parameters anymore. Simply share the (identical) code for interfaces. Adjust code in types.NewMethodSet accordingly. No functional difference. Preparation for fix of issues below. For #50233. For #50417. Change-Id: Ib2deadd12f89e6918dec224b4ce35583001c3101 Reviewed-on: https://go-review.googlesource.com/c/go/+/375514 Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
…aints This change implements field the access p.f where the type of p is a type parameter with a structural constraint that is a struct with a field f. This is only the fix for the type checker. The compiler will need a separate CL. This makes the behavior consistent with the fact that we can write struct composite literals for type parameters with a struct structural type. For #50417. For #50233. Change-Id: I87d07e016f97cbf19c45cde19165eae3ec0bad2b Reviewed-on: https://go-review.googlesource.com/c/go/+/375795 Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
|
This is now implemented on the type-checker side. |
|
Change https://golang.org/cl/376174 mentions this issue: |
|
Change https://golang.org/cl/376194 mentions this issue: |
For #50417. Change-Id: Ic55727c454ec342354f7fbffd22aa350e0d392c2 Reviewed-on: https://go-review.googlesource.com/c/go/+/376174 Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Dan Scales <danscales@google.com> Trust: Dan Scales <danscales@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
|
Should this be working yet in the tip playground? The following example still fails (Golang playground): package main
import (
"fmt"
)
func SomeFunc[S ~struct{ Name string }](s S) {
fmt.Println(s.Name)
}
func main() {
SomeFunc(struct{ Name string }{Name: "fake"})
}The failure is: ./prog.go:12:10: internal compiler error: assertion failedThe full stacktrace can be obtained by running the playground example. |
|
I tried last night and got the same error on tip. I don't see the commit on main yet though. |
|
Thanks for verifying. I am working on a repo called Go generics the hard way and I just added a link to this issue to the page on structural constraints. However, given @griesemer's comment that this was working in the type checker and compiler side (#50233 (comment)), I also thought I would check to see if this was working in the tip playground yet. Thank you again for verifying it is not @bmizerany, I appreciate it. |
|
@akutz It is working in the CL most recently posted. I see |
|
I am sorry, I am confused. Is https://gotipplay.golang.org not using the tip? |
|
The necessary CL has not been submitted yet as of now. This is not working at tip (only type-checking works). |
Ah, I think I understand. It works in that changelist, and you meant the commit has not been merged to My apologies. This is because I just have not used the Go dev builds or CLs in the past. TIL 😃 |
|
Yep, it works like you said @bmizerany, I just had to actually follow the instructions to use the changelist: $ gotip download 376194
This will download and execute code from golang.org/cl/376194, continue? [y/n] y
Fetching CL 376194, Patch Set 11...
remote: Finding sources: 100% (489313/489313)
remote: Total 489313 (delta 354730), reused 480761 (delta 354730)
Receiving objects: 100% (489313/489313), 392.70 MiB | 36.45 MiB/s, done.
Resolving deltas: 100% (354730/354730), done.
From https://go.googlesource.com/go
* branch refs/changes/94/376194/11 -> FETCH_HEAD
Previous HEAD position was 3b5eec9370 runtime/race: be less picky about test run time
HEAD is now at f037b818eb cmd/compile: support field access for typeparam with structural constraint
Building Go cmd/dist using /Users/akutz/.go/active. (go1.18beta1 darwin/amd64)
Building Go toolchain1 using /Users/akutz/.go/active.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for darwin/amd64.
---
Installed Go for darwin/amd64 in /Users/akutz/sdk/gotip
Installed commands in /Users/akutz/sdk/gotip/bin
Success. You may now run 'gotip'!$ cat x.go
package main
import (
"fmt"
)
func SomeFunc[S ~struct{ Name string }](s S) {
fmt.Println(s.Name)
}
func main() {
SomeFunc(struct{ Name string }{Name: "fake"})
}$ gotip run x.go
fakeThanks again to both of you, @bmizerany and @griesemer. |
…raint In the compiler, we need to distinguish field and method access on a type param. For field access, we avoid the dictionary access (to create an interface bound) and just do the normal transformDot() (which will create the field access on the shape type). This field access works fine for non-pointer types, since the shape type preserves the underlying type of all types in the shape. But we generally merge all pointer types into a single shape, which means the field will not be accessible via the shape type. So, we need to change Shapify() so that a type which is a pointer type is mapped to its underlying type, rather than being merged with other pointers. Because we don't want to change the export format at this point in the release, we need to compute StructuralType() directly in types1, rather than relying on types2. That implementation is in types/type.go, along with the helper specificTypes(). I enabled the compiler-related tests in issue50417.go, added an extra test for unnamed pointer types, and added a bunch more tests for interesting cases involving StructuralType(). I added a test issue50417b.go similar to the original example, but also tests access to an embedded field. I also added a unit test in cmd/compile/internal/types/structuraltype_test.go that tests a bunch of unusual cases directly (some of which have no structural type). Updates #50417 Change-Id: I77c55cbad98a2b95efbd4a02a026c07dfbb46caa Reviewed-on: https://go-review.googlesource.com/c/go/+/376194 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org> Trust: Dan Scales <danscales@google.com> Run-TryBot: Dan Scales <danscales@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
|
@danscales , should this issue be closed now that the CL has been merged? |
|
I found a new, possible bug related to this. The following example works fine (Golang playground): package main
import (
"fmt"
)
type Printer[T ~string] struct {
PrintFn func(T)
}
func Print[T ~string](s T) {
fmt.Println(s)
}
func PrintWithPrinter[T ~string, S struct{ PrintFn func(T) }](message T, obj S) {
obj.PrintFn(message)
}
func main() {
PrintWithPrinter(
"Hello, world.",
struct{ PrintFn func(string) }{PrintFn: Print[string]},
)
}However, this example fails to run with the following error (Golang playground): package main
import (
"fmt"
)
// Numeric expresses a type constraint satisfied by any numeric type.
type Numeric interface {
~uint | ~uint8 | ~uint16 | ~uint32 | ~uint64 |
~int | ~int8 | ~int16 | ~int32 | ~int64 |
~float32 | ~float64 |
~complex64 | ~complex128
}
// Sum returns the sum of the provided arguments.
func Sum[T Numeric](args ...T) T {
var sum T
for i := 0; i < len(args); i++ {
sum += args[i]
}
return sum
}
// Ledger is an identifiable, financial record.
type Ledger[T ~string, K Numeric] struct {
// ID identifies the ledger.
ID T
// Amounts is a list of monies associated with this ledger.
Amounts []K
// SumFn is a function that can be used to sum the amounts
// in this ledger.
SumFn func(...K) K
}
func PrintLedger[
T ~string,
K Numeric,
L ~struct {
ID T
Amounts []K
SumFn func(...K) K
},
](l L) {
fmt.Printf("%s has a sum of %v\n", l.ID, l.SumFn(l.Amounts...))
}
func main() {
PrintLedger(Ledger[string, int]{
ID: "fake",
Amounts: []int{1, 2, 3},
SumFn: Sum[int],
})
}The above example compiles as long as I do not call func PrintLedger[
T ~string,
K Numeric,
L ~struct {
ID T
Amounts []K
SumFn func(...K) K
},
](l L) {
fmt.Printf("%s has a sum of %v\n", l.ID, l.Amounts)
} |
|
For reasons discussed in #51576, field accesses through type parameters will be disabled for Go 1.18. |
The underlying type of a type parameter is an interface, so we don't need a special case for type parameters anymore. Simply share the (identical) code for interfaces. Adjust code in types.NewMethodSet accordingly. No functional difference. Preparation for fix of issues below. For golang#50233. For golang#50417. Change-Id: Ib2deadd12f89e6918dec224b4ce35583001c3101 Reviewed-on: https://go-review.googlesource.com/c/go/+/375514 Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
…aints This change implements field the access p.f where the type of p is a type parameter with a structural constraint that is a struct with a field f. This is only the fix for the type checker. The compiler will need a separate CL. This makes the behavior consistent with the fact that we can write struct composite literals for type parameters with a struct structural type. For golang#50417. For golang#50233. Change-Id: I87d07e016f97cbf19c45cde19165eae3ec0bad2b Reviewed-on: https://go-review.googlesource.com/c/go/+/375795 Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
For golang#50417. Change-Id: Ic55727c454ec342354f7fbffd22aa350e0d392c2 Reviewed-on: https://go-review.googlesource.com/c/go/+/376174 Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Dan Scales <danscales@google.com> Trust: Dan Scales <danscales@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
…raint In the compiler, we need to distinguish field and method access on a type param. For field access, we avoid the dictionary access (to create an interface bound) and just do the normal transformDot() (which will create the field access on the shape type). This field access works fine for non-pointer types, since the shape type preserves the underlying type of all types in the shape. But we generally merge all pointer types into a single shape, which means the field will not be accessible via the shape type. So, we need to change Shapify() so that a type which is a pointer type is mapped to its underlying type, rather than being merged with other pointers. Because we don't want to change the export format at this point in the release, we need to compute StructuralType() directly in types1, rather than relying on types2. That implementation is in types/type.go, along with the helper specificTypes(). I enabled the compiler-related tests in issue50417.go, added an extra test for unnamed pointer types, and added a bunch more tests for interesting cases involving StructuralType(). I added a test issue50417b.go similar to the original example, but also tests access to an embedded field. I also added a unit test in cmd/compile/internal/types/structuraltype_test.go that tests a bunch of unusual cases directly (some of which have no structural type). Updates golang#50417 Change-Id: I77c55cbad98a2b95efbd4a02a026c07dfbb46caa Reviewed-on: https://go-review.googlesource.com/c/go/+/376194 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org> Trust: Dan Scales <danscales@google.com> Run-TryBot: Dan Scales <danscales@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
What version of Go are you using (
go version)?Does this issue reproduce with the latest release?
na. This is only currently possible in 1.18beta1.
What operating system and processor architecture are you using (
go env)?go envOutputWhat did you do?
Over the holidays I started digging into Golang generics, and after much consideration, I decided I wanted to create a constraint like so:
It is my understanding the above constraint should be valid based upon the following example from the type parameter proposal (from the section Composite Types in Constraints):
While the above example shows the function is
INVALID, it also implies that such a constraint would be valid if not for the mixed-type ofx. However, when I tried to use this in practice, I received the following error:I was able to replicate this in the Go 1.18 playgound with the following example:
The above program produces the following error:
What did you expect to see?
I expected the program to build and print the line
Andrew.What did you see instead?
The aforementioned compile error:
It would be incredibly useful for this constraint to work as I understand it from the proposal. Perhaps:
Regardless, the reason I would like this to work is as follows:
C Condition[T, S]c CTypefield onc, I can just doc.Type = ...Condition[T, S], that means the struct must have a function likefunc (c *MyCondition) SetType(type3 MyConditionType).MyConditionthat satisfies the constraintCondition[T, S], but*MyCondition~struct {...}and to use those fields directlyThanks!
The text was updated successfully, but these errors were encountered: