From 0f6397384b583a18bae90421a71b6abad39a437f Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 21 Jul 2025 14:35:08 -0400 Subject: [PATCH] go/types: relax NewSignatureType for append(slice, str...) CL 688815 contained a partial fix for the reported bug, but NewSignatureType continued to panic. This change relaxes it to permit construction of the type "func([]byte, B) []byte" where "type B []byte". We must do so because a client may instantiate the type "func([]byte, T...)" where [T ~string|~[]byte] at T=B, and may have no way to know that they are dealing with this very special edge case of append. Added a regression test of NewSignatureType, which I should have done in the earlier CL. Also, make typestring less pedantic and fragile. Fixes #73871 Change-Id: I3d8f8609582149f9c9f8402a04ad516c2c63bbc6 Reviewed-on: https://go-review.googlesource.com/c/go/+/689277 TryBot-Bypass: Alan Donovan Reviewed-by: Robert Findley Auto-Submit: Alan Donovan Reviewed-by: Mark Freeman --- src/cmd/compile/internal/types2/signature.go | 38 +++++++++++++++---- src/cmd/compile/internal/types2/typestring.go | 23 ++++++----- src/go/types/api_test.go | 19 +++++++--- src/go/types/signature.go | 38 +++++++++++++++---- src/go/types/typestring.go | 23 ++++++----- 5 files changed, 101 insertions(+), 40 deletions(-) diff --git a/src/cmd/compile/internal/types2/signature.go b/src/cmd/compile/internal/types2/signature.go index ea60254fa6..d569ba8013 100644 --- a/src/cmd/compile/internal/types2/signature.go +++ b/src/cmd/compile/internal/types2/signature.go @@ -28,17 +28,28 @@ type Signature struct { recv *Var // nil if not a method params *Tuple // (incoming) parameters from left to right; or nil results *Tuple // (outgoing) results from left to right; or nil - variadic bool // true if the last parameter's type is of the form ...T (or string, for append built-in only) + variadic bool // true if the last parameter's type is of the form ...T + + // If variadic, the last element of params ordinarily has an + // unnamed Slice type. As a special case, in a call to append, + // it may be string, or a TypeParam T whose typeset ⊇ {string, []byte}. + // It may even be a named []byte type if a client instantiates + // T at such a type. } // NewSignatureType creates a new function type for the given receiver, // receiver type parameters, type parameters, parameters, and results. +// // If variadic is set, params must hold at least one parameter and the // last parameter must be an unnamed slice or a type parameter whose // type set has an unnamed slice as common underlying type. -// As a special case, for variadic signatures the last parameter may -// also be a string type, or a type parameter containing a mix of byte -// slices and string types in its type set. +// +// As a special case, to support append([]byte, str...), for variadic +// signatures the last parameter may also be a string type, or a type +// parameter containing a mix of byte slices and string types in its +// type set. It may even be a named []byte slice type resulting from +// substitution of such a type parameter. +// // If recv is non-nil, typeParams must be empty. If recvTypeParams is // non-empty, recv must be non-nil. func NewSignatureType(recv *Var, recvTypeParams, typeParams []*TypeParam, params, results *Tuple, variadic bool) *Signature { @@ -54,17 +65,29 @@ func NewSignatureType(recv *Var, recvTypeParams, typeParams []*TypeParam, params if isString(t) { s = NewSlice(universeByte) } else { - s, _ = Unalias(t).(*Slice) // don't accept a named slice type + // Variadic Go functions have a last parameter of type []T, + // suggesting we should reject a named slice type B here. + // + // However, a call to built-in append(slice, x...) + // where x has a TypeParam type [T ~string | ~[]byte], + // has the type func([]byte, T). Since a client may + // instantiate this type at T=B, we must permit + // named slice types, even when this results in a + // signature func([]byte, B) where type B []byte. + // + // (The caller of NewSignatureType may have no way to + // know that it is dealing with the append special case.) + s, _ = t.Underlying().(*Slice) } if S == nil { S = s - } else if !Identical(S, s) { + } else if s == nil || !Identical(S, s) { S = nil break } } if S == nil { - panic(fmt.Sprintf("got %s, want variadic parameter of unnamed slice or string type", last)) + panic(fmt.Sprintf("got %s, want variadic parameter of slice or string type", last)) } } sig := &Signature{recv: recv, params: params, results: results, variadic: variadic} @@ -98,6 +121,7 @@ func (s *Signature) TypeParams() *TypeParamList { return s.tparams } func (s *Signature) RecvTypeParams() *TypeParamList { return s.rparams } // Params returns the parameters of signature s, or nil. +// See [NewSignatureType] for details of variadic functions. func (s *Signature) Params() *Tuple { return s.params } // Results returns the results of signature s, or nil. diff --git a/src/cmd/compile/internal/types2/typestring.go b/src/cmd/compile/internal/types2/typestring.go index b1f0d0929b..e68f6a2886 100644 --- a/src/cmd/compile/internal/types2/typestring.go +++ b/src/cmd/compile/internal/types2/typestring.go @@ -449,22 +449,25 @@ func (w *typeWriter) tuple(tup *Tuple, variadic bool) { } typ := v.typ if variadic && i == len(tup.vars)-1 { - if s, ok := typ.(*Slice); ok { + if slice, ok := typ.(*Slice); ok { w.string("...") - typ = s.elem + w.typ(slice.elem) } else { - // special case: - // append(s, "foo"...) leads to signature func([]byte, string...) - if t, _ := typ.Underlying().(*Basic); t == nil || t.kind != String { - w.error("expected string type") - continue - } + // append(slice, str...) entails various special + // cases, especially in conjunction with generics. + // str may be: + // - a string, + // - a TypeParam whose typeset includes string, or + // - a named []byte slice type B resulting from + // a client instantiating append([]byte, T) at T=B. + // For such cases we use the irregular notation + // func([]byte, T...), with the dots after the type. w.typ(typ) w.string("...") - continue } + } else { + w.typ(typ) } - w.typ(typ) } } w.byte(')') diff --git a/src/go/types/api_test.go b/src/go/types/api_test.go index f31b9d30c5..b5f9cbbed6 100644 --- a/src/go/types/api_test.go +++ b/src/go/types/api_test.go @@ -3180,7 +3180,6 @@ func TestIssue73871(t *testing.T) { func f[T ~[]byte](y T) []byte { return append([]byte(nil), y...) } -// for illustration only: type B []byte var _ = f[B] ` @@ -3196,17 +3195,25 @@ var _ = f[B] // Check type inferred for 'append'. // - // Before the fix, the inferred type of append's y parameter + // Before CL 688815, 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, + if got, want := fmt.Sprint(tAppend), "func([]byte, ...byte) []byte"; got != want { + // Before CL 688815, tAppend was func([]byte, T) []byte, // where T prints as "". - t.Errorf("for append, inferred type %s, want %s", tAppend, want) + t.Errorf("append: got type %s, want %s", got, want) + } + + // Instantiate the type inferred for append(...) at T=B. + // Before the fix in CL 689277, NewSignatureType would panic. + params := slices.Collect(tAppend.Params().Variables()) + params[1] = NewVar(token.NoPos, pkg, "", pkg.Scope().Lookup("B").Type()) + sig := NewSignatureType(nil, nil, nil, NewTuple(params...), tAppend.Results(), true) + if got, want := fmt.Sprint(sig), "func([]byte, p.B...) []byte"; got != want { + t.Errorf("instantiated: got type %s, want %s", got, want) } } diff --git a/src/go/types/signature.go b/src/go/types/signature.go index 2f8be54e17..13f60c0772 100644 --- a/src/go/types/signature.go +++ b/src/go/types/signature.go @@ -29,7 +29,13 @@ type Signature struct { recv *Var // nil if not a method params *Tuple // (incoming) parameters from left to right; or nil results *Tuple // (outgoing) results from left to right; or nil - variadic bool // true if the last parameter's type is of the form ...T (or string, for append built-in only) + variadic bool // true if the last parameter's type is of the form ...T + + // If variadic, the last element of params ordinarily has an + // unnamed Slice type. As a special case, in a call to append, + // it may be string, or a TypeParam T whose typeset ⊇ {string, []byte}. + // It may even be a named []byte type if a client instantiates + // T at such a type. } // NewSignature returns a new function type for the given receiver, parameters, @@ -46,12 +52,17 @@ func NewSignature(recv *Var, params, results *Tuple, variadic bool) *Signature { // NewSignatureType creates a new function type for the given receiver, // receiver type parameters, type parameters, parameters, and results. +// // If variadic is set, params must hold at least one parameter and the // last parameter must be an unnamed slice or a type parameter whose // type set has an unnamed slice as common underlying type. -// As a special case, for variadic signatures the last parameter may -// also be a string type, or a type parameter containing a mix of byte -// slices and string types in its type set. +// +// As a special case, to support append([]byte, str...), for variadic +// signatures the last parameter may also be a string type, or a type +// parameter containing a mix of byte slices and string types in its +// type set. It may even be a named []byte slice type resulting from +// instantiation of such a type parameter. +// // If recv is non-nil, typeParams must be empty. If recvTypeParams is // non-empty, recv must be non-nil. func NewSignatureType(recv *Var, recvTypeParams, typeParams []*TypeParam, params, results *Tuple, variadic bool) *Signature { @@ -67,17 +78,29 @@ func NewSignatureType(recv *Var, recvTypeParams, typeParams []*TypeParam, params if isString(t) { s = NewSlice(universeByte) } else { - s, _ = Unalias(t).(*Slice) // don't accept a named slice type + // Variadic Go functions have a last parameter of type []T, + // suggesting we should reject a named slice type B here. + // + // However, a call to built-in append(slice, x...) + // where x has a TypeParam type [T ~string | ~[]byte], + // has the type func([]byte, T). Since a client may + // instantiate this type at T=B, we must permit + // named slice types, even when this results in a + // signature func([]byte, B) where type B []byte. + // + // (The caller of NewSignatureType may have no way to + // know that it is dealing with the append special case.) + s, _ = t.Underlying().(*Slice) } if S == nil { S = s - } else if !Identical(S, s) { + } else if s == nil || !Identical(S, s) { S = nil break } } if S == nil { - panic(fmt.Sprintf("got %s, want variadic parameter of unnamed slice or string type", last)) + panic(fmt.Sprintf("got %s, want variadic parameter of slice or string type", last)) } } sig := &Signature{recv: recv, params: params, results: results, variadic: variadic} @@ -111,6 +134,7 @@ func (s *Signature) TypeParams() *TypeParamList { return s.tparams } func (s *Signature) RecvTypeParams() *TypeParamList { return s.rparams } // Params returns the parameters of signature s, or nil. +// See [NewSignatureType] for details of variadic functions. func (s *Signature) Params() *Tuple { return s.params } // Results returns the results of signature s, or nil. diff --git a/src/go/types/typestring.go b/src/go/types/typestring.go index bd13459832..766d2b2960 100644 --- a/src/go/types/typestring.go +++ b/src/go/types/typestring.go @@ -452,22 +452,25 @@ func (w *typeWriter) tuple(tup *Tuple, variadic bool) { } typ := v.typ if variadic && i == len(tup.vars)-1 { - if s, ok := typ.(*Slice); ok { + if slice, ok := typ.(*Slice); ok { w.string("...") - typ = s.elem + w.typ(slice.elem) } else { - // special case: - // append(s, "foo"...) leads to signature func([]byte, string...) - if t, _ := typ.Underlying().(*Basic); t == nil || t.kind != String { - w.error("expected string type") - continue - } + // append(slice, str...) entails various special + // cases, especially in conjunction with generics. + // str may be: + // - a string, + // - a TypeParam whose typeset includes string, or + // - a named []byte slice type B resulting from + // a client instantiating append([]byte, T) at T=B. + // For such cases we use the irregular notation + // func([]byte, T...), with the dots after the type. w.typ(typ) w.string("...") - continue } + } else { + w.typ(typ) } - w.typ(typ) } } w.byte(')') -- 2.52.0