]> Cypherpunks repositories - gostls13.git/commitdiff
go/types, types2: replace pendingType with completion check
authorMark Freeman <mark@golang.org>
Thu, 8 Jan 2026 21:06:45 +0000 (16:06 -0500)
committerGopher Robot <gobot@golang.org>
Tue, 27 Jan 2026 15:52:02 +0000 (07:52 -0800)
This change establishes the invariant that Underlying() cannot observe
a nil RHS for a defined type, unless that type was created by go/types
with an explicitly nil underlying type.

It does so using isComplete, which is a guard to check that a type has
an underlying type. This guard is needed whenever we could produce a
value of a defined type or access some property of a defined type.

Examples include T{}, *x (where x has type *T), T.x, etc. (see CL
734600 for more).

The pendingType mechanism to deeply traverse values of a defined type
is moved to hasVarSize, since this is only truly needed at the site
of a built-in such as unsafe.Sizeof.

This ties cycle detection across value context directly to the syntax,
which seems like the right direction.

Change-Id: Ic47862a2038fb2ba3ae6621e9907265ccbd86ea3
Reviewed-on: https://go-review.googlesource.com/c/go/+/734980
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Mark Freeman <markfreeman@google.com>

19 files changed:
src/cmd/compile/internal/types2/builtins.go
src/cmd/compile/internal/types2/call.go
src/cmd/compile/internal/types2/cycles.go
src/cmd/compile/internal/types2/decl.go
src/cmd/compile/internal/types2/expr.go
src/cmd/compile/internal/types2/index.go
src/cmd/compile/internal/types2/literals.go
src/cmd/compile/internal/types2/named.go
src/go/types/builtins.go
src/go/types/call.go
src/go/types/cycles.go
src/go/types/decl.go
src/go/types/expr.go
src/go/types/index.go
src/go/types/literals.go
src/go/types/named.go
src/internal/types/testdata/fixedbugs/issue52915.go
src/internal/types/testdata/fixedbugs/issue75918.go
src/internal/types/testdata/fixedbugs/issue76478.go

index 549d94615bcceca653ebe0646489f4fd7ce9a766..b11bacdecf8e8c81a7d6b10c362d6749467d37ce 100644 (file)
@@ -752,7 +752,7 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
                        return
                }
 
