From 32636cd1ffc6cd9ef81d09b8320d2aaad4a21117 Mon Sep 17 00:00:00 2001 From: Dan Scales Date: Tue, 18 Jan 2022 12:55:17 -0800 Subject: [PATCH] cmd/compile: make sure multiple blank typeparams remain unique 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 $ 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 Trust: Dan Scales --- src/cmd/compile/internal/importer/iimport.go | 11 ++++--- src/cmd/compile/internal/noder/types.go | 9 +++-- src/cmd/compile/internal/typecheck/iexport.go | 31 +++++++++++++++++ src/cmd/compile/internal/types2/decl.go | 11 ------- src/go/internal/gcimporter/iimport.go | 28 ++++++++++++---- src/go/types/decl.go | 13 -------- test/typeparam/issue50419.go | 33 +++++++++++++++++++ test/typeparam/issue50481.go | 21 ------------ test/typeparam/issue50481b.dir/b.go | 16 +++++++++ test/typeparam/issue50481b.dir/main.go | 23 +++++++++++++ test/typeparam/issue50481b.go | 7 ++++ test/typeparam/issue50481c.dir/a.go | 30 +++++++++++++++++ test/typeparam/issue50481c.dir/main.go | 18 ++++++++++ test/typeparam/issue50481c.go | 7 ++++ test/typeparam/issue50481c.out | 1 + 15 files changed, 200 insertions(+), 59 deletions(-) create mode 100644 test/typeparam/issue50419.go delete mode 100644 test/typeparam/issue50481.go create mode 100644 test/typeparam/issue50481b.dir/b.go create mode 100644 test/typeparam/issue50481b.dir/main.go create mode 100644 test/typeparam/issue50481b.go create mode 100644 test/typeparam/issue50481c.dir/a.go create mode 100644 test/typeparam/issue50481c.dir/main.go create mode 100644 test/typeparam/issue50481c.go create mode 100644 test/typeparam/issue50481c.out diff --git a/src/cmd/compile/internal/importer/iimport.go b/src/cmd/compile/internal/importer/iimport.go index 691e9b6976..a827987a48 100644 --- a/src/cmd/compile/internal/importer/iimport.go +++ b/src/cmd/compile/internal/importer/iimport.go @@ -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. diff --git a/src/cmd/compile/internal/noder/types.go b/src/cmd/compile/internal/noder/types.go index 3f3c9566ca..e7ce4c1089 100644 --- a/src/cmd/compile/internal/noder/types.go +++ b/src/cmd/compile/internal/noder/types.go @@ -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 diff --git a/src/cmd/compile/internal/typecheck/iexport.go b/src/cmd/compile/internal/typecheck/iexport.go index 7ebabe7314..ae3c41ca04 100644 --- a/src/cmd/compile/internal/typecheck/iexport.go +++ b/src/cmd/compile/internal/typecheck/iexport.go @@ -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 +// "$". 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") diff --git a/src/cmd/compile/internal/types2/decl.go b/src/cmd/compile/internal/types2/decl.go index 69388f78be..d61d2a8b0d 100644 --- a/src/cmd/compile/internal/types2/decl.go +++ b/src/cmd/compile/internal/types2/decl.go @@ -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 diff --git a/src/go/internal/gcimporter/iimport.go b/src/go/internal/gcimporter/iimport.go index ee8dd0ee7c..8ec4c5413b 100644 --- a/src/go/internal/gcimporter/iimport.go +++ b/src/go/internal/gcimporter/iimport.go @@ -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 +} diff --git a/src/go/types/decl.go b/src/go/types/decl.go index bbd3f04b7e..02af0d5f3e 100644 --- a/src/go/types/decl.go +++ b/src/go/types/decl.go @@ -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 index 0000000000..ff9d08d089 --- /dev/null +++ b/test/typeparam/issue50419.go @@ -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 index 23038356bf..0000000000 --- a/test/typeparam/issue50481.go +++ /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 index 0000000000..d458357c51 --- /dev/null +++ b/test/typeparam/issue50481b.dir/b.go @@ -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 index 0000000000..909d6e43fd --- /dev/null +++ b/test/typeparam/issue50481b.dir/main.go @@ -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 index 0000000000..642f4bf49f --- /dev/null +++ b/test/typeparam/issue50481b.go @@ -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 index 0000000000..384ba23f98 --- /dev/null +++ b/test/typeparam/issue50481c.dir/a.go @@ -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 index 0000000000..4661976034 --- /dev/null +++ b/test/typeparam/issue50481c.dir/main.go @@ -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 index 0000000000..642f4bf49f --- /dev/null +++ b/test/typeparam/issue50481c.go @@ -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 index 0000000000..573541ac97 --- /dev/null +++ b/test/typeparam/issue50481c.out @@ -0,0 +1 @@ +0 -- 2.50.0