Skip to content

Commit 57362e9

Browse files
Robert Griesemergopherbot
authored andcommitted
go/types, types2: check for direct cycles as a separate phase
A direct cycle is the most basic form of cycle, where no type literal or predeclared type is reached. It is formed by a series of only TypeNames. To illustrate, type T T is a direct cycle, but type T [1]T and type T *T are not. Likewise, the below is also a direct cycle: type A B type B C type C = A Direct cycles are handled explicitly as part of resolveUnderlying, since they are the only cycle which can prevent reaching an underlying type. If we move this check to an earlier compiler phase, we can simplify resolveUnderlying. This is the first of (hopefully) several cycle kinds to be moved into a preliminary phase, with the goal of simplifying the main type-checking pass. For that reason, the bulk of the logic is placed in cycles.go. CL based on an earlier version by Mark Freeman. Change-Id: I3044c383278deb6acb8767c498d8cb68099ba8ef Reviewed-on: https://go-review.googlesource.com/c/go/+/717343 Auto-Submit: Robert Griesemer <[email protected]> Reviewed-by: Mark Freeman <[email protected]> Reviewed-by: Robert Griesemer <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 099e002 commit 57362e9

File tree

6 files changed

+248
-38
lines changed

6 files changed

+248
-38
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,9 @@ func (check *Checker) checkFiles(files []*syntax.File) {
497497
print("== sortObjects ==")
498498
check.sortObjects()
499499

500+
print("== directCycles ==")
501+
check.directCycles()
502+
500503
print("== packageObjects ==")
501504
check.packageObjects()
502505

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package types2
6+
7+
import "cmd/compile/internal/syntax"
8+
9+
// directCycles searches for direct cycles among package level type declarations.
10+
// See directCycle for details.
11+
func (check *Checker) directCycles() {
12+
pathIdx := make(map[*TypeName]int)
13+
for _, obj := range check.objList {
14+
if tname, ok := obj.(*TypeName); ok {
15+
check.directCycle(tname, pathIdx)
16+
}
17+
}
18+
}
19+
20+
// directCycle checks if the declaration of the type given by tname contains a direct cycle.
21+
// A direct cycle exists if the path from tname's declaration's RHS leads from type name to
22+
// type name and eventually ends up on that path again, via regular or alias declarations;
23+
// in other words if there are no type literals (or basic types) on the path, and the path
24+
// doesn't end in an undeclared object.
25+
// If a cycle is detected, a cycle error is reported and the type at the start of the cycle
26+
// is marked as invalid.
27+
//
28+
// The pathIdx map tracks which type names have been processed. An entry can be
29+
// in 1 of 3 states as used in a typical 3-state (white/grey/black) graph marking
30+
// algorithm for cycle detection:
31+
//
32+
// - entry not found: tname has not been seen before (white)
33+
// - value is >= 0 : tname has been seen but is not done (grey); the value is the path index
34+
// - value is < 0 : tname has been seen and is done (black)
35+
//
36+
// When directCycle returns, the pathIdx entries for all type names on the path
37+
// that starts at tname are marked black, regardless of whether there was a cycle.
38+
// This ensures that a type name is traversed only once.
39+
func (check *Checker) directCycle(tname *TypeName, pathIdx map[*TypeName]int) {
40+
if debug && check.conf.Trace {
41+
check.trace(tname.Pos(), "-- check direct cycle for %s", tname)
42+
}
43+
44+
var path []*TypeName
45+
for {
46+
start, found := pathIdx[tname]
47+
if start < 0 {
48+
// tname is marked black - do not traverse it again.
49+
// (start can only be < 0 if it was found in the first place)
50+
break
51+
}
52+
53+
if found {
54+
// tname is marked grey - we have a cycle on the path beginning at start.
55+
// Mark tname as invalid.
56+
tname.setType(Typ[Invalid])
57+
tname.setColor(black)
58+
59+
// collect type names on cycle
60+
var cycle []Object
61+
for _, tname := range path[start:] {
62+
cycle = append(cycle, tname)
63+
}
64+
65+
check.cycleError(cycle, firstInSrc(cycle))
66+
break
67+
}
68+
69+
// tname is marked white - mark it grey and add it to the path.
70+
pathIdx[tname] = len(path)
71+
path = append(path, tname)
72+
73+
// For direct cycle detection, we don't care about whether we have an alias or not.
74+
// If the associated type is not a name, we're at the end of the path and we're done.
75+
rhs, ok := check.objMap[tname].tdecl.Type.(*syntax.Name)
76+
if !ok {
77+
break
78+
}
79+
80+
// Determine the RHS type. If it is not found in the package scope, we either
81+
// have an error (which will be reported later), or the type exists elsewhere
82+
// (universe scope, file scope via dot-import) and a cycle is not possible in
83+
// the first place. If it is not a type name, we cannot have a direct cycle
84+
// either. In all these cases we can stop.
85+
tname1, ok := check.pkg.scope.Lookup(rhs.Value).(*TypeName)
86+
if !ok {
87+
break
88+
}
89+
90+
// Otherwise, continue with the RHS.
91+
tname = tname1
92+
}
93+
94+
// Mark all traversed type names black.
95+
// (ensure that pathIdx doesn't contain any grey entries upon returning)
96+
for _, tname := range path {
97+
pathIdx[tname] = -1
98+
}
99+
100+
if debug {
101+
for _, i := range pathIdx {
102+
assert(i < 0)
103+
}
104+
}
105+
}

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

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -613,13 +613,18 @@ func (t *Named) String() string { return TypeString(t, nil) }
613613
// type set to T. Aliases are skipped because their underlying type is
614614
// not memoized.
615615
//
616-
// This method also checks for cycles among alias and named types, which will
617-
// yield no underlying type. If such a cycle is found, the underlying type is
618-
// set to Typ[Invalid] and a cycle is reported.
616+
// resolveUnderlying assumes that there are no direct cycles; if there were
617+
// any, they were broken (by setting the respective types to invalid) during
618+
// the directCycles check phase.
619619
func (n *Named) resolveUnderlying() {
620620
assert(n.stateHas(unpacked))
621621

622-
var seen map[*Named]int // allocated lazily
622+
var seen map[*Named]bool // for debugging only
623+
if debug {
624+
seen = make(map[*Named]bool)
625+
}
626+
627+
var path []*Named
623628
var u Type
624629
for rhs := Type(n); u == nil; {
625630
switch t := rhs.(type) {
@@ -630,17 +635,9 @@ func (n *Named) resolveUnderlying() {
630635
rhs = unalias(t)
631636

632637
case *Named:
633-
if i, ok := seen[t]; ok {
634-
// compute cycle path
635-
path := make([]Object, len(seen))
636-
for t, j := range seen {
637-
path[j] = t.obj
638-
}
639-
path = path[i:]
640-
// only called during type checking, hence n.check != nil
641-
n.check.cycleError(path, firstInSrc(path))
642-
u = Typ[Invalid]
643-
break
638+
if debug {
639+
assert(!seen[t])
640+
seen[t] = true
644641
}
645642

646643
// don't recalculate the underlying
@@ -649,10 +646,10 @@ func (n *Named) resolveUnderlying() {
649646
break
650647
}
651648

652-
if seen == nil {
653-
seen = make(map[*Named]int)
649+
if debug {
650+
seen[t] = true
654651
}
655-
seen[t] = len(seen)
652+
path = append(path, t)
656653

657654
t.unpack()
658655
assert(t.rhs() != nil || t.allowNilRHS)
@@ -663,7 +660,7 @@ func (n *Named) resolveUnderlying() {
663660
}
664661
}
665662

666-
for t := range seen {
663+
for _, t := range path {
667664
func() {
668665
t.mu.Lock()
669666
defer t.mu.Unlock()

src/go/types/check.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,9 @@ func (check *Checker) checkFiles(files []*ast.File) {
522522
print("== sortObjects ==")
523523
check.sortObjects()
524524

525+
print("== directCycles ==")
526+
check.directCycles()
527+
525528
print("== packageObjects ==")
526529
check.packageObjects()
527530

src/go/types/cycles.go

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package types
6+
7+
import "go/ast"
8+
9+
// directCycles searches for direct cycles among package level type declarations.
10+
// See directCycle for details.
11+
func (check *Checker) directCycles() {
12+
pathIdx := make(map[*TypeName]int)
13+
for _, obj := range check.objList {
14+
if tname, ok := obj.(*TypeName); ok {
15+
check.directCycle(tname, pathIdx)
16+
}
17+
}
18+
}
19+
20+
// directCycle checks if the declaration of the type given by tname contains a direct cycle.
21+
// A direct cycle exists if the path from tname's declaration's RHS leads from type name to
22+
// type name and eventually ends up on that path again, via regular or alias declarations;
23+
// in other words if there are no type literals (or basic types) on the path, and the path
24+
// doesn't end in an undeclared object.
25+
// If a cycle is detected, a cycle error is reported and the type at the start of the cycle
26+
// is marked as invalid.
27+
//
28+
// The pathIdx map tracks which type names have been processed. An entry can be
29+
// in 1 of 3 states as used in a typical 3-state (white/grey/black) graph marking
30+
// algorithm for cycle detection:
31+
//
32+
// - entry not found: tname has not been seen before (white)
33+
// - value is >= 0 : tname has been seen but is not done (grey); the value is the path index
34+
// - value is < 0 : tname has been seen and is done (black)
35+
//
36+
// When directCycle returns, the pathIdx entries for all type names on the path
37+
// that starts at tname are marked black, regardless of whether there was a cycle.
38+
// This ensures that a type name is traversed only once.
39+
func (check *Checker) directCycle(tname *TypeName, pathIdx map[*TypeName]int) {
40+
if debug && check.conf._Trace {
41+
check.trace(tname.Pos(), "-- check direct cycle for %s", tname)
42+
}
43+
44+
var path []*TypeName
45+
for {
46+
start, found := pathIdx[tname]
47+
if start < 0 {
48+
// tname is marked black - do not traverse it again.
49+
// (start can only be < 0 if it was found in the first place)
50+
break
51+
}
52+
53+
if found {
54+
// tname is marked grey - we have a cycle on the path beginning at start.
55+
// Mark tname as invalid.
56+
tname.setType(Typ[Invalid])
57+
tname.setColor(black)
58+
59+
// collect type names on cycle
60+
var cycle []Object
61+
for _, tname := range path[start:] {
62+
cycle = append(cycle, tname)
63+
}
64+
65+
check.cycleError(cycle, firstInSrc(cycle))
66+
break
67+
}
68+
69+
// tname is marked white - mark it grey and add it to the path.
70+
pathIdx[tname] = len(path)
71+
path = append(path, tname)
72+
73+
// For direct cycle detection, we don't care about whether we have an alias or not.
74+
// If the associated type is not a name, we're at the end of the path and we're done.
75+
rhs, ok := check.objMap[tname].tdecl.Type.(*ast.Ident)
76+
if !ok {
77+
break
78+
}
79+
80+
// Determine the RHS type. If it is not found in the package scope, we either
81+
// have an error (which will be reported later), or the type exists elsewhere
82+
// (universe scope, file scope via dot-import) and a cycle is not possible in
83+
// the first place. If it is not a type name, we cannot have a direct cycle
84+
// either. In all these cases we can stop.
85+
tname1, ok := check.pkg.scope.Lookup(rhs.Name).(*TypeName)
86+
if !ok {
87+
break
88+
}
89+
90+
// Otherwise, continue with the RHS.
91+
tname = tname1
92+
}
93+
94+
// Mark all traversed type names black.
95+
// (ensure that pathIdx doesn't contain any grey entries upon returning)
96+
for _, tname := range path {
97+
pathIdx[tname] = -1
98+
}
99+
100+
if debug {
101+
for _, i := range pathIdx {
102+
assert(i < 0)
103+
}
104+
}
105+
}

src/go/types/named.go

Lines changed: 16 additions & 19 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)