]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: make sure multiple blank typeparams remain unique
authorDan Scales <danscales@google.com>
Tue, 18 Jan 2022 20:55:17 +0000 (12:55 -0800)
committerDan Scales <danscales@google.com>
Fri, 21 Jan 2022 00:39:55 +0000 (00:39 +0000)
In a method declaration "func (f *Foo[_, _]) String() string { ... }",
the two blank typeparams have the same name, but our current design with
types1 needs unique names for type params. Similarly, for export/import,
we need unique names to keep the type params straight in generic types
and connect the proper type param with the proper constraint. We make
blank type params unique by changing them to $1, $2, etc in noder.typ0()
via typecheck.TparamExportName(). We then revert $<num> back to _ during
type2 import via typecheck.TparamName(). We similarly revert
during gcimporter import. We don't need/want to revert in the types1
importer, since we want unique names for type params.

Rob Findley has made a similar change to x/tools (and we tried to make
the source code changes similar for the gcimporter and types2 importer
changes).

Fixes #50419

Change-Id: I855cc3d90d06bcf59541ed0c879e9a0e4ede45bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/379194
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Dan Scales <danscales@google.com>

15 files changed:
src/cmd/compile/internal/importer/iimport.go
src/cmd/compile/internal/noder/types.go
src/cmd/compile/internal/typecheck/iexport.go
src/cmd/compile/internal/types2/decl.go
src/go/internal/gcimporter/iimport.go
src/go/types/decl.go
test/typeparam/issue50419.go [new file with mode: 0644]
test/typeparam/issue50481.go [deleted file]
test/typeparam/issue50481b.dir/b.go [new file with mode: 0644]
test/typeparam/issue50481b.dir/main.go [new file with mode: 0644]
test/typeparam/issue50481b.go [new file with mode: 0644]
test/typeparam/issue50481c.dir/a.go [new file with mode: 0644]
test/typeparam/issue50481c.dir/main.go [new file with mode: 0644]
test/typeparam/issue50481c.go [new file with mode: 0644]
test/typeparam/issue50481c.out [new file with mode: 0644]

