]> Cypherpunks repositories - gostls13.git/commitdiff
go/types, types2: use nil to represent incomplete explicit aliases
authorMark Freeman <mark@golang.org>
Tue, 24 Jun 2025 21:16:24 +0000 (17:16 -0400)
committerMark Freeman <mark@golang.org>
Fri, 25 Jul 2025 20:43:32 +0000 (13:43 -0700)
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 <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

src/cmd/compile/internal/types2/alias.go
src/cmd/compile/internal/types2/decl.go
src/go/types/alias.go
src/go/types/alias_test.go [new file with mode: 0644]
src/go/types/decl.go

index 6a6b96a6e3277475263f4693d3ea3e0f3ac2134d..90dda18cc88426437e2dea4913150c31372abe5c 100644 (file)
@@ -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)
 }
index bedcc4c0150aa65d549c8c2c7ea7170fe1384200..64047aa84ff42cbf8716e7e3f4ccd1dbb093b5a1 100644 (file)
@@ -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")
index 3836ce9bb96420387283b27cf79f1558be85fb35..f15ff57030354015c363a9646ae72bffcd8a8678 100644 (file)
@@ -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 (file)
index 0000000..aa12336
--- /dev/null
@@ -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)
+}
index 742191cc1c9e6e3098b2b83561cb12e0669dfe76..f40a8e54b9bf136def16ee999ad8e1c16651ab05 100644 (file)
@@ -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.