go/types: fix bug in premature Unalias of alias cycle

Unalias memoizes the result of removing Alias constructors.
When Unalias is called too soon on a type in a cycle,
the initial value of the alias, Invalid, gets latched by
the memoization, causing it to appear Invalid forever.

This change disables memoization of Invalid, and adds
a regression test.

Fixes #66704
Updates #65294

Change-Id: I479fe14c88c802504a69f177869f091656489cd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/576975
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>

Author: adonovan

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

@@ -24,7 +24,7 @@ type Alias struct {
 func NewAlias(obj *TypeName, rhs Type) *Alias {
 	alias := (*Checker)(nil).newAlias(obj, rhs)
 	// Ensure that alias.actual is set (#65455).
-	unalias(alias)
+	alias.cleanup()
 	return alias
 }
 
@@ -60,7 +60,16 @@ func unalias(a0 *Alias) Type {
 	if t == nil {
 		panic(fmt.Sprintf("non-terminated alias %s", a0.obj.name))
 	}
-	a0.actual = t
+
+	// Memoize the type only if valid.
+	// In the presence of unfinished cyclic declarations, Unalias
+	// would otherwise latch the invalid value (#66704).
+	// TODO(adonovan): rethink, along with checker.typeDecl's use
+	// of Invalid to mark unfinished aliases.
+	if t != Typ[Invalid] {
+		a0.actual = t
+	}
+
 	return t
 }
 
@@ -89,5 +98,13 @@ func (check *Checker) newAlias(obj *TypeName, rhs Type) *Alias {
 }
 
 func (a *Alias) cleanup() {
-	Unalias(a)
+	// Ensure a.actual is set before types are published,
+	// so Unalias is a pure "getter", not a "setter".
+	actual := Unalias(a)
+
+	if actual == Typ[Invalid] {
+		// We don't set a.actual to Typ[Invalid] during type checking,
+		// as it may indicate that the RHS is not fully set up.
+		a.actual = actual
+	}
 }

src/go/types/alias.go

@@ -2994,3 +2994,31 @@ func TestTooNew(t *testing.T) {
 		}
 	}
 }
+
+// This is a regression test for #66704.
+func TestUnaliasTooSoonInCycle(t *testing.T) {
+	t.Setenv("GODEBUG", "gotypesalias=1")
+	const src = `package a
+
+var x T[B] // this appears to cause Unalias to be called on B while still Invalid
+
+type T[_ any] struct{}
+type A T[B]
+type B = T[A]
+`
+	fset := token.NewFileSet()
+	f, err := parser.ParseFile(fset, "a.go", src, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	pkg, err := new(Config).Check("a", fset, []*ast.File{f}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	B := pkg.Scope().Lookup("B")
+	got, want := Unalias(B.Type()).String(), "a.T[a.A]"
+	if got != want {
+		t.Errorf("Unalias(type B = T[A]) = %q, want %q", got, want)
+	}
+}

src/go/types/api_test.go

@@ -587,6 +587,11 @@ func (check *Checker) typeDecl(obj *TypeName, tdecl *ast.TypeSpec, def *TypeName
 			// TODO(gri) Should be able to use nil instead of Typ[Invalid] to mark
 			//           the alias as incomplete. Currently this causes problems
 			//           with certain cycles. Investigate.
+			//
+			// NOTE(adonovan): to avoid the Invalid being prematurely observed
+			// by (e.g.) a var whose type is an unfinished cycle,
+			// Unalias does not memoize if Invalid. Perhaps we should use a
+			// special sentinel distinct from Invalid.
 			alias := check.newAlias(obj, Typ[Invalid])
 			setDefType(def, alias)