index 691e9b697656bcf4d2df5ad59100df5855aec4c1..a827987a48701bf968b63cb569fa5f384dab5d08 100644 (file)
@@ -9,6 +9,7 @@ package importer
 
 import (
        "cmd/compile/internal/syntax"
+       "cmd/compile/internal/typecheck"
        "cmd/compile/internal/types2"
        "encoding/binary"
        "fmt"
@@ -376,12 +377,12 @@ func (r *importReader) obj(name string) {
                if r.p.exportVersion < iexportVersionGenerics {
                        errorf("unexpected type param type")
                }
-               // Remove the "path" from the type param name that makes it unique
-               ix := strings.LastIndex(name, ".")
-               if ix < 0 {
-                       errorf("missing path for type param")
+               name0 := typecheck.TparamName(name)
+               if name0 == "" {
+                       errorf("malformed type parameter export name %s: missing prefix", name)
                }
-               tn := types2.NewTypeName(pos, r.currPkg, name[ix+1:], nil)
+
+               tn := types2.NewTypeName(pos, r.currPkg, name0, nil)
                t := types2.NewTypeParam(tn, nil)
                // To handle recursive references to the typeparam within its
                // bound, save the partial type in tparamIndex before reading the bounds.
index 3f3c9566ca12fd414f050974af9b468d76dce502..e7ce4c1089ff2f96018f89c62beb656a7febd634 100644 (file)
@@ -239,10 +239,13 @@ func (g *irgen) typ0(typ types2.Type) *types.Type {
                // Save the name of the type parameter in the sym of the type.
                // Include the types2 subscript in the sym name
                pkg := g.tpkg(typ)
-               // Create the unique types1 name for a type param, using its context with a
-               // function, type, or method declaration.
+               // Create the unique types1 name for a type param, using its context
+               // with a function, type, or method declaration. Also, map blank type
+               // param names to a unique name based on their type param index. The
+               // unique blank names will be exported, but will be reverted during
+               // types2 and gcimporter import.
                assert(g.curDecl != "")
-               nm := g.curDecl + "." + typ.Obj().Name()
+               nm := typecheck.TparamExportName(g.curDecl, typ.Obj().Name(), typ.Index())
                sym := pkg.Lookup(nm)
                if sym.Def != nil {
                        // Make sure we use the same type param type for the same
index 7ebabe7314536694e9f13e7cc974c5d6eba4f0af..ae3c41ca04304c9dc99b991c816f80dc35d65ddd 100644 (file)
@@ -243,6 +243,7 @@ import (
        "io"
        "math/big"
        "sort"
+       "strconv"
        "strings"
 
        "cmd/compile/internal/base"
@@ -730,6 +731,36 @@ func (w *exportWriter) qualifiedIdent(n *ir.Name) {
        w.pkg(s.Pkg)
 }
 
+const blankMarker = "$"
+
+// TparamExportName creates a unique name for type param in a method or a generic
+// type, using the specified unique prefix and the index of the type param. The index
+// is only used if the type param is blank, in which case the blank is replace by
+// "$<index>". A unique name is needed for later substitution in the compiler and
+// export/import that keeps blank type params associated with the correct constraint.
+func TparamExportName(prefix string, name string, index int) string {
+       if name == "_" {
+               name = blankMarker + strconv.Itoa(index)
+       }
+       return prefix + "." + name
+}
+
+// TparamName returns the real name of a type parameter, after stripping its
+// qualifying prefix and reverting blank-name encoding. See TparamExportName
+// for details.
+func TparamName(exportName string) string {
+       // Remove the "path" from the type param name that makes it unique.
+       ix := strings.LastIndex(exportName, ".")
+       if ix < 0 {
+               return ""
+       }
+       name := exportName[ix+1:]
+       if strings.HasPrefix(name, blankMarker) {
+               return "_"
+       }
+       return name
+}
+
 func (w *exportWriter) selector(s *types.Sym) {
        if w.currPkg == nil {
                base.Fatalf("missing currPkg")
index 69388f78be37248f6ac99fce5180ecec39c4d9e4..d61d2a8b0d98ad99a409b56d5afc8af30aba62ce 100644 (file)
@@ -632,19 +632,8 @@ func (check *Checker) collectTypeParams(dst **TypeParamList, list []*syntax.Fiel
        // Declare type parameters up-front.
        // The scope of type parameters starts at the beginning of the type parameter
        // list (so we can have mutually recursive parameterized type bounds).
-       nblanks := 0
        for i, f := range list {
                tparams[i] = check.declareTypeParam(f.Name)
-               // Issue #50481: For now, disallow multiple blank type parameters because
-               // it causes problems with export data. Report an error unless we are in
-               // testing mode ("assert" is defined).
-               // We expect to lift this restriction for Go 1.19.
-               if f.Name.Value == "_" {
-                       nblanks++
-                       if nblanks == 2 && Universe.Lookup("assert") == nil {
-                               check.softErrorf(f, "cannot have multiple blank type parameters (temporary restriction, see issue #50481)")
-                       }
-               }
        }
 
        // Set the type parameters before collecting the type constraints because
index ee8dd0ee7c38805d4f2ddc53eddd82587ae24989..8ec4c5413ba4bbaf3c38a787b6697fc80c657511 100644 (file)
@@ -369,12 +369,10 @@ func (r *importReader) obj(name string) {
                if r.p.exportVersion < iexportVersionGenerics {
                        errorf("unexpected type param type")
                }
-               // Remove the "path" from the type param name that makes it unique
-               ix := strings.LastIndex(name, ".")
-               if ix < 0 {
-                       errorf("missing path for type param")
-               }
-               tn := types.NewTypeName(pos, r.currPkg, name[ix+1:], nil)
+               // Remove the "path" from the type param name that makes it unique,
+               // and revert any unique name used for blank typeparams.
+               name0 := tparamName(name)
+               tn := types.NewTypeName(pos, r.currPkg, name0, nil)
                t := types.NewTypeParam(tn, nil)
                // To handle recursive references to the typeparam within its
                // bound, save the partial type in tparamIndex before reading the bounds.
@@ -772,3 +770,21 @@ func baseType(typ types.Type) *types.Named {
        n, _ := typ.(*types.Named)
        return n
 }
+
+const blankMarker = "$"
+
+// tparamName returns the real name of a type parameter, after stripping its
+// qualifying prefix and reverting blank-name encoding. See tparamExportName
+// for details.
+func tparamName(exportName string) string {
+       // Remove the "path" from the type param name that makes it unique.
+       ix := strings.LastIndex(exportName, ".")
+       if ix < 0 {
+               errorf("malformed type parameter export name %s: missing prefix", exportName)
+       }
+       name := exportName[ix+1:]
+       if strings.HasPrefix(name, blankMarker) {
+               return "_"
+       }
+       return name
+}
index bbd3f04b7e9a9093deb52e3a68c14ad6bea8b14c..02af0d5f3ea2113978bb8aa3efeb0699d1a52f75 100644 (file)
@@ -684,21 +684,8 @@ func (check *Checker) collectTypeParams(dst **TypeParamList, list *ast.FieldList
        // Declare type parameters up-front, with empty interface as type bound.
        // The scope of type parameters starts at the beginning of the type parameter
        // list (so we can have mutually recursive parameterized interfaces).
-       nblanks := 0
        for _, f := range list.List {
                tparams = check.declareTypeParams(tparams, f.Names)
-               // Issue #50481: For now, disallow multiple blank type parameters because
-               // it causes problems with export data. Report an error unless we are in
-               // testing mode ("assert" is defined).
-               // We expect to lift this restriction for Go 1.19.
-               for _, name := range f.Names {
-                       if name.Name == "_" {
-                               nblanks++
-                               if nblanks == 2 && Universe.Lookup("assert") == nil {
-                                       check.softErrorf(name, _InvalidBlank, "cannot have multiple blank type parameters (temporary restriction, see issue #50481)")
-                               }
-                       }
-               }
        }
 
        // Set the type parameters before collecting the type constraints because
diff --git a/test/typeparam/issue50419.go b/test/typeparam/issue50419.go
new file mode 100644 (file)
index 0000000..ff9d08d
--- /dev/null
@@ -0,0 +1,33 @@
+// run -gcflags=-G=3
+
+// Copyright 2022 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.
+
+// Test that type substitution works correctly even for a method of a generic type
+// that has multiple blank type params.
+
+package main
+
+import (
+       "fmt"
+)
+
+func main() {
+       foo := &Foo[string, int]{
+               valueA: "i am a string",
+               valueB: 123,
+       }
+       if got, want := fmt.Sprintln(foo), "i am a string 123\n"; got != want {
+               panic(fmt.Sprintf("got %s, want %s", got, want))
+       }
+}
+
+type Foo[T1 any, T2 any] struct {
+       valueA T1
+       valueB T2
+}
+
+func (f *Foo[_, _]) String() string {
+       return fmt.Sprintf("%v %v", f.valueA, f.valueB)
+}
diff --git a/test/typeparam/issue50481.go b/test/typeparam/issue50481.go
deleted file mode 100644 (file)
index 2303835..0000000
+++ /dev/null
@@ -1,21 +0,0 @@
-// errorcheck -G=3
-
-// Copyright 2022 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 p
-
-type _[_ any] struct{}
-type _[_, _ any] struct{}             // ERROR "cannot have multiple blank type parameters"
-type _[_, _, _ any] struct{}          // ERROR "cannot have multiple blank type parameters"
-type _[a, _, b, _, c, _ any] struct{} // ERROR "cannot have multiple blank type parameters"
-
-func _[_ any]()                {}
-func _[_, _ any]()             {} // ERROR "cannot have multiple blank type parameters"
-func _[_, _, _ any]()          {} // ERROR "cannot have multiple blank type parameters"
-func _[a, _, b, _, c, _ any]() {} // ERROR "cannot have multiple blank type parameters"
-
-type S[P1, P2 any] struct{}
-
-func (_ S[_, _]) m() {} // this is ok
diff --git a/test/typeparam/issue50481b.dir/b.go b/test/typeparam/issue50481b.dir/b.go
new file mode 100644 (file)
index 0000000..d458357
--- /dev/null
@@ -0,0 +1,16 @@
+// Copyright 2022 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 b
+
+import "fmt"
+
+type Foo[T1 ~string, T2 ~int] struct {
+       ValueA T1
+       ValueB T2
+}
+
+func (f *Foo[_, _]) String() string {
+       return fmt.Sprintf("%v %v", f.ValueA, f.ValueB)
+}
diff --git a/test/typeparam/issue50481b.dir/main.go b/test/typeparam/issue50481b.dir/main.go
new file mode 100644 (file)
index 0000000..909d6e4
--- /dev/null
@@ -0,0 +1,23 @@
+// Copyright 2022 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.
+
+// Test that type substitution and export/import works correctly even for a method of
+// a generic type that has multiple blank type params.
+
+package main
+
+import (
+       "b"
+       "fmt"
+)
+
+func main() {
+       foo := &b.Foo[string, int]{
+               ValueA: "i am a string",
+               ValueB: 123,
+       }
+       if got, want := fmt.Sprintln(foo), "i am a string 123\n"; got != want {
+               panic(fmt.Sprintf("got %s, want %s", got, want))
+       }
+}
diff --git a/test/typeparam/issue50481b.go b/test/typeparam/issue50481b.go
new file mode 100644 (file)
index 0000000..642f4bf
--- /dev/null
@@ -0,0 +1,7 @@
+// rundir -G=3
+
+// Copyright 2022 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
diff --git a/test/typeparam/issue50481c.dir/a.go b/test/typeparam/issue50481c.dir/a.go
new file mode 100644 (file)
index 0000000..384ba23
--- /dev/null
@@ -0,0 +1,30 @@
+// Copyright 2022 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
+
+type A interface {
+       int | int64
+}
+
+type B interface {
+       string
+}
+
+type C interface {
+       String() string
+}
+
+type Myint int
+
+func (i Myint) String() string {
+       return "aa"
+}
+
+type T[P A, _ C, _ B] int
+
+func (v T[P, Q, R]) test() {
+       var r Q
+       r.String()
+}
diff --git a/test/typeparam/issue50481c.dir/main.go b/test/typeparam/issue50481c.dir/main.go
new file mode 100644 (file)
index 0000000..4661976
--- /dev/null
@@ -0,0 +1,18 @@
+// Copyright 2022 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.
+
+// Test that type substitution works and export/import works correctly even for a
+// generic type that has multiple blank type params.
+
+package main
+
+import (
+       "a"
+       "fmt"
+)
+
+func main() {
+       var x a.T[int, a.Myint, string]
+       fmt.Printf("%v\n", x)
+}
diff --git a/test/typeparam/issue50481c.go b/test/typeparam/issue50481c.go
new file mode 100644 (file)
index 0000000..642f4bf
--- /dev/null
@@ -0,0 +1,7 @@
+// rundir -G=3
+
+// Copyright 2022 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
diff --git a/test/typeparam/issue50481c.out b/test/typeparam/issue50481c.out
new file mode 100644 (file)
index 0000000..573541a
--- /dev/null
@@ -0,0 +1 @@
+0