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 $<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>

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.

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

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
+// "$<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")

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

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
+}

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

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)
+}

test/typeparam/issue50481.go

@@ -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

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)
+}

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))
+	}
+}

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

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()
+}

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)
+}

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

test/typeparam/issue50481c.out

@@ -0,0 +1 @@
+0