Golang go/types,cmd/compile/internal/types2: shared type expressions are evaluated once per variable

If a VarDecl has an explicit type expression, it will get evaluated once for each declared variable. So if you have a VarDecl like var x, y interface { M() } you end up with two different *types2.Interface objects.

I ran into this because I wanted to map all types2.Type objects (that appeared in source) back to some syntactic element, so my initial approach was to just walk the AST looking for type expressions and then using Info.Types[expr].Type to get the types2.Type object and insert them into a map[types2.Type]syntax.Expr map.

However, this approach missed some types, because of VarDecls with multiple variables and an explicit type expression.

I don't think this is strictly a violation of the types2 API (you're supposed to use types2.Identical), but I think it would be nice and less surprising if there was a 1:1 correspondance.

I assume this also affects go/types.

/cc @griesemer @findleyr

Comment From: findleyr

Indeed this affects both go/types and types2. Here's the relevant TODO: https://cs.opensource.google/go/go/+/master:src/cmd/compile/internal/types2/decl.go;l=464;drc=a9705e157beb51574233e23cc2e2a412d4681a15

I think this could be done, but we should probably handle decls with init exprs as well, which IIRC would be a non-trivial refactoring.

Comment From: mdempsky

~~I discovered this issue also affects struct fields (struct { a, b T }) and function signatures (func(a, b T)).~~

~~It presumably in theory also affects constant declarations, but if a constant declaration includes an explicit type expression, then it must be a defined type. So they'll always have a unique types.Type instance (right?).~~

Edit: Disregard. It appears that the problem I ran into with structs and signatures is that types2 evaluates the field type expressions just once each, but cmd/compile was evaluating them multiple times.

Comment From: mdempsky

For fun, I created these two test cases:

package p

var x struct {
a, b, c, d, e, f, g, h struct {
a, b, c, d, e, f, g, h struct {
a, b, c, d, e, f, g, h struct {
a, b, c, d, e, f, g, h struct {
a, b, c, d, e, f, g, h struct {
a, b, c, d, e, f, g, h struct {
a, b, c, d, e, f, g, h struct {
a, b, c, d, e, f, g, h struct {
}}}}}}}}}

package p

import "unsafe"

var a, b, c, d, e, f, g, h [unsafe.Sizeof(func() {
var a, b, c, d, e, f, g, h [unsafe.Sizeof(func() {
var a, b, c, d, e, f, g, h [unsafe.Sizeof(func() {
var a, b, c, d, e, f, g, h [unsafe.Sizeof(func() {
var a, b, c, d, e, f, g, h [unsafe.Sizeof(func() {
var a, b, c, d, e, f, g, h [unsafe.Sizeof(func() {
var a, b, c, d, e, f, g, h [unsafe.Sizeof(func() {
})]byte; b, c, d, e, f, g, h, a = a, b, c, d, e, f, g, h
})]byte; b, c, d, e, f, g, h, a = a, b, c, d, e, f, g, h
})]byte; b, c, d, e, f, g, h, a = a, b, c, d, e, f, g, h
})]byte; b, c, d, e, f, g, h, a = a, b, c, d, e, f, g, h
})]byte; b, c, d, e, f, g, h, a = a, b, c, d, e, f, g, h
})]byte; b, c, d, e, f, g, h, a = a, b, c, d, e, f, g, h
})]byte

cmd/compile and gccgo both appear to go exponential on the first test. go/types handles it instantly.

cmd/compile handles the second one instantly. gccgo and go/types take somewhat longer, but still complete within a few seconds. (To be fair to gccgo, I'm running it from a network file system, and it takes a few seconds to compile even a file with just package p.)

I tried a few more minor variations on the idea, but couldn't figure out a way to make go/types go exponential.

Comment From: gopherbot

Change https://golang.org/cl/320789 mentions this issue: [dev.typeparams] cmd/compile: avoid some redundant type construction

Comment From: ianlancetaylor

@mdempsky This is in the 1.18 milestone; time to move to 1.19? Thanks.

Comment From: ianlancetaylor

@mdempsky This is in the 1.19 milestone; time to move to 1.20? Or Backlog? Thanks.