go/types: infer correct type for y in append(bytes, y...)

The type-checking logic for append has a special case for
append(bytes, s...) where the typeset for s contains string.
However, this case was triggering even when the typeset contained
only []byte, causing the creation of Signature types of the form
func([]byte, Y) []byte, with the variadic flag set, where Y
is the type of Y (e.g. a type parameter constrained to ~[]byte).
This is an illegal combination: a variadic signature's last
parameter must be a slice, or its typeset must contain string.
This caused x/tools/go/ssa to crash.

This CL narrows the special case to only typesets that contain
string, and adds a test for the inferred signature.
(There's little point in testing that a subsequent NewSignatureType
call would succeed, because the inferred type plainly has
no free type parameters.)

Fixes #73871

Change-Id: Id7641104133371dd6b0077f281cdaa9db84cc1c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/688815
Reviewed-by: Mark Freeman <mark@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

src/cmd/compile/internal/types2/builtins.go

@@ -91,22 +91,25 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
 		// to type []byte with a second argument of string type followed by ... .
 		// This form appends the bytes of the string."
 
-		// get special case out of the way
+		// Handle append(bytes, y...) special case, where
+		// the type set of y is {string} or {string, []byte}.
 		var sig *Signature
 		if nargs == 2 && hasDots(call) {
 			if ok, _ := x.assignableTo(check, NewSlice(universeByte), nil); ok {
 				y := args[1]
+				hasString := false
 				typeset(y.typ, func(_, u Type) bool {
 					if s, _ := u.(*Slice); s != nil && Identical(s.elem, universeByte) {
 						return true
 					}
 					if isString(u) {
+						hasString = true
 						return true
 					}
 					y = nil
 					return false
 				})
-				if y != nil {
+				if y != nil && hasString {
 					// setting the signature also signals that we're done
 					sig = makeSig(x.typ, x.typ, y.typ)
 					sig.variadic = true

src/go/types/api_test.go

@@ -3176,3 +3176,39 @@ func (recv T) f(param int) (result int) {
 		t.Errorf("got:\n%s\nwant:\n%s", got, want)
 	}
 }
+
+func TestIssue73871(t *testing.T) {
+	const src = `package p
+
+func f[T ~[]byte](y T) []byte { return append([]byte(nil), y...) }
+
+// for illustration only:
+type B []byte
+var _ = f[B]
+`
+	fset := token.NewFileSet()
+	f, _ := parser.ParseFile(fset, "p.go", src, 0)
+
+	pkg := NewPackage("p", "p")
+	info := &Info{Types: make(map[ast.Expr]TypeAndValue)}
+	check := NewChecker(&Config{}, fset, pkg, info)
+	if err := check.Files([]*ast.File{f}); err != nil {
+		t.Fatal(err)
+	}
+
+	// Check type inferred for 'append'.
+	//
+	// Before the fix, the inferred type of append's y parameter
+	// was T. When a client such as x/tools/go/ssa instantiated T=B,
+	// it would result in the Signature "func([]byte, B)" with the
+	// variadic flag set, an invalid combination that caused
+	// NewSignatureType to panic.
+	append := f.Decls[0].(*ast.FuncDecl).Body.List[0].(*ast.ReturnStmt).Results[0].(*ast.CallExpr).Fun
+	tAppend := info.TypeOf(append).(*Signature)
+	want := "func([]byte, ...byte) []byte"
+	if got := fmt.Sprint(tAppend); got != want {
+		// Before the fix, tAppend was func([]byte, T) []byte,
+		// where T prints as "<expected string type>".
+		t.Errorf("for append, inferred type %s, want %s", tAppend, want)
+	}
+}

src/go/types/builtins.go

@@ -94,22 +94,25 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
 		// to type []byte with a second argument of string type followed by ... .
 		// This form appends the bytes of the string."
 
-		// get special case out of the way
+		// Handle append(bytes, y...) special case, where
+		// the type set of y is {string} or {string, []byte}.
 		var sig *Signature
 		if nargs == 2 && hasDots(call) {
 			if ok, _ := x.assignableTo(check, NewSlice(universeByte), nil); ok {
 				y := args[1]
+				hasString := false
 				typeset(y.typ, func(_, u Type) bool {
 					if s, _ := u.(*Slice); s != nil && Identical(s.elem, universeByte) {
 						return true
 					}
 					if isString(u) {
+						hasString = true
 						return true
 					}
 					y = nil
 					return false
 				})
-				if y != nil {
+				if y != nil && hasString {
 					// setting the signature also signals that we're done
 					sig = makeSig(x.typ, x.typ, y.typ)
 					sig.variadic = true