cmd/compile: field reference not working for type parameter with structural constraint

[akutz]

What version of Go are you using (go version)?

$ go version
go version go1.18beta1 darwin/amd64

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 env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/akutz/Library/Caches/go-build"
GOENV="/Users/akutz/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/akutz/Projects/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/akutz/Projects/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/akutz/.go/active"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/akutz/.go/active/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.18beta1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/akutz/Projects/controller-runtime/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/0c/ffr4wl4d37924s44ctztx_ym00bgms/T/go-build1780269713=/tmp/go-build -gno-record-gcc-switches -fno-common"

What 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:

type Condition[T ~string, S ~string] interface {
	~struct{
		Type               T
		Status             string
		Severity           S
		LastTransitionTime int64
		Reason             string
		Message            string
	}
}

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

// structField is a type constraint whose type set consists of some
// struct types that all have a field named x.
type structField interface {
	struct { a int; x int } |
		struct { b int; x float64 } |
		struct { c int; x uint64 }
}

// This function is INVALID.
func IncrementX[T structField](p *T) {
	v := p.x // INVALID: type of p.x is not the same for all types in set
	v++
	p.x = v
}

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 of x. However, when I tried to use this in practice, I received the following error:

c.Type undefined (interface Condition has no method Type)

I was able to replicate this in the Go 1.18 playgound with the following example:

package main

import (
	"fmt"
)

type Cat struct {
	Name string
}

type Dog struct {
	Name string
}

type Person struct {
	Name string
}

type Named interface {
	~struct{ Name string }
}

func SayName[T Named](t T) {
	fmt.Println(t.Name)
}

func main() {
	SayName(Person{Name: "Andrew"})
}

The above program produces the following error:

./prog.go:24:16: t.Name undefined (interface Named has no method Name)

Go build failed.

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:

./prog.go:24:16: t.Name undefined (interface Named has no method Name)

Go build failed.

It would be incredibly useful for this constraint to work as I understand it from the proposal. Perhaps:

• I misunderstood the proposal
• The proposal is not up-to-date with the implementation
• The implementation is incomplete in 1.18beta1

Regardless, the reason I would like this to work is as follows:

• Using the above example, imagine C Condition[T, S]
• Now imagine a value of c C
• If I want to set the Type field on c, I can just do c.Type = ...
• But without the above constraint, I would have to change to something like the following:

  type Condition[T ~string, S ~string] interface {
  	SetType(type3 T)
  }

• The problem with the above is that if I have a struct that implements the interface Condition[T, S], that means the struct must have a function like func (c *MyCondition) SetType(type3 MyConditionType).
• The receiver for the setter function is necessarily by address
• Which means it is not MyCondition that satisfies the constraint Condition[T, S], but *MyCondition
• This greatly complicates things, and it would be much nicer to constrain based on the ~struct {...} and to use those fields directly

Thanks!

[akutz]

@ianlancetaylor Thank you for fixing the title 😃 . I should have realized it was the cmd/compile package, doh!

[ianlancetaylor]

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.

[bitfield]

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 int field x, then shouldn't we be able to set the x field on a T parameter? It certainly does seem that we should. Indeed, if we can't reference a struct field in a generic function constrained on that struct, that rather limits the usefulness of such functions! And, by extension, constraints involving struct types.

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."?

[akutz]

smaller repro

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

[danscales]

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.

[akutz]

Thanks @danscales! I strongly prefer option 3 from #50233 (support setting x.b1) as it can avoid constraints using interfaces with mutating functions (i.e. setters), which necessarily have to be defined on an address, not the struct itself. This results in the situation where new(T) ends up being **T instead of just *T. A structural constraint would also enable var t T to define a new instance of T, where such behavior would not work for constraints expressed with interfaces.

[akutz]

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!

[danscales]

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

[gopherbot]

Change https://golang.org/cl/375795 mentions this issue: cmd/compile/internal/types2: implement field access for struct structural constraints

[gopherbot]

Change https://golang.org/cl/375794 mentions this issue: cmd/compile/internal/types2: minor restructuring of lookup code in call

[gopherbot]

Change https://golang.org/cl/375514 mentions this issue: cmd/compile/internal/types2: remove unused code in lookupFieldOrMethod

[griesemer]

This is now implemented on the type-checker side.

[gopherbot]

Change https://golang.org/cl/376174 mentions this issue: test/typeparam: adjust test preamble (fix longtests)

[gopherbot]

Change https://golang.org/cl/376194 mentions this issue: cmd/compile: support field access for typeparam with structural constraint

[akutz]

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 failed

The full stacktrace can be obtained by running the playground example.

[bmizerany]

I tried last night and got the same error on tip. I don't see the commit on main yet though.

[akutz]

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.

[bmizerany]

@akutz It is working in the CL most recently posted. I see Andrew when running with gotip download 376194 && gotip run x.go

[akutz]

I am sorry, I am confused. Is https://gotipplay.golang.org not using the tip?

[griesemer]

The necessary CL has not been submitted yet as of now. This is not working at tip (only type-checking works).
cc: @danscales

[akutz]

I don't see the commit on main yet though.

Ah, I think I understand. It works in that changelist, and you meant the commit has not been merged to main which the "tip" playground uses. I was confused because you then said gotip download, and I just ignored the number after that. It was not until I tried that I realized it was downloading a specific changelist (hence the CL).