-               if hasVarSize(x.typ, nil) {
+               if check.hasVarSize(x.typ) {
                        x.mode = value
                        if check.recordTypes() {
                                check.recordBuiltinType(call.Fun, makeSig(Typ[Uintptr], x.typ))
@@ -816,7 +816,7 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
                // the part of the struct which is variable-sized. This makes both the rules
                // simpler and also permits (or at least doesn't prevent) a compiler from re-
                // arranging struct fields if it wanted to.
-               if hasVarSize(base, nil) {
+               if check.hasVarSize(base) {
                        x.mode = value
                        if check.recordTypes() {
                                check.recordBuiltinType(call.Fun, makeSig(Typ[Uintptr], obj.Type()))
@@ -840,7 +840,7 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
                        return
                }
 
-               if hasVarSize(x.typ, nil) {
+               if check.hasVarSize(x.typ) {
                        x.mode = value
                        if check.recordTypes() {
                                check.recordBuiltinType(call.Fun, makeSig(Typ[Uintptr], x.typ))
@@ -1007,37 +1007,56 @@ func sliceElem(x *operand) (Type, *typeError) {
 // hasVarSize reports if the size of type t is variable due to type parameters
 // or if the type is infinitely-sized due to a cycle for which the type has not
 // yet been checked.
-func hasVarSize(t Type, seen map[*Named]bool) (varSized bool) {
-       // Cycles are only possible through *Named types.
-       // The seen map is used to detect cycles and track
-       // the results of previously seen types.
-       if named := asNamed(t); named != nil {
-               if v, ok := seen[named]; ok {
-                       return v
-               }
-               if seen == nil {
-                       seen = make(map[*Named]bool)
-               }
-               seen[named] = true // possibly cyclic until proven otherwise
-               defer func() {
-                       seen[named] = varSized // record final determination for named
-               }()
-       }
+func (check *Checker) hasVarSize(t Type) bool {
+       // Note: We could use Underlying here, but passing through the RHS may yield
+       // better error messages.
+       switch t := Unalias(t).(type) {
+       case *Named:
+               if t.stateHas(hasFinite) {
+                       // TODO(mark): Rename t.finite to t.varSize to avoid inversion.
+                       return !t.finite
+               }
+
+               if i, ok := check.objPathIdx[t.obj]; ok {
+                       cycle := check.objPath[i:]
+                       check.cycleError(cycle, firstInSrc(cycle))
+                       return true
+               }
+
+               check.push(t.obj)
+               defer check.pop()
+
+               varSize := check.hasVarSize(t.fromRHS)
+
+               t.mu.Lock()
+               defer t.mu.Unlock()
+
+               // Careful, t.finite has lock-free readers. Since we might be racing
+               // another call to finiteSize, we have to avoid overwriting t.finite.
+               // Otherwise, the race detector will be tripped.
+               if !t.stateHas(hasFinite) {
+                       t.finite = !varSize
+                       t.setState(hasFinite)
+               }
+
+               return varSize
 
-       switch u := t.Underlying().(type) {
        case *Array:
-               return hasVarSize(u.elem, seen)
+               // The array length is already computed. If it was a valid length, it
+               // is finite; else, an error was reported in the computation.
+               return check.hasVarSize(t.elem)
+
        case *Struct:
-               for _, f := range u.fields {
-                       if hasVarSize(f.typ, seen) {
+               for _, f := range t.fields {
+                       if check.hasVarSize(f.typ) {
                                return true
                        }
                }
-       case *Interface:
-               return isTypeParam(t)
-       case *Named, *Union:
-               panic("unreachable")
+
+       case *TypeParam:
+               return true
        }
+
        return false
 }
 
index c23a1048f23d7d8c69bc633e2196ed26501acf22..cf950959f9f7cc68b4ee42eff58635d93b43e3c9 100644 (file)
@@ -199,6 +199,11 @@ func (check *Checker) callExpr(x *operand, call *syntax.CallExpr) exprKind {
                }
                T := x.typ
                x.mode = invalid
+               // We cannot convert a value to an incomplete type; make sure it's complete.
+               if !check.isComplete(T) {
+                       x.expr = call
+                       return conversion
+               }
                switch n := len(call.ArgList); n {
                case 0:
                        check.errorf(call, WrongArgCount, "missing argument in conversion to %s", T)
@@ -319,7 +324,14 @@ func (check *Checker) callExpr(x *operand, call *syntax.CallExpr) exprKind {
                } else {
                        x.mode = value
                }
-               x.typ = sig.results.vars[0].typ // unpack tuple
+               typ := sig.results.vars[0].typ // unpack tuple
+               // We cannot return a value of an incomplete type; make sure it's complete.
+               if !check.isComplete(typ) {
+                       x.mode = invalid
+                       x.expr = call
+                       return statement
+               }
+               x.typ = typ
        default:
                x.mode = value
                x.typ = sig.results
@@ -784,8 +796,12 @@ func (check *Checker) selector(x *operand, e *syntax.SelectorExpr, wantType bool
                goto Error
        }
 
-       // Avoid crashing when checking an invalid selector in a method declaration
-       // (i.e., where def is not set):
+       // We cannot select on an incomplete type; make sure it's complete.
+       if !check.isComplete(x.typ) {
+               goto Error
+       }
+
+       // Avoid crashing when checking an invalid selector in a method declaration.
        //
        //   type S[T any] struct{}
        //   type V = S[any]
@@ -795,14 +811,17 @@ func (check *Checker) selector(x *operand, e *syntax.SelectorExpr, wantType bool
        // expecting a type expression, it is an error.
        //
        // See go.dev/issue/57522 for more details.
-       //
-       // TODO(rfindley): We should do better by refusing to check selectors in all cases where
-       // x.typ is incomplete.
        if wantType {
                check.errorf(e.Sel, NotAType, "%s is not a type", syntax.Expr(e))
                goto Error
        }
 
+       // Additionally, if x.typ is a pointer type, selecting implicitly dereferences the value, meaning
+       // its base type must also be complete.
+       if p, ok := x.typ.Underlying().(*Pointer); ok && !check.isComplete(p.base) {
+               goto Error
+       }
+
        obj, index, indirect = lookupFieldOrMethod(x.typ, x.mode == variable, check.pkg, sel, false)
        if obj == nil {
                // Don't report another error if the underlying type was invalid (go.dev/issue/49541).
index 84a05c96474b5666387e2c938c4744b31d4d81f4..14bd7f2630bea0c432d78ff877291406c17634d3 100644 (file)
@@ -103,49 +103,25 @@ func (check *Checker) directCycle(tname *TypeName, pathIdx map[*TypeName]int) {
        }
 }
 
-// TODO(markfreeman): Can the value cached on Named be used in validType / hasVarSize?
-
-// finiteSize returns whether a type has finite size.
-func (check *Checker) finiteSize(t Type) bool {
-       switch t := Unalias(t).(type) {
-       case *Named:
-               if t.stateHas(hasFinite) {
-                       return t.finite
-               }
-
-               if i, ok := check.objPathIdx[t.obj]; ok {
+// isComplete returns whether a type is complete (i.e. up to having an underlying type).
+// Incomplete types will panic if [Type.Underlying] is called on them.
+func (check *Checker) isComplete(t Type) bool {
+       if n, ok := Unalias(t).(*Named); ok {
+               if i, found := check.objPathIdx[n.obj]; found {
                        cycle := check.objPath[i:]
                        check.cycleError(cycle, firstInSrc(cycle))
                        return false
                }
-               check.push(t.obj)
-               defer check.pop()
-
-               isFinite := check.finiteSize(t.fromRHS)
-
-               t.mu.Lock()
-               defer t.mu.Unlock()
-               // Careful, t.finite has lock-free readers. Since we might be racing
-               // another call to finiteSize, we have to avoid overwriting t.finite.
-               // Otherwise, the race detector will be tripped.
-               if !t.stateHas(hasFinite) {
-                       t.finite = isFinite
-                       t.setState(hasFinite)
-               }
-
-               return isFinite
 
-       case *Array:
-               // The array length is already computed. If it was a valid length, it
-               // is finite; else, an error was reported in the computation.
-               return check.finiteSize(t.elem)
-
-       case *Struct:
-               for _, f := range t.fields {
-                       if !check.finiteSize(f.typ) {
-                               return false
-                       }
-               }
+               // We must walk through names because we permit certain cycles of names.
+               // Consider:
+               //
+               //   type A B
+               //   type B [unsafe.Sizeof(A{})]int
+               //
+               // starting at B. At the site of A{}, A has no underlying type, and so a
+               // cycle must be reported.
+               return check.isComplete(n.fromRHS)
        }
 
        return true
index be9c63bc8cab0785b97523f359d6f017e4e06afb..97d486cc5a6f83916901a09569529127386a706f 100644 (file)
@@ -483,20 +483,6 @@ func (check *Checker) typeDecl(obj *TypeName, tdecl *syntax.TypeDecl) {
        }
 
        named := check.newNamed(obj, nil, nil)
-
-       // TODO: adjust this comment (gotypesalias) as needed if we don't need allowNilRHS anymore.
-       // The RHS of a named N can be nil if, for example, N is defined as a cycle of aliases with
-       // gotypesalias=0. Consider:
-       //
-       //   type D N    // N.unpack() will panic
-       //   type N A
-       //   type A = N  // N.fromRHS is not set before N.unpack(), since A does not call setDefType
-       //
-       // There is likely a better way to detect such cases, but it may not be worth the effort.
-       // Instead, we briefly permit a nil N.fromRHS while type-checking D.
-       named.allowNilRHS = true
-       defer (func() { named.allowNilRHS = false })()
-
        if tdecl.TParamList != nil {
                check.openScope(tdecl, "type parameters")
                defer check.closeScope()
index e3ef1af1ce35ca33f1aedab4bbffbb1a576653d4..81aeb5ffd00ceffc019265709cc1423ad970d621 100644 (file)
@@ -148,7 +148,8 @@ func (check *Checker) unary(x *operand, e *syntax.Operation) {
                return
 
        case syntax.Recv:
-               if elem := check.chanElem(x, x, true); elem != nil {
+               // We cannot receive a value with an incomplete type; make sure it's complete.
+               if elem := check.chanElem(x, x, true); elem != nil && check.isComplete(elem) {
                        x.mode = commaok
                        x.typ = elem
                        check.hasCallOrRecv = true
@@ -993,13 +994,6 @@ func (check *Checker) rawExpr(T *target, x *operand, e syntax.Expr, hint Type, a
                check.nonGeneric(T, x)
        }
 
-       // Here, x is a value, meaning it has a type. If that type is pending, then we have
-       // a cycle. As an example:
-       //
-       //  type T [unsafe.Sizeof(T{})]int
-       //
-       // has a cycle T->T which is deemed valid (by decl.go), but which is in fact invalid.
-       check.pendingType(x)
        check.record(x)
 
        return kind
@@ -1034,19 +1028,6 @@ func (check *Checker) nonGeneric(T *target, x *operand) {
        }
 }
 
-// If x has a pending type (i.e. its declaring object is on the object path), pendingType
-// reports an error and invalidates x.mode and x.typ.
-// Otherwise it leaves x alone.
-func (check *Checker) pendingType(x *operand) {
-       if x.mode == invalid || x.mode == novalue {
-               return
-       }
-       if !check.finiteSize(x.typ) {
-               x.mode = invalid
-               x.typ = Typ[Invalid]
-       }
-}
-
 // exprInternal contains the core of type checking of expressions.
 // Must only be called by rawExpr.
 // (See rawExpr for an explanation of the parameters.)
@@ -1140,6 +1121,10 @@ func (check *Checker) exprInternal(T *target, x *operand, e syntax.Expr, hint Ty
                if !isValid(T) {
                        goto Error
                }
+               // We cannot assert to an incomplete type; make sure it's complete.
+               if !check.isComplete(T) {
+                       goto Error
+               }
                check.typeAssertion(e, x, T, false)
                x.mode = commaok
                x.typ = T
@@ -1207,6 +1192,10 @@ func (check *Checker) exprInternal(T *target, x *operand, e syntax.Expr, hint Ty
                                        }) {
                                                goto Error
                                        }
+                                       // We cannot dereference a pointer with an incomplete base type; make sure it's complete.
+                                       if !check.isComplete(base) {
+                                               goto Error
+                                       }
                                        x.mode = variable
                                        x.typ = base
                                }
index ca84184d35afedd6b793a783b2bc9cc19a3540e7..98d83833c82e1f70fd9b8404294fb86204674f3d 100644 (file)
@@ -47,6 +47,18 @@ func (check *Checker) indexExpr(x *operand, e *syntax.IndexExpr) (isFuncInst boo
                return false
        }
 
+       // We cannot index on an incomplete type; make sure it's complete.
+       if !check.isComplete(x.typ) {
+               x.mode = invalid
+               return false
+       }
+       // Additionally, if x.typ is a pointer to an array type, indexing implicitly dereferences the value, meaning
+       // its base type must also be complete.
+       if p, ok := x.typ.Underlying().(*Pointer); ok && !check.isComplete(p.base) {
+               x.mode = invalid
+               return false
+       }
+
        // ordinary index expression
        valid := false
        length := int64(-1) // valid if >= 0
@@ -251,6 +263,14 @@ func (check *Checker) sliceExpr(x *operand, e *syntax.SliceExpr) {
                }
        }
 
+       // Note that we don't permit slice expressions where x is a type expression, so we don't check for that here.
+       // However, if x.typ is a pointer to an array type, slicing implicitly dereferences the value, meaning
+       // its base type must also be complete.
+       if p, ok := x.typ.Underlying().(*Pointer); ok && !check.isComplete(p.base) {
+               x.mode = invalid
+               return
+       }
+
        valid := false
        length := int64(-1) // valid if >= 0
        switch u := cu.(type) {
index ed1c3f695c8807d0fe5409b65a8d6b6d70caf33c..6817231a763dcf139bc11c382e8539f04d0b4b31 100644 (file)
@@ -143,6 +143,12 @@ func (check *Checker) compositeLit(x *operand, e *syntax.CompositeLit, hint Type
                base = typ
        }
 
+       // We cannot create a literal of an incomplete type; make sure it's complete.
+       if !check.isComplete(base) {
+               x.mode = invalid
+               return
+       }
+
        switch u, _ := commonUnder(base, nil); utyp := u.(type) {
        case *Struct:
                if len(e.ElemList) == 0 {
index 2922fb55eba270d66730c46a635944ddd78269d7..77e958139f7c6bcdac7dca2fb3c87c9b3679dbf1 100644 (file)
@@ -106,9 +106,7 @@ type Named struct {
        check *Checker  // non-nil during type-checking; nil otherwise
        obj   *TypeName // corresponding declared object for declared types; see above for instantiated types
 
-       // flags indicating temporary violations of the invariants for fromRHS and underlying
-       allowNilRHS        bool // same as below, as well as briefly during checking of a type declaration
-       allowNilUnderlying bool // may be true from creation via [NewNamed] until [Named.SetUnderlying]
+       allowNilRHS bool // may be true from creation via [NewNamed] until [Named.SetUnderlying]
 
        inst *instance // information for instantiated types; nil otherwise
 
@@ -192,7 +190,6 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named {
        n := (*Checker)(nil).newNamed(obj, underlying, methods)
        if underlying == nil {
                n.allowNilRHS = true
-               n.allowNilUnderlying = true
        } else {
                n.SetUnderlying(underlying)
        }
@@ -532,7 +529,6 @@ func (t *Named) SetUnderlying(u Type) {
        t.setState(lazyLoaded | unpacked | hasMethods) // TODO(markfreeman): Why hasMethods?
 
        t.underlying = u
-       t.allowNilUnderlying = false
        t.setState(hasUnder)
 }
 
@@ -594,9 +590,7 @@ func (n *Named) Underlying() Type {
        // and complicating things there, we just check for that special case here.
        if n.rhs() == nil {
                assert(n.allowNilRHS)
-               if n.allowNilUnderlying {
-                       return nil
-               }
+               return nil
        }
 
        if !n.stateHas(hasUnder) { // minor performance optimization
@@ -637,9 +631,6 @@ func (n *Named) resolveUnderlying() {
        var u Type
        for rhs := Type(n); u == nil; {
                switch t := rhs.(type) {
-               case nil:
-                       u = Typ[Invalid]
-
                case *Alias:
                        rhs = unalias(t)
 
@@ -661,8 +652,8 @@ func (n *Named) resolveUnderlying() {
                        path = append(path, t)
 
                        t.unpack()
-                       assert(t.rhs() != nil || t.allowNilRHS)
                        rhs = t.rhs()
+                       assert(rhs != nil)
 
                default:
                        u = rhs // any type literal or predeclared type works
index 90a3b4a901f8e71ecb23e41fb7871ec7ca3537a8..bf0b8a4b4bc73dc662936508f654f7bb5c617c06 100644 (file)
@@ -755,7 +755,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                        return
                }
 
-               if hasVarSize(x.typ, nil) {
+               if check.hasVarSize(x.typ) {
                        x.mode = value
                        if check.recordTypes() {
                                check.recordBuiltinType(call.Fun, makeSig(Typ[Uintptr], x.typ))
@@ -819,7 +819,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                // the part of the struct which is variable-sized. This makes both the rules
                // simpler and also permits (or at least doesn't prevent) a compiler from re-
                // arranging struct fields if it wanted to.
-               if hasVarSize(base, nil) {
+               if check.hasVarSize(base) {
                        x.mode = value
                        if check.recordTypes() {
                                check.recordBuiltinType(call.Fun, makeSig(Typ[Uintptr], obj.Type()))
@@ -843,7 +843,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                        return
                }
 
-               if hasVarSize(x.typ, nil) {
+               if check.hasVarSize(x.typ) {
                        x.mode = value
                        if check.recordTypes() {
                                check.recordBuiltinType(call.Fun, makeSig(Typ[Uintptr], x.typ))
@@ -1010,37 +1010,56 @@ func sliceElem(x *operand) (Type, *typeError) {
 // hasVarSize reports if the size of type t is variable due to type parameters
 // or if the type is infinitely-sized due to a cycle for which the type has not
 // yet been checked.
-func hasVarSize(t Type, seen map[*Named]bool) (varSized bool) {
-       // Cycles are only possible through *Named types.
-       // The seen map is used to detect cycles and track
-       // the results of previously seen types.
-       if named := asNamed(t); named != nil {
-               if v, ok := seen[named]; ok {
-                       return v
-               }
-               if seen == nil {
-                       seen = make(map[*Named]bool)
-               }
-               seen[named] = true // possibly cyclic until proven otherwise
-               defer func() {
-                       seen[named] = varSized // record final determination for named
-               }()
-       }
+func (check *Checker) hasVarSize(t Type) bool {
+       // Note: We could use Underlying here, but passing through the RHS may yield
+       // better error messages.
+       switch t := Unalias(t).(type) {
+       case *Named:
+               if t.stateHas(hasFinite) {
+                       // TODO(mark): Rename t.finite to t.varSize to avoid inversion.
+                       return !t.finite
+               }
+
+               if i, ok := check.objPathIdx[t.obj]; ok {
+                       cycle := check.objPath[i:]
+                       check.cycleError(cycle, firstInSrc(cycle))
+                       return true
+               }
+
+               check.push(t.obj)
+               defer check.pop()
+
+               varSize := check.hasVarSize(t.fromRHS)
+
+               t.mu.Lock()
+               defer t.mu.Unlock()
+
+               // Careful, t.finite has lock-free readers. Since we might be racing
+               // another call to finiteSize, we have to avoid overwriting t.finite.
+               // Otherwise, the race detector will be tripped.
+               if !t.stateHas(hasFinite) {
+                       t.finite = !varSize
+                       t.setState(hasFinite)
+               }
+
+               return varSize
 
-       switch u := t.Underlying().(type) {
        case *Array:
-               return hasVarSize(u.elem, seen)
+               // The array length is already computed. If it was a valid length, it
+               // is finite; else, an error was reported in the computation.
+               return check.hasVarSize(t.elem)
+
        case *Struct:
-               for _, f := range u.fields {
-                       if hasVarSize(f.typ, seen) {
+               for _, f := range t.fields {
+                       if check.hasVarSize(f.typ) {
                                return true
                        }
                }
-       case *Interface:
-               return isTypeParam(t)
-       case *Named, *Union:
-               panic("unreachable")
+
+       case *TypeParam:
+               return true
        }
+
        return false
 }
 
index 1b62569d8f0362ccea6db540684d3d23e9d497fb..978a7ca3cc5c0bee66a2a5fc53ed686ebf832b2d 100644 (file)
@@ -201,6 +201,11 @@ func (check *Checker) callExpr(x *operand, call *ast.CallExpr) exprKind {
                }
                T := x.typ
                x.mode = invalid
+               // We cannot convert a value to an incomplete type; make sure it's complete.
+               if !check.isComplete(T) {
+                       x.expr = call
+                       return conversion
+               }
                switch n := len(call.Args); n {
                case 0:
                        check.errorf(inNode(call, call.Rparen), WrongArgCount, "missing argument in conversion to %s", T)
@@ -321,7 +326,14 @@ func (check *Checker) callExpr(x *operand, call *ast.CallExpr) exprKind {
                } else {
                        x.mode = value
                }
-               x.typ = sig.results.vars[0].typ // unpack tuple
+               typ := sig.results.vars[0].typ // unpack tuple
+               // We cannot return a value of an incomplete type; make sure it's complete.
+               if !check.isComplete(typ) {
+                       x.mode = invalid
+                       x.expr = call
+                       return statement
+               }
+               x.typ = typ
        default:
                x.mode = value
                x.typ = sig.results
@@ -787,8 +799,12 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr, wantType bool) {
                goto Error
        }
 
-       // Avoid crashing when checking an invalid selector in a method declaration
-       // (i.e., where def is not set):
+       // We cannot select on an incomplete type; make sure it's complete.
+       if !check.isComplete(x.typ) {
+               goto Error
+       }
+
+       // Avoid crashing when checking an invalid selector in a method declaration.
        //
        //   type S[T any] struct{}
        //   type V = S[any]
@@ -798,14 +814,17 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr, wantType bool) {
        // expecting a type expression, it is an error.
        //
        // See go.dev/issue/57522 for more details.
-       //
-       // TODO(rfindley): We should do better by refusing to check selectors in all cases where
-       // x.typ is incomplete.
        if wantType {
                check.errorf(e.Sel, NotAType, "%s is not a type", ast.Expr(e))
                goto Error
        }
 
+       // Additionally, if x.typ is a pointer type, selecting implicitly dereferences the value, meaning
+       // its base type must also be complete.
+       if p, ok := x.typ.Underlying().(*Pointer); ok && !check.isComplete(p.base) {
+               goto Error
+       }
+
        obj, index, indirect = lookupFieldOrMethod(x.typ, x.mode == variable, check.pkg, sel, false)
        if obj == nil {
                // Don't report another error if the underlying type was invalid (go.dev/issue/49541).
index 9f1e0e8b58879a946e63709c40161bd91d46323f..957370cd9fcf6ff8ccb9fbc9d3ff46f7104e39bb 100644 (file)
@@ -106,49 +106,25 @@ func (check *Checker) directCycle(tname *TypeName, pathIdx map[*TypeName]int) {
        }
 }
 
-// TODO(markfreeman): Can the value cached on Named be used in validType / hasVarSize?
-
-// finiteSize returns whether a type has finite size.
-func (check *Checker) finiteSize(t Type) bool {
-       switch t := Unalias(t).(type) {
-       case *Named:
-               if t.stateHas(hasFinite) {
-                       return t.finite
-               }
-
-               if i, ok := check.objPathIdx[t.obj]; ok {
+// isComplete returns whether a type is complete (i.e. up to having an underlying type).
+// Incomplete types will panic if [Type.Underlying] is called on them.
+func (check *Checker) isComplete(t Type) bool {
+       if n, ok := Unalias(t).(*Named); ok {
+               if i, found := check.objPathIdx[n.obj]; found {
                        cycle := check.objPath[i:]
                        check.cycleError(cycle, firstInSrc(cycle))
                        return false
                }
-               check.push(t.obj)
-               defer check.pop()
-
-               isFinite := check.finiteSize(t.fromRHS)
-
-               t.mu.Lock()
-               defer t.mu.Unlock()
-               // Careful, t.finite has lock-free readers. Since we might be racing
-               // another call to finiteSize, we have to avoid overwriting t.finite.
-               // Otherwise, the race detector will be tripped.
-               if !t.stateHas(hasFinite) {
-                       t.finite = isFinite
-                       t.setState(hasFinite)
-               }
-
-               return isFinite
 
-       case *Array:
-               // The array length is already computed. If it was a valid length, it
-               // is finite; else, an error was reported in the computation.
-               return check.finiteSize(t.elem)
-
-       case *Struct:
-               for _, f := range t.fields {
-                       if !check.finiteSize(f.typ) {
-                               return false
-                       }
-               }
+               // We must walk through names because we permit certain cycles of names.
+               // Consider:
+               //
+               //   type A B
+               //   type B [unsafe.Sizeof(A{})]int
+               //
+               // starting at B. At the site of A{}, A has no underlying type, and so a
+               // cycle must be reported.
+               return check.isComplete(n.fromRHS)
        }
 
        return true
index 576424f608fe87a187ca3b2ab4d450efc923495d..486ef6f1e051ba217ffb3a074977628d4d4090da 100644 (file)
@@ -559,19 +559,6 @@ func (check *Checker) typeDecl(obj *TypeName, tdecl *ast.TypeSpec) {
        }
 
        named := check.newNamed(obj, nil, nil)
-
-       // The RHS of a named N can be nil if, for example, N is defined as a cycle of aliases with
-       // gotypesalias=0. Consider:
-       //
-       //   type D N    // N.unpack() will panic
-       //   type N A
-       //   type A = N  // N.fromRHS is not set before N.unpack(), since A does not call setDefType
-       //
-       // There is likely a better way to detect such cases, but it may not be worth the effort.
-       // Instead, we briefly permit a nil N.fromRHS while type-checking D.
-       named.allowNilRHS = true
-       defer (func() { named.allowNilRHS = false })()
-
        if tdecl.TypeParams != nil {
                check.openScope(tdecl, "type parameters")
                defer check.closeScope()
index 6ff5695390d1c549922c55b04ac64b235749d85d..f096b2a47e8862e40c196ba17deb61473fd06b07 100644 (file)
@@ -147,7 +147,8 @@ func (check *Checker) unary(x *operand, e *ast.UnaryExpr) {
                return
 
        case token.ARROW:
-               if elem := check.chanElem(x, x, true); elem != nil {
+               // We cannot receive a value with an incomplete type; make sure it's complete.
+               if elem := check.chanElem(x, x, true); elem != nil && check.isComplete(elem) {
                        x.mode = commaok
                        x.typ = elem
                        check.hasCallOrRecv = true
@@ -985,13 +986,6 @@ func (check *Checker) rawExpr(T *target, x *operand, e ast.Expr, hint Type, allo
                check.nonGeneric(T, x)
        }
 
-       // Here, x is a value, meaning it has a type. If that type is pending, then we have
-       // a cycle. As an example:
-       //
-       //  type T [unsafe.Sizeof(T{})]int
-       //
-       // has a cycle T->T which is deemed valid (by decl.go), but which is in fact invalid.
-       check.pendingType(x)
        check.record(x)
 
        return kind
@@ -1026,19 +1020,6 @@ func (check *Checker) nonGeneric(T *target, x *operand) {
        }
 }
 
-// If x has a pending type (i.e. its declaring object is on the object path), pendingType
-// reports an error and invalidates x.mode and x.typ.
-// Otherwise it leaves x alone.
-func (check *Checker) pendingType(x *operand) {
-       if x.mode == invalid || x.mode == novalue {
-               return
-       }
-       if !check.finiteSize(x.typ) {
-               x.mode = invalid
-               x.typ = Typ[Invalid]
-       }
-}
-
 // exprInternal contains the core of type checking of expressions.
 // Must only be called by rawExpr.
 // (See rawExpr for an explanation of the parameters.)
@@ -1129,6 +1110,10 @@ func (check *Checker) exprInternal(T *target, x *operand, e ast.Expr, hint Type)
                if !isValid(T) {
                        goto Error
                }
+               // We cannot assert to an incomplete type; make sure it's complete.
+               if !check.isComplete(T) {
+                       goto Error
+               }
                check.typeAssertion(e, x, T, false)
                x.mode = commaok
                x.typ = T
@@ -1161,6 +1146,10 @@ func (check *Checker) exprInternal(T *target, x *operand, e ast.Expr, hint Type)
                        }) {
                                goto Error
                        }
+                       // We cannot dereference a pointer with an incomplete base type; make sure it's complete.
+                       if !check.isComplete(base) {
+                               goto Error
+                       }
                        x.mode = variable
                        x.typ = base
                }
index 42e472001318cbc5097bad170c6fc2c382932407..720cbbf0ea2c6c549d41323f877f34eb268743f6 100644 (file)
@@ -48,6 +48,18 @@ func (check *Checker) indexExpr(x *operand, e *indexedExpr) (isFuncInst bool) {
                return false
        }
 
+       // We cannot index on an incomplete type; make sure it's complete.
+       if !check.isComplete(x.typ) {
+               x.mode = invalid
+               return false
+       }
+       // Additionally, if x.typ is a pointer to an array type, indexing implicitly dereferences the value, meaning
+       // its base type must also be complete.
+       if p, ok := x.typ.Underlying().(*Pointer); ok && !check.isComplete(p.base) {
+               x.mode = invalid
+               return false
+       }
+
        // ordinary index expression
        valid := false
        length := int64(-1) // valid if >= 0
@@ -256,6 +268,14 @@ func (check *Checker) sliceExpr(x *operand, e *ast.SliceExpr) {
                }
        }
 
+       // Note that we don't permit slice expressions where x is a type expression, so we don't check for that here.
+       // However, if x.typ is a pointer to an array type, slicing implicitly dereferences the value, meaning
+       // its base type must also be complete.
+       if p, ok := x.typ.Underlying().(*Pointer); ok && !check.isComplete(p.base) {
+               x.mode = invalid
+               return
+       }
+
        valid := false
        length := int64(-1) // valid if >= 0
        switch u := cu.(type) {
index 7ca351e60b1087df69f0209408a69b36659e492f..3bbb5d5cbd553fa5fd143aa7bddda23791ce4f44 100644 (file)
@@ -147,6 +147,12 @@ func (check *Checker) compositeLit(x *operand, e *ast.CompositeLit, hint Type) {
                base = typ
        }
 
+       // We cannot create a literal of an incomplete type; make sure it's complete.
+       if !check.isComplete(base) {
+               x.mode = invalid
+               return
+       }
+
        switch u, _ := commonUnder(base, nil); utyp := u.(type) {
        case *Struct:
                if len(e.Elts) == 0 {
index 3b618f403dc629d11e23e36ff08c1388807e418d..42f33e6af494361d211707748d6f4a58b112e47b 100644 (file)
@@ -109,9 +109,7 @@ type Named struct {
        check *Checker  // non-nil during type-checking; nil otherwise
        obj   *TypeName // corresponding declared object for declared types; see above for instantiated types
 
-       // flags indicating temporary violations of the invariants for fromRHS and underlying
-       allowNilRHS        bool // same as below, as well as briefly during checking of a type declaration
-       allowNilUnderlying bool // may be true from creation via [NewNamed] until [Named.SetUnderlying]
+       allowNilRHS bool // may be true from creation via [NewNamed] until [Named.SetUnderlying]
 
        inst *instance // information for instantiated types; nil otherwise
 
@@ -195,7 +193,6 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named {
        n := (*Checker)(nil).newNamed(obj, underlying, methods)
        if underlying == nil {
                n.allowNilRHS = true
-               n.allowNilUnderlying = true
        } else {
                n.SetUnderlying(underlying)
        }
@@ -535,7 +532,6 @@ func (t *Named) SetUnderlying(u Type) {
        t.setState(lazyLoaded | unpacked | hasMethods) // TODO(markfreeman): Why hasMethods?
 
        t.underlying = u
-       t.allowNilUnderlying = false
        t.setState(hasUnder)
 }
 
@@ -597,9 +593,7 @@ func (n *Named) Underlying() Type {
        // and complicating things there, we just check for that special case here.
        if n.rhs() == nil {
                assert(n.allowNilRHS)
-               if n.allowNilUnderlying {
-                       return nil
-               }
+               return nil
        }
 
        if !n.stateHas(hasUnder) { // minor performance optimization
@@ -640,9 +634,6 @@ func (n *Named) resolveUnderlying() {
        var u Type
        for rhs := Type(n); u == nil; {
                switch t := rhs.(type) {
-               case nil:
-                       u = Typ[Invalid]
-
                case *Alias:
                        rhs = unalias(t)
 
@@ -664,8 +655,8 @@ func (n *Named) resolveUnderlying() {
                        path = append(path, t)
 
                        t.unpack()
-                       assert(t.rhs() != nil || t.allowNilRHS)
                        rhs = t.rhs()
+                       assert(rhs != nil)
 
                default:
                        u = rhs // any type literal or predeclared type works
index e60c1767e98aefc6a8831913eac08d0dc60f9332..4c7adff5c508ce68c056e394546ef3db96749e39 100644 (file)
@@ -18,4 +18,4 @@ func _[P any]() {
        _ = unsafe.Sizeof(struct{ T[P] }{})
 }
 
-const _ = unsafe.Sizeof(T /* ERROR "invalid recursive type" */ [int]{})
+const _ = unsafe /* ERROR "not constant" */ .Sizeof(T /* ERROR "invalid recursive type" */ [int]{})
index 373db4a21bb16f2a0ee1f441985246deee1dfde1..713bb2e2079af101d9ccecd54cc6d5762f6c57b6 100644 (file)
@@ -6,7 +6,7 @@ package p
 
 import "unsafe"
 
-type A /* ERROR "invalid recursive type" */ [unsafe.Sizeof(S{})]byte
+type A /* ERROR "invalid recursive type" */ [unsafe/* ERROR "must be constant" */.Sizeof(S{})]byte
 
 type S struct {
        a A
index f16b40e04ea1c4775c91a0c55971e8c9860e422c..76f71dba745460c41c06c63b7a966c009831222e 100644 (file)
@@ -15,7 +15,7 @@ func f() D {
 }
 type D C
 
-type E /* ERROR "invalid recursive type" */ [unsafe.Sizeof(g[F]())]int
+type E /* ERROR "invalid recursive type" */ [unsafe/* ERROR "must be constant" */.Sizeof(g[F]())]int
 func g[P any]() P {
        panic(0)
 }