From d28b27cd8e612f3e3e6ffcfb7444a4e8829dbb48 Mon Sep 17 00:00:00 2001 From: Mark Freeman Date: Tue, 24 Jun 2025 17:16:24 -0400 Subject: [PATCH] go/types, types2: use nil to represent incomplete explicit aliases Using Invalid to represent an incomplete alias is problematic since it implies that an error has been reported somewhere. This causes confusion for observers of invalid aliases trying not to emit follow-on errors. This change uses nil instead to represent an incomplete alias. This has a mild benefit of making alias memoization more convenient. We additionally can now memoize Invalid aliases. This necessitates a minor change to our cycle error reporting for aliases. Care is taken to separate logic according to gotypesalias. Otherwise, a cycle as simple as "type T = T" panics. A test is also added which uses go/types to inspect for Invalid types. Currently, the problematic Invalid does not cause an error in type checking, but rather a panic in noding. Thus, we cannot use the familiar test facilities relying on error reporting. Fixes #74181 Change-Id: Iea5ebce567a2805f5647de0fb7ded4a96f6c5f8d Reviewed-on: https://go-review.googlesource.com/c/go/+/683796 Reviewed-by: Robert Griesemer LUCI-TryBot-Result: Go LUCI --- src/cmd/compile/internal/types2/alias.go | 32 +++------ src/cmd/compile/internal/types2/decl.go | 36 +++++----- src/go/types/alias.go | 32 +++------ src/go/types/alias_test.go | 85 ++++++++++++++++++++++++ src/go/types/decl.go | 36 +++++----- 5 files changed, 143 insertions(+), 78 deletions(-) create mode 100644 src/go/types/alias_test.go diff --git a/src/cmd/compile/internal/types2/alias.go b/src/cmd/compile/internal/types2/alias.go index 6a6b96a6e3..90dda18cc8 100644 --- a/src/cmd/compile/internal/types2/alias.go +++ b/src/cmd/compile/internal/types2/alias.go @@ -6,7 +6,6 @@ package types2 import ( "cmd/compile/internal/syntax" - "fmt" ) // An Alias represents an alias type. @@ -50,7 +49,7 @@ type Alias struct { } // NewAlias creates a new Alias type with the given type name and rhs. -// rhs must not be nil. +// If rhs is nil, the alias is incomplete. func NewAlias(obj *TypeName, rhs Type) *Alias { alias := (*Checker)(nil).newAlias(obj, rhs) // Ensure that alias.actual is set (#65455). @@ -98,6 +97,7 @@ func (a *Alias) Rhs() Type { return a.fromRHS } // otherwise it follows t's alias chain until it // reaches a non-alias type which is then returned. // Consequently, the result is never an alias type. +// Returns nil if the alias is incomplete. func Unalias(t Type) Type { if a0, _ := t.(*Alias); a0 != nil { return unalias(a0) @@ -113,19 +113,10 @@ func unalias(a0 *Alias) Type { for a := a0; a != nil; a, _ = t.(*Alias) { t = a.fromRHS } - if t == nil { - panic(fmt.Sprintf("non-terminated alias %s", a0.obj.name)) - } - - // 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 - } + // It's fine to memoize nil types since it's the zero value for actual. + // It accomplishes nothing. + a0.actual = t return t } @@ -137,9 +128,8 @@ func asNamed(t Type) *Named { } // newAlias creates a new Alias type with the given type name and rhs. -// rhs must not be nil. +// If rhs is nil, the alias is incomplete. func (check *Checker) newAlias(obj *TypeName, rhs Type) *Alias { - assert(rhs != nil) a := new(Alias) a.obj = obj a.orig = a @@ -172,12 +162,6 @@ func (check *Checker) newAliasInstance(pos syntax.Pos, orig *Alias, targs []Type func (a *Alias) cleanup() { // 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 - } + // so unalias is a pure "getter", not a "setter". + unalias(a) } diff --git a/src/cmd/compile/internal/types2/decl.go b/src/cmd/compile/internal/types2/decl.go index bedcc4c015..64047aa84f 100644 --- a/src/cmd/compile/internal/types2/decl.go +++ b/src/cmd/compile/internal/types2/decl.go @@ -320,11 +320,15 @@ func (check *Checker) cycleError(cycle []Object, start int) { // If obj is a type alias, mark it as valid (not broken) in order to avoid follow-on errors. obj := cycle[start] tname, _ := obj.(*TypeName) - if tname != nil && tname.IsAlias() { - // If we use Alias nodes, it is initialized with Typ[Invalid]. - // TODO(gri) Adjust this code if we initialize with nil. - if !check.conf.EnableAlias { - check.validAlias(tname, Typ[Invalid]) + if tname != nil { + if check.conf.EnableAlias { + if a, ok := tname.Type().(*Alias); ok { + a.fromRHS = Typ[Invalid] + } + } else { + if tname.IsAlias() { + check.validAlias(tname, Typ[Invalid]) + } } } @@ -507,17 +511,18 @@ func (check *Checker) typeDecl(obj *TypeName, tdecl *syntax.TypeDecl, def *TypeN } if check.conf.EnableAlias { - // 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]) + alias := check.newAlias(obj, nil) setDefType(def, alias) + // If we could not type the RHS, set it to invalid. This should + // only ever happen if we panic before setting. + defer func() { + if alias.fromRHS == nil { + alias.fromRHS = Typ[Invalid] + unalias(alias) + } + }() + // handle type parameters even if not allowed (Alias type is supported) if tparam0 != nil { if !versionErr && !buildcfg.Experiment.AliasTypeParams { @@ -531,8 +536,9 @@ func (check *Checker) typeDecl(obj *TypeName, tdecl *syntax.TypeDecl, def *TypeN rhs = check.definedType(tdecl.Type, obj) assert(rhs != nil) + alias.fromRHS = rhs - Unalias(alias) // resolve alias.actual + unalias(alias) // resolve alias.actual } else { if !versionErr && tparam0 != nil { check.error(tdecl, UnsupportedFeature, "generic type alias requires GODEBUG=gotypesalias=1 or unset") diff --git a/src/go/types/alias.go b/src/go/types/alias.go index 3836ce9bb9..f15ff57030 100644 --- a/src/go/types/alias.go +++ b/src/go/types/alias.go @@ -8,7 +8,6 @@ package types import ( - "fmt" "go/token" ) @@ -53,7 +52,7 @@ type Alias struct { } // NewAlias creates a new Alias type with the given type name and rhs. -// rhs must not be nil. +// If rhs is nil, the alias is incomplete. func NewAlias(obj *TypeName, rhs Type) *Alias { alias := (*Checker)(nil).newAlias(obj, rhs) // Ensure that alias.actual is set (#65455). @@ -101,6 +100,7 @@ func (a *Alias) Rhs() Type { return a.fromRHS } // otherwise it follows t's alias chain until it // reaches a non-alias type which is then returned. // Consequently, the result is never an alias type. +// Returns nil if the alias is incomplete. func Unalias(t Type) Type { if a0, _ := t.(*Alias); a0 != nil { return unalias(a0) @@ -116,19 +116,10 @@ func unalias(a0 *Alias) Type { for a := a0; a != nil; a, _ = t.(*Alias) { t = a.fromRHS } - if t == nil { - panic(fmt.Sprintf("non-terminated alias %s", a0.obj.name)) - } - - // 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 - } + // It's fine to memoize nil types since it's the zero value for actual. + // It accomplishes nothing. + a0.actual = t return t } @@ -140,9 +131,8 @@ func asNamed(t Type) *Named { } // newAlias creates a new Alias type with the given type name and rhs. -// rhs must not be nil. +// If rhs is nil, the alias is incomplete. func (check *Checker) newAlias(obj *TypeName, rhs Type) *Alias { - assert(rhs != nil) a := new(Alias) a.obj = obj a.orig = a @@ -175,12 +165,6 @@ func (check *Checker) newAliasInstance(pos token.Pos, orig *Alias, targs []Type, func (a *Alias) cleanup() { // 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 - } + // so unalias is a pure "getter", not a "setter". + unalias(a) } diff --git a/src/go/types/alias_test.go b/src/go/types/alias_test.go new file mode 100644 index 0000000000..aa12336e21 --- /dev/null +++ b/src/go/types/alias_test.go @@ -0,0 +1,85 @@ +// Copyright 2025 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 types_test + +import ( + "go/ast" + "go/parser" + "go/token" + "go/types" + "testing" +) + +func TestIssue74181(t *testing.T) { + t.Setenv("GODEBUG", "gotypesalias=1") + + src := `package p + +type AB = A[B] + +type _ struct { + _ AB +} + +type B struct { + f *AB +} + +type A[T any] struct{}` + + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "p.go", src, parser.ParseComments) + if err != nil { + t.Fatalf("could not parse: %v", err) + } + + conf := types.Config{} + pkg, err := conf.Check(file.Name.Name, fset, []*ast.File{file}, &types.Info{}) + if err != nil { + t.Fatalf("could not type check: %v", err) + } + + b := pkg.Scope().Lookup("B").Type() + if n, ok := b.(*types.Named); ok { + if s, ok := n.Underlying().(*types.Struct); ok { + got := s.Field(0).Type() + want := types.NewPointer(pkg.Scope().Lookup("AB").Type()) + if !types.Identical(got, want) { + t.Errorf("wrong type for f: got %v, want %v", got, want) + } + return + } + } + t.Errorf("unexpected type for B: %v", b) +} + +func TestPartialTypeCheckUndeclaredAliasPanic(t *testing.T) { + t.Setenv("GODEBUG", "gotypesalias=1") + + src := `package p + +type A = B // undeclared` + + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "p.go", src, parser.ParseComments) + if err != nil { + t.Fatalf("could not parse: %v", err) + } + + conf := types.Config{} // no error handler, panic + pkg, _ := conf.Check(file.Name.Name, fset, []*ast.File{file}, &types.Info{}) + a := pkg.Scope().Lookup("A").Type() + + if alias, ok := a.(*types.Alias); ok { + got := alias.Rhs() + want := types.Typ[types.Invalid] + + if !types.Identical(got, want) { + t.Errorf("wrong type for B: got %v, want %v", got, want) + } + return + } + t.Errorf("unexpected type for A: %v", a) +} diff --git a/src/go/types/decl.go b/src/go/types/decl.go index 742191cc1c..f40a8e54b9 100644 --- a/src/go/types/decl.go +++ b/src/go/types/decl.go @@ -321,11 +321,15 @@ func (check *Checker) cycleError(cycle []Object, start int) { // If obj is a type alias, mark it as valid (not broken) in order to avoid follow-on errors. obj := cycle[start] tname, _ := obj.(*TypeName) - if tname != nil && tname.IsAlias() { - // If we use Alias nodes, it is initialized with Typ[Invalid]. - // TODO(gri) Adjust this code if we initialize with nil. - if !check.conf._EnableAlias { - check.validAlias(tname, Typ[Invalid]) + if tname != nil { + if check.conf._EnableAlias { + if a, ok := tname.Type().(*Alias); ok { + a.fromRHS = Typ[Invalid] + } + } else { + if tname.IsAlias() { + check.validAlias(tname, Typ[Invalid]) + } } } @@ -582,17 +586,18 @@ func (check *Checker) typeDecl(obj *TypeName, tdecl *ast.TypeSpec, def *TypeName } if check.conf._EnableAlias { - // 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]) + alias := check.newAlias(obj, nil) setDefType(def, alias) + // If we could not type the RHS, set it to invalid. This should + // only ever happen if we panic before setting. + defer func() { + if alias.fromRHS == nil { + alias.fromRHS = Typ[Invalid] + unalias(alias) + } + }() + // handle type parameters even if not allowed (Alias type is supported) if tparam0 != nil { if !versionErr && !buildcfg.Experiment.AliasTypeParams { @@ -606,8 +611,9 @@ func (check *Checker) typeDecl(obj *TypeName, tdecl *ast.TypeSpec, def *TypeName rhs = check.definedType(tdecl.Type, obj) assert(rhs != nil) + alias.fromRHS = rhs - Unalias(alias) // resolve alias.actual + unalias(alias) // resolve alias.actual } else { // With Go1.23, the default behavior is to use Alias nodes, // reflected by check.enableAlias. Signal non-default behavior. -- 2.51.0