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 <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Robert Griesemer <[email protected]>

Author: griesemer

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

src/go/types/validtype.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]