cmd/compile/internal/typecheck: fix closure field naming

When creating the struct type to hold variables captured by a function
literal, we currently reuse the captured variable names as fields.

However, there's no particular reason to do this: these struct types
aren't visible to users, and it adds extra complexity in making sure
fields belong to the correct packages.

Further, it turns out we were getting that subtly wrong. If two
function literals from different packages capture variables with
identical names starting with an uppercase letter (and in the same
order and with corresponding identical types) end up in the same
function (e.g., due to inlining), then we could end up creating
closure struct types that are "different" (i.e., not types.Identical)
yet end up with equal LinkString representations (which violates
LinkString's contract).

The easy fix is to just always use simple, exported, generated field
names in the struct. This should allow further struct reuse across
packages too, and shrink binary sizes slightly.

Fixes #62498.

Change-Id: I9c973f5087bf228649a8f74f7dc1522d84a26b51
Reviewed-on: https://go-review.googlesource.com/c/go/+/527135
Auto-Submit: Matthew Dempsky <[email protected]>
Reviewed-by: Cuong Manh Le <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: mdempsky

src/cmd/compile/internal/typecheck/func.go

@@ -91,40 +91,24 @@ func ClosureType(clo *ir.ClosureExpr) *types.Type {
 	// and has one float64 argument and no results,
 	// the generated code looks like:
 	//
-	//	clos = &struct{.F uintptr; i *int; s *string}{func.1, &i, &s}
+	//	clos = &struct{F uintptr; X0 *int; X1 *string}{func.1, &i, &s}
 	//
 	// The use of the struct provides type information to the garbage
-	// collector so that it can walk the closure. We could use (in this case)
-	// [3]unsafe.Pointer instead, but that would leave the gc in the dark.
-	// The information appears in the binary in the form of type descriptors;
-	// the struct is unnamed so that closures in multiple packages with the
-	// same struct type can share the descriptor.
-
-	// Make sure the .F field is in the same package as the rest of the
-	// fields. This deals with closures in instantiated functions, which are
-	// compiled as if from the source package of the generic function.
-	var pkg *types.Pkg
-	if len(clo.Func.ClosureVars) == 0 {
-		pkg = types.LocalPkg
-	} else {
-		for _, v := range clo.Func.ClosureVars {
-			if pkg == nil {
-				pkg = v.Sym().Pkg
-			} else if pkg != v.Sym().Pkg {
-				base.Fatalf("Closure variables from multiple packages: %+v", clo)
-			}
-		}
-	}
-
-	fields := []*types.Field{
-		types.NewField(base.Pos, pkg.Lookup(".F"), types.Types[types.TUINTPTR]),
-	}
-	for _, v := range clo.Func.ClosureVars {
+	// collector so that it can walk the closure. We could use (in this
+	// case) [3]unsafe.Pointer instead, but that would leave the gc in
+	// the dark. The information appears in the binary in the form of
+	// type descriptors; the struct is unnamed and uses exported field
+	// names so that closures in multiple packages with the same struct
+	// type can share the descriptor.
+
+	fields := make([]*types.Field, 1+len(clo.Func.ClosureVars))
+	fields[0] = types.NewField(base.AutogeneratedPos, types.LocalPkg.Lookup("F"), types.Types[types.TUINTPTR])
+	for i, v := range clo.Func.ClosureVars {
 		typ := v.Type()
 		if !v.Byval() {
 			typ = types.NewPtr(typ)
 		}
-		fields = append(fields, types.NewField(base.Pos, v.Sym(), typ))
+		fields[1+i] = types.NewField(base.AutogeneratedPos, types.LocalPkg.LookupNum("X", i), typ)
 	}
 	typ := types.NewStruct(fields)
 	typ.SetNoalg(true)

src/cmd/compile/internal/types/fmt.go

@@ -633,9 +633,6 @@ func fldconv(b *bytes.Buffer, f *Field, verb rune, mode fmtMode, visited map[*Ty
 				name = fmt.Sprint(f.Nname)
 			} else if verb == 'L' {
 				name = s.Name
-				if name == ".F" {
-					name = "F" // Hack for toolstash -cmp.
-				}
 				if !IsExported(name) && mode != fmtTypeIDName {
 					name = sconv(s, 0, mode) // qualify non-exported names (used on structs, not on funarg)
 				}

test/fixedbugs/issue62498.dir/a.go

@@ -0,0 +1,13 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package a
+
+func One(L any) {
+	func() {
+		defer F(L)
+	}()
+}
+
+func F(any) {}

test/fixedbugs/issue62498.dir/main.go

@@ -0,0 +1,18 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import "./a"
+
+func main() {
+	a.One(nil)
+	Two(nil)
+}
+
+func Two(L any) {
+	func() {
+		defer a.F(L)
+	}()
+}

test/fixedbugs/issue62498.go

@@ -0,0 +1,7 @@
+// rundir
+
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package ignored