[release-branch.go1.21] 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.

For #62498.
Fixes #62545.

Change-Id: I9c973f5087bf228649a8f74f7dc1522d84a26b51
Reviewed-on: https://go-review.googlesource.com/c/go/+/527135
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit e3ce3126212115808bc248bdc9ad92c0a46436fe)
Reviewed-on: https://go-review.googlesource.com/c/go/+/534916
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

5 files changed, 50 insertions, 31 deletions

diff --git a/src/cmd/compile/internal/typecheck/func.go b/src/cmd/compile/internal/typecheck/func.go
index 1d1de5bf94..47d6c1e09a 100644
--- a/src/cmd/compile/internal/typecheck/func.go
+++ b/src/cmd/compile/internal/typecheck/func.go
@@ -94,40 +94,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)
diff --git a/src/cmd/compile/internal/types/fmt.go b/src/cmd/compile/internal/types/fmt.go
index 0016fb9606..c5d9941cfd 100644
--- a/src/cmd/compile/internal/types/fmt.go
+++ b/src/cmd/compile/internal/types/fmt.go
@@ -642,9 +642,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)
 				}
diff --git a/test/fixedbugs/issue62498.dir/a.go b/test/fixedbugs/issue62498.dir/a.go
new file mode 100644
index 0000000000..68f97475ab
--- /dev/null
+++ b/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) {}
diff --git a/test/fixedbugs/issue62498.dir/main.go b/test/fixedbugs/issue62498.dir/main.go
new file mode 100644
index 0000000000..e55a24fb3a
--- /dev/null
+++ b/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)
+	}()
+}
diff --git a/test/fixedbugs/issue62498.go b/test/fixedbugs/issue62498.go
new file mode 100644
index 0000000000..8fe8d8783f
--- /dev/null
+++ b/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