My apologies. This is because I just have not used the Go dev builds or CLs in the past. TIL 😃

[akutz]

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       
fake

Thanks again to both of you, @bmizerany and @griesemer.

[aclements]

@danscales , should this issue be closed now that the CL has been merged?

[akutz]

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]},
	)
}

Hello, world.

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],
	})
}

./prog.go:47:52: internal compiler error: panic: runtime error: invalid memory address or nil pointer dereference

goroutine 1 [running]:
runtime/debug.Stack()
	/usr/local/go/src/runtime/debug/stack.go:24 +0x65
cmd/compile/internal/base.FatalfAt({0x61e784?, 0x0?}, {0xd1f694, 0x9}, {0xc0003f0ed0, 0x1, 0x1})
	/usr/local/go/src/cmd/compile/internal/base/print.go:227 +0x1ca
cmd/compile/internal/base.Fatalf(...)
	/usr/local/go/src/cmd/compile/internal/base/print.go:196
cmd/compile/internal/gc.handlePanic()
	/usr/local/go/src/cmd/compile/internal/gc/main.go:49 +0x85
panic({0xca9440, 0x130cdf0})
	/usr/local/go/src/runtime/panic.go:838 +0x207
cmd/compile/internal/noder.(*genInst).getDictionarySym(0x1395e00, 0xc0003cf930, {0xc0003daea0, 0x3, 0x3}, 0x0)
	/usr/local/go/src/cmd/compile/internal/noder/stencil.go:1615 +0xf29
cmd/compile/internal/noder.(*genInst).getDictionaryValue(0x2?, {0x3daac8?, 0xc0?}, 0xc0003daeb8?, {0xc0003daea0?, 0xc0003bf4a0?, 0xc0003daee8?}, 0x50?)
	/usr/local/go/src/cmd/compile/internal/noder/stencil.go:1763 +0x39
cmd/compile/internal/noder.(*genInst).getDictOrSubdict(0x1395e00?, 0xc0003cf930?, {0xe970f8?, 0xc000310a20?}, 0xc0003f13a8?, {0xc0003daea0, 0x3, 0x3}, 0x0?)
	/usr/local/go/src/cmd/compile/internal/noder/stencil.go:601 +0xdc
cmd/compile/internal/noder.(*genInst).scanForGenCalls.func1({0xe970f8, 0xc000310a20?})
	/usr/local/go/src/cmd/compile/internal/noder/stencil.go:152 +0x32f
cmd/compile/internal/ir.Visit.func1({0xe970f8, 0xc000310a20})
	/usr/local/go/src/cmd/compile/internal/ir/visit.go:105 +0x30
cmd/compile/internal/ir.doNodes(...)
	/usr/local/go/src/cmd/compile/internal/ir/node_gen.go:1512
cmd/compile/internal/ir.(*Func).doChildren(0xe97a58?, 0xc0003dae88?)
	/usr/local/go/src/cmd/compile/internal/ir/func.go:152 +0x6e
cmd/compile/internal/ir.DoChildren(...)
	/usr/local/go/src/cmd/compile/internal/ir/visit.go:94
cmd/compile/internal/ir.Visit.func1({0xe97a58, 0xc0003bf1e0})
	/usr/local/go/src/cmd/compile/internal/ir/visit.go:106 +0x57
cmd/compile/internal/ir.Visit({0xe97a58, 0xc0003bf1e0}, 0xc0003e6d80)
	/usr/local/go/src/cmd/compile/internal/ir/visit.go:108 +0xb8
cmd/compile/internal/noder.(*genInst).scanForGenCalls(0x1395e00, {0xe97a58, 0xc0003bf1e0})
	/usr/local/go/src/cmd/compile/internal/noder/stencil.go:133 +0x1f0
cmd/compile/internal/noder.(*genInst).buildInstantiations(0x1395e00)
	/usr/local/go/src/cmd/compile/internal/noder/stencil.go:60 +0x69
cmd/compile/internal/noder.BuildInstantiations(...)
	/usr/local/go/src/cmd/compile/internal/noder/stencil.go:44
cmd/compile/internal/noder.(*irgen).generate(0xc0000fe240, {0xc0000ac440, 0x1, 0x203000?})
	/usr/local/go/src/cmd/compile/internal/noder/irgen.go:331 +0x3d1
cmd/compile/internal/noder.check2({0xc0000ac440, 0x1, 0x1})
	/usr/local/go/src/cmd/compile/internal/noder/irgen.go:92 +0x16d
cmd/compile/internal/noder.LoadPackage({0xc0000b20f0, 0x1, 0x0?})
	/usr/local/go/src/cmd/compile/internal/noder/noder.go:90 +0x335
cmd/compile/internal/gc.Main(0xd4f6d0)
	/usr/local/go/src/cmd/compile/internal/gc/main.go:191 +0xb13
main.main()
	/usr/local/go/src/cmd/compile/main.go:55 +0xdd

The above example compiles as long as I do not call l.SumFn, even though it is passed into the struct (Golang playground):

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

fake has a sum of [1 2 3]

[danscales]

OK, I closed this issue, since the original example and several other examples are now fixed. @akutz I filed your latest test case as a new issue #50690 (which has lower priority - it's quite an unusual case).

[griesemer]

For reasons discussed in #51576, field accesses through type parameters will be disabled for Go 1.18.