]> Cypherpunks.ru repositories - gostls13.git/commitdiff
cmd/compile/internal/typecheck: fix closure field naming
authorMatthew Dempsky <mdempsky@google.com>
Sat, 9 Sep 2023 01:04:31 +0000 (18:04 -0700)
committerGopher Robot <gobot@golang.org>
Mon, 11 Sep 2023 16:02:11 +0000 (16:02 +0000)
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 <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>

src/cmd/compile/internal/typecheck/func.go
src/cmd/compile/internal/types/fmt.go
test/fixedbugs/issue62498.dir/a.go [new file with mode: 0644]
test/fixedbugs/issue62498.dir/main.go [new file with mode: 0644]
test/fixedbugs/issue62498.go [new file with mode: 0644]

index 4f076c485406f7097486e2f6afb4523ac236bfa2..3dbe51007c33944b119be3a85c0e9a5d1313993f 100644 (file)
@@ -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)
index f45a44eed950dbfe5711296ae1219cde40a4517f..1b53430af7b1e797199751537d37f4528cd4e049 100644 (file)
@@ -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)
                                }
diff --git a/test/fixedbugs/issue62498.dir/a.go b/test/fixedbugs/issue62498.dir/a.go
new file mode 100644 (file)
index 0000000..68f9747
--- /dev/null
@@ -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 (file)
index 0000000..e55a24f
--- /dev/null
@@ -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 (file)
index 0000000..8fe8d87
--- /dev/null
@@ -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