From 3531ac23d4aac6bdd914f14f65ee5fdc5e6e98fa Mon Sep 17 00:00:00 2001 From: Mark Freeman Date: Mon, 24 Nov 2025 17:04:49 -0500 Subject: [PATCH] go/types, types2: replace setDefType with pending type check Given a type definition of the form: type T RHS The setDefType function would set T.fromRHS as soon as we knew its top-level type. For instance, in: type S struct { ... } S.fromRHS is set to a struct type before type-checking anything inside the struct. This permit access to the (incomplete) RHS type in a cyclic type declaration. Accessing this information is fraught (as it's incomplete), but was used for reporting certain types of cycles. This CL replaces setDefType with a check that ensures no value of type T is used before its RHS is set up. This CL is strictly more complete than what setDefType achieved. For instance, it enables correct reporting for the below cycles: type A [unsafe.Sizeof(A{})]int var v any = 42 type B [v.(B)]int func f() C { return C{} } type C [unsafe.Sizeof(f())]int Fixes #76383 Fixes #76384 Change-Id: I9dfab5b708013b418fa66e43362bb4d8483fedec Reviewed-on: https://go-review.googlesource.com/c/go/+/724140 Auto-Submit: Mark Freeman Reviewed-by: Robert Griesemer LUCI-TryBot-Result: Go LUCI --- src/cmd/compile/internal/types2/expr.go | 23 +++++++++++++++++++ src/cmd/compile/internal/types2/typexpr.go | 16 ++----------- src/go/types/expr.go | 23 +++++++++++++++++++ src/go/types/typexpr.go | 16 ++----------- src/internal/types/errors/codes.go | 11 +++++---- src/internal/types/testdata/check/cycles0.go | 10 ++++---- src/internal/types/testdata/check/cycles2.go | 4 ++-- src/internal/types/testdata/check/cycles4.go | 4 ++-- src/internal/types/testdata/check/issues0.go | 4 ++-- .../types/testdata/fixedbugs/issue39634.go | 4 ++-- .../types/testdata/fixedbugs/issue49276.go | 16 ++++++------- .../types/testdata/fixedbugs/issue76383.go | 13 +++++++++++ .../types/testdata/fixedbugs/issue76384.go | 13 +++++++++++ test/fixedbugs/issue18392.go | 7 ++---- 14 files changed, 105 insertions(+), 59 deletions(-) create mode 100644 src/internal/types/testdata/fixedbugs/issue76383.go create mode 100644 src/internal/types/testdata/fixedbugs/issue76384.go diff --git a/src/cmd/compile/internal/types2/expr.go b/src/cmd/compile/internal/types2/expr.go index 39bf4055a3..9d7580cb01 100644 --- a/src/cmd/compile/internal/types2/expr.go +++ b/src/cmd/compile/internal/types2/expr.go @@ -993,6 +993,13 @@ 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 @@ -1027,6 +1034,22 @@ 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 n, ok := Unalias(x.typ).(*Named); ok { + if i, ok := check.objPathIdx[n.obj]; ok { + check.cycleError(check.objPath, i) + 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.) diff --git a/src/cmd/compile/internal/types2/typexpr.go b/src/cmd/compile/internal/types2/typexpr.go index 303f782ac4..c3e40184f5 100644 --- a/src/cmd/compile/internal/types2/typexpr.go +++ b/src/cmd/compile/internal/types2/typexpr.go @@ -417,20 +417,8 @@ func (check *Checker) typInternal(e0 syntax.Expr, def *TypeName) (T Type) { return typ } -func setDefType(def *TypeName, typ Type) { - if def != nil { - switch t := def.typ.(type) { - case *Alias: - t.fromRHS = typ - case *Basic: - assert(t == Typ[Invalid]) - case *Named: - t.fromRHS = typ - default: - panic(fmt.Sprintf("unexpected type %T", t)) - } - } -} +// TODO(markfreeman): Remove this function. +func setDefType(def *TypeName, typ Type) {} func (check *Checker) instantiatedType(x syntax.Expr, xlist []syntax.Expr, def *TypeName) (res Type) { if check.conf.Trace { diff --git a/src/go/types/expr.go b/src/go/types/expr.go index 8b3f764f19..790e0111c3 100644 --- a/src/go/types/expr.go +++ b/src/go/types/expr.go @@ -985,6 +985,13 @@ 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 @@ -1019,6 +1026,22 @@ 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 n, ok := Unalias(x.typ).(*Named); ok { + if i, ok := check.objPathIdx[n.obj]; ok { + check.cycleError(check.objPath, i) + 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.) diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go index b44fe4d768..c1381ddf4a 100644 --- a/src/go/types/typexpr.go +++ b/src/go/types/typexpr.go @@ -413,20 +413,8 @@ func (check *Checker) typInternal(e0 ast.Expr, def *TypeName) (T Type) { return typ } -func setDefType(def *TypeName, typ Type) { - if def != nil { - switch t := def.typ.(type) { - case *Alias: - t.fromRHS = typ - case *Basic: - assert(t == Typ[Invalid]) - case *Named: - t.fromRHS = typ - default: - panic(fmt.Sprintf("unexpected type %T", t)) - } - } -} +// TODO(markfreeman): Remove this function. +func setDefType(def *TypeName, typ Type) {} func (check *Checker) instantiatedType(ix *indexedExpr, def *TypeName) (res Type) { if check.conf._Trace { diff --git a/src/internal/types/errors/codes.go b/src/internal/types/errors/codes.go index b0f7d2d446..5b68cc3af7 100644 --- a/src/internal/types/errors/codes.go +++ b/src/internal/types/errors/codes.go @@ -114,15 +114,16 @@ const ( // S // } // - InvalidDeclCycle - - // InvalidTypeCycle occurs when a cycle in type definitions results in a - // type that is not well-defined. - // // Example: // import "unsafe" // // type T [unsafe.Sizeof(T{})]int + InvalidDeclCycle + + // TODO(markfreeman): Retire InvalidTypeCycle, as it's never emitted. + + // InvalidTypeCycle occurs when a cycle in type definitions results in a + // type that is not well-defined. InvalidTypeCycle // InvalidConstInit occurs when a const declaration has a non-constant diff --git a/src/internal/types/testdata/check/cycles0.go b/src/internal/types/testdata/check/cycles0.go index 8ad7877f94..d13dc447dd 100644 --- a/src/internal/types/testdata/check/cycles0.go +++ b/src/internal/types/testdata/check/cycles0.go @@ -45,7 +45,7 @@ type ( // pointers P0 *P0 - PP *struct{ PP.f /* ERROR "PP.f is not a type" */ } + PP /* ERROR "invalid recursive type" */ *struct{ PP.f } // functions F0 func(F0) @@ -157,10 +157,10 @@ type ( // test cases for issue 18643 // (type cycle detection when non-type expressions are involved) type ( - T14 [len(T14 /* ERROR "invalid recursive type" */ {})]int - T15 [][len(T15 /* ERROR "invalid recursive type" */ {})]int - T16 map[[len(T16 /* ERROR "invalid recursive type" */ {1:2})]int]int - T17 map[int][len(T17 /* ERROR "invalid recursive type" */ {1:2})]int + T14 /* ERROR "invalid recursive type" */ [len(T14{})]int + T15 /* ERROR "invalid recursive type" */ [][len(T15{})]int + T16 /* ERROR "invalid recursive type" */ map[[len(T16{1:2})]int]int + T17 /* ERROR "invalid recursive type" */ map[int][len(T17{1:2})]int ) // Test case for types depending on function literals (see also #22992). diff --git a/src/internal/types/testdata/check/cycles2.go b/src/internal/types/testdata/check/cycles2.go index a932d288b5..75016dbe8b 100644 --- a/src/internal/types/testdata/check/cycles2.go +++ b/src/internal/types/testdata/check/cycles2.go @@ -64,8 +64,8 @@ var _ = x == y // Test case for issue 6638. -type T interface { - m() [T(nil).m /* ERROR "undefined" */ ()[0]]int +type T /* ERROR "invalid recursive type" */ interface { + m() [T(nil).m()[0]]int } // Variations of this test case. diff --git a/src/internal/types/testdata/check/cycles4.go b/src/internal/types/testdata/check/cycles4.go index e82300125c..86f4f9aa03 100644 --- a/src/internal/types/testdata/check/cycles4.go +++ b/src/internal/types/testdata/check/cycles4.go @@ -114,8 +114,8 @@ type Event interface { // Check that accessing an interface method too early doesn't lead // to follow-on errors due to an incorrectly computed type set. -type T8 interface { - m() [unsafe.Sizeof(T8.m /* ERROR "undefined" */ )]int +type T8 /* ERROR "invalid recursive type" */ interface { + m() [unsafe.Sizeof(T8.m)]int } var _ = T8.m // no error expected here diff --git a/src/internal/types/testdata/check/issues0.go b/src/internal/types/testdata/check/issues0.go index 2b59a9c9b5..6117f7a8b9 100644 --- a/src/internal/types/testdata/check/issues0.go +++ b/src/internal/types/testdata/check/issues0.go @@ -96,8 +96,8 @@ func issue10979() { type _ interface { nosuchpkg /* ERROR "undefined: nosuchpkg" */ .Nosuchtype } - type I interface { - I.m /* ERROR "I.m is not a type" */ + type I /* ERROR "invalid recursive type" */ interface { + I.m m() } } diff --git a/src/internal/types/testdata/fixedbugs/issue39634.go b/src/internal/types/testdata/fixedbugs/issue39634.go index 6fbc7cd7bc..58fc43eea6 100644 --- a/src/internal/types/testdata/fixedbugs/issue39634.go +++ b/src/internal/types/testdata/fixedbugs/issue39634.go @@ -23,7 +23,7 @@ func(*ph1[e,e /* ERROR "redeclared" */ ])h(d /* ERROR "undefined" */ ) // func t2[T Numeric2](s[]T){0 /* ERROR "not a type */ []{s /* ERROR cannot index" */ [0][0]}} // crash 3 -type t3 *interface{ t3.p /* ERROR "t3.p is not a type" */ } +type t3 /* ERROR "invalid recursive type" */ *interface{ t3.p } // crash 4 type Numeric4 interface{t4 /* ERROR "not a type" */ } @@ -66,7 +66,7 @@ func F17[T Z17](T) {} type o18[T any] []func(_ o18[[]_ /* ERROR "cannot use _" */ ]) // crash 19 -type Z19 [][[]Z19{}[0][0]]c19 /* ERROR "undefined" */ +type Z19 /* ERROR "invalid recursive type: Z19 refers to itself" */ [][[]Z19{}[0][0]]int // crash 20 type Z20 /* ERROR "invalid recursive type" */ interface{ Z20 } diff --git a/src/internal/types/testdata/fixedbugs/issue49276.go b/src/internal/types/testdata/fixedbugs/issue49276.go index bdfb42f407..00da1a72cf 100644 --- a/src/internal/types/testdata/fixedbugs/issue49276.go +++ b/src/internal/types/testdata/fixedbugs/issue49276.go @@ -14,33 +14,33 @@ var s S // Since f is a pointer, this case could be valid. // But it's pathological and not worth the expense. -type T struct { - f *[unsafe.Sizeof(T /* ERROR "invalid recursive type" */ {})]int +type T /* ERROR "invalid recursive type" */ struct { + f *[unsafe.Sizeof(T{})]int } // a mutually recursive case using unsafe.Sizeof type ( - A1 struct { + A1/* ERROR "invalid recursive type" */ struct { _ [unsafe.Sizeof(B1{})]int } B1 struct { - _ [unsafe.Sizeof(A1 /* ERROR "invalid recursive type" */ {})]int + _ [unsafe.Sizeof(A1{})]int } ) // a mutually recursive case using len type ( - A2 struct { + A2/* ERROR "invalid recursive type" */ struct { f [len(B2{}.f)]int } B2 struct { - f [len(A2 /* ERROR "invalid recursive type" */ {}.f)]int + f [len(A2{}.f)]int } ) // test case from issue -type a struct { - _ [42 - unsafe.Sizeof(a /* ERROR "invalid recursive type" */ {})]byte +type a /* ERROR "invalid recursive type" */ struct { + _ [42 - unsafe.Sizeof(a{})]byte } diff --git a/src/internal/types/testdata/fixedbugs/issue76383.go b/src/internal/types/testdata/fixedbugs/issue76383.go new file mode 100644 index 0000000000..519174ab28 --- /dev/null +++ b/src/internal/types/testdata/fixedbugs/issue76383.go @@ -0,0 +1,13 @@ +// 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 p + +import "unsafe" + +var v any = 42 + +type T /* ERROR "invalid recursive type" */ struct { + f [unsafe.Sizeof(v.(T))]int +} diff --git a/src/internal/types/testdata/fixedbugs/issue76384.go b/src/internal/types/testdata/fixedbugs/issue76384.go new file mode 100644 index 0000000000..0737eb9e1a --- /dev/null +++ b/src/internal/types/testdata/fixedbugs/issue76384.go @@ -0,0 +1,13 @@ +// 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 p + +import "unsafe" + +type T /* ERROR "invalid recursive type" */ [unsafe.Sizeof(f())]int + +func f() T { + return T{} +} \ No newline at end of file diff --git a/test/fixedbugs/issue18392.go b/test/fixedbugs/issue18392.go index 32c39c3a7f..9e4d48d6ea 100644 --- a/test/fixedbugs/issue18392.go +++ b/test/fixedbugs/issue18392.go @@ -6,9 +6,6 @@ package p -type A interface { - // TODO(mdempsky): This should be an error, but this error is - // nonsense. The error should actually mention that there's a - // type loop. - Fn(A.Fn) // ERROR "type A has no method Fn|A.Fn undefined|A.Fn is not a type" +type A interface { // ERROR "invalid recursive type" + Fn(A.Fn) } -- 2.52.0