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 <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Griesemer <gri@google.com>

diff --git a/src/cmd/compile/internal/types2/validtype.go b/src/cmd/compile/internal/types2/validtype.go
index 7397318511733f82c92488ca549946e5659b7988..32e389a6562445323975a1e18f7ddd2cedcc5a6d 100644 (file)
--- 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 eae61266de0b9ca13cff9d4bdb2e9cfe0a2b54c5..4fc46faabded8e8f3bb0815516976de7c803c5b9 100644 (file)
--- 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 (file)
index 0000000..482c094
--- /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]