From: Robert Griesemer Date: Thu, 14 Mar 2024 22:45:30 +0000 (-0700) Subject: go/types, types2: do not overwrite nest entries in Checker.validType X-Git-Tag: go1.23rc1~874 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=83b2b47b5da8fd9d713909d0b0a10f5e13d9f177;p=gostls13.git go/types, types2: do not overwrite nest entries in Checker.validType In Checker.validType, when we encounter a type parameter, we evaluate the validity of the respective type argument in the "type nest" of the enclosing type (at the nesting depth at which the type argument was passed) (*). Specifically, we call validType recursively, with the slice representing the type nest shortened by 1. This recursive call continues to use the nest slice and in the process may overwrite the (previously) last entry. Upon return of that recursive call, validType proceeds with the old length, possibly using an incorrect last nest entry. In the concrete example for this issue we have the type S type S[T any] struct { a T b time.Time } instantiated with time.Time. When validType encounters the type parameter T inside the struct (S is in the type nest) it evaluates the type argument (time.Time) in the empty type nest (outside of S). In the process of evaluating the time.Time struct, the time.Time type is appended to the (shortened) nest slice and overwrites the previous last nest entry (S). Once processing of T is done, validType continues with struct field b, using the original-length nest slice, which now has time.Time rather than S as a last element. The type of b has type time.Time, which now appears to be nested in time.Time (rather than S), which (incorrectly) means that there's a type cycle. validType proceeds with reporting the error. But time.Time is an imported type, imported types are correct (otherwise they could not be imported in the first place), and the assertion checking that package of time.Time is local fails. The fix is trivial: restore the last entry of the nest slice when it may have been overwriten. (*) In hindsight we may be able to sigificantly simplify validType by evaluating type arguments when they are passed instead of when the respective type parameters are encountered. For another CL. Fixes #66323. Change-Id: I3bf23acb8ed14d349db342ca5c886323a6c7af58 Reviewed-on: https://go-review.googlesource.com/c/go/+/571836 Reviewed-by: Russ Cox Reviewed-by: Robert Griesemer LUCI-TryBot-Result: Go LUCI Auto-Submit: Robert Griesemer --- diff --git a/src/cmd/compile/internal/types2/validtype.go b/src/cmd/compile/internal/types2/validtype.go index 7397318511..32e389a656 100644 --- a/src/cmd/compile/internal/types2/validtype.go +++ b/src/cmd/compile/internal/types2/validtype.go @@ -158,8 +158,8 @@ func (check *Checker) validType0(pos syntax.Pos, typ Type, nest, path []*Named) // A type parameter stands for the type (argument) it was instantiated with. // Check the corresponding type argument for validity if we are in an // instantiated type. - if len(nest) > 0 { - inst := nest[len(nest)-1] // the type instance + if d := len(nest) - 1; d >= 0 { + inst := nest[d] // the type instance // Find the corresponding type argument for the type parameter // and proceed with checking that type argument. for i, tparam := range inst.TypeParams().list() { @@ -173,7 +173,12 @@ func (check *Checker) validType0(pos syntax.Pos, typ Type, nest, path []*Named) // the current (instantiated) type (see the example // at the end of this file). // For error reporting we keep the full path. - return check.validType0(pos, targ, nest[:len(nest)-1], path) + res := check.validType0(pos, targ, nest[:d], path) + // The check.validType0 call with nest[:d] may have + // overwritten the entry at the current depth d. + // Restore the entry (was issue go.dev/issue/66323). + nest[d] = inst + return res } } } diff --git a/src/go/types/validtype.go b/src/go/types/validtype.go index eae61266de..4fc46faabd 100644 --- a/src/go/types/validtype.go +++ b/src/go/types/validtype.go @@ -160,8 +160,8 @@ func (check *Checker) validType0(pos token.Pos, typ Type, nest, path []*Named) b // A type parameter stands for the type (argument) it was instantiated with. // Check the corresponding type argument for validity if we are in an // instantiated type. - if len(nest) > 0 { - inst := nest[len(nest)-1] // the type instance + if d := len(nest) - 1; d >= 0 { + inst := nest[d] // the type instance // Find the corresponding type argument for the type parameter // and proceed with checking that type argument. for i, tparam := range inst.TypeParams().list() { @@ -175,7 +175,12 @@ func (check *Checker) validType0(pos token.Pos, typ Type, nest, path []*Named) b // the current (instantiated) type (see the example // at the end of this file). // For error reporting we keep the full path. - return check.validType0(pos, targ, nest[:len(nest)-1], path) + res := check.validType0(pos, targ, nest[:d], path) + // The check.validType0 call with nest[:d] may have + // overwritten the entry at the current depth d. + // Restore the entry (was issue go.dev/issue/66323). + nest[d] = inst + return res } } } diff --git a/src/internal/types/testdata/fixedbugs/issue66323.go b/src/internal/types/testdata/fixedbugs/issue66323.go new file mode 100644 index 0000000000..482c094dde --- /dev/null +++ b/src/internal/types/testdata/fixedbugs/issue66323.go @@ -0,0 +1,17 @@ +// Copyright 2024 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 + +import "time" + +// This type declaration must not cause problems with +// the type validity checker. + +type S[T any] struct { + a T + b time.Time +} + +var _ S[time.Time]