Skip to content

Commit ddd8558

Browse files
Mark Freemangopherbot
authored andcommitted
go/types, types2: swap object.color for Checker.objPathIdx
The type checker assigns types to objects. An object can be in 1 of 3 states, which are mapped to colors. This is stored as a field on the object. - white : type not known, awaiting processing - grey : type pending, in processing - black : type known, done processing With the addition of Checker.objPathIdx, which maps objects to a path index, presence in the map could signal grey coloring. White and black coloring can be signaled by presence of object.typ (a simple nil check). This change removes the object.color field and its associated methods, replacing it with an equivalent use of Checker.objPathIdx. Checker.objPathIdx is integrated into the existing push and pop methods, while slightly simplifying their signatures. Note that the concept of object coloring remains the same - we are merely changing and simplifying the mechanism which represents the colors. Change-Id: I91fb5e9a59dcb34c08ffc5b4ebc3f20a400094b6 Reviewed-on: https://go-review.googlesource.com/c/go/+/715840 Reviewed-by: Robert Griesemer <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Mark Freeman <[email protected]>
1 parent 9daaab3 commit ddd8558

File tree

14 files changed

+170
-348
lines changed

14 files changed

+170
-348
lines changed

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

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -171,12 +171,13 @@ type Checker struct {
171171
usedPkgNames map[*PkgName]bool // set of used package names
172172
mono monoGraph // graph for detecting non-monomorphizable instantiation loops
173173

174-
firstErr error // first error encountered
175-
methods map[*TypeName][]*Func // maps package scope type names to associated non-blank (non-interface) methods
176-
untyped map[syntax.Expr]exprInfo // map of expressions without final type
177-
delayed []action // stack of delayed action segments; segments are processed in FIFO order
178-
objPath []Object // path of object dependencies during type inference (for cycle reporting)
179-
cleaners []cleaner // list of types that may need a final cleanup at the end of type-checking
174+
firstErr error // first error encountered
175+
methods map[*TypeName][]*Func // maps package scope type names to associated non-blank (non-interface) methods
176+
untyped map[syntax.Expr]exprInfo // map of expressions without final type
177+
delayed []action // stack of delayed action segments; segments are processed in FIFO order
178+
objPath []Object // path of object dependencies during type-checking (for cycle reporting)
179+
objPathIdx map[Object]int // map of object to object path index during type-checking (for cycle reporting)
180+
cleaners []cleaner // list of types that may need a final cleanup at the end of type-checking
180181

181182
// environment within which the current object is type-checked (valid only
182183
// for the duration of type-checking a specific object)
@@ -248,19 +249,22 @@ func (check *Checker) later(f func()) *action {
248249
return &check.delayed[i]
249250
}
250251

251-
// push pushes obj onto the object path and returns its index in the path.
252-
func (check *Checker) push(obj Object) int {
252+
// push pushes obj onto the object path and records its index in the path index map.
253+
func (check *Checker) push(obj Object) {
254+
if check.objPathIdx == nil {
255+
check.objPathIdx = make(map[Object]int)
256+
}
257+
check.objPathIdx[obj] = len(check.objPath)
253258
check.objPath = append(check.objPath, obj)
254-
return len(check.objPath) - 1
255259
}
256260

257-
// pop pops and returns the topmost object from the object path.
258-
func (check *Checker) pop() Object {
261+
// pop pops an object from the object path and removes it from the path index map.
262+
func (check *Checker) pop() {
259263
i := len(check.objPath) - 1
260264
obj := check.objPath[i]
261-
check.objPath[i] = nil
265+
check.objPath[i] = nil // help the garbage collector
262266
check.objPath = check.objPath[:i]
263-
return obj
267+
delete(check.objPathIdx, obj)
264268
}
265269

266270
type cleaner interface {
@@ -319,6 +323,7 @@ func (check *Checker) initFiles(files []*syntax.File) {
319323
check.untyped = nil
320324
check.delayed = nil
321325
check.objPath = nil
326+
check.objPathIdx = nil
322327
check.cleaners = nil
323328

324329
// We must initialize usedVars and usedPkgNames both here and in NewChecker,

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ func (check *Checker) directCycle(tname *TypeName, pathIdx map[*TypeName]int) {
5454
// tname is marked grey - we have a cycle on the path beginning at start.
5555
// Mark tname as invalid.
5656
tname.setType(Typ[Invalid])
57-
tname.setColor(black)
5857

5958
// collect type names on cycle
6059
var cycle []Object

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

Lines changed: 50 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -62,114 +62,77 @@ func (check *Checker) objDecl(obj Object, def *TypeName) {
6262
if check.indent == 0 {
6363
fmt.Println() // empty line between top-level objects for readability
6464
}
65-
check.trace(obj.Pos(), "-- checking %s (%s, objPath = %s)", obj, obj.color(), pathString(check.objPath))
65+
check.trace(obj.Pos(), "-- checking %s (objPath = %s)", obj, pathString(check.objPath))
6666
check.indent++
6767
defer func() {
6868
check.indent--
69-
check.trace(obj.Pos(), "=> %s (%s)", obj, obj.color())
69+
check.trace(obj.Pos(), "=> %s", obj)
7070
}()
7171
}
7272

73-
// Checking the declaration of obj means inferring its type
74-
// (and possibly its value, for constants).
75-
// An object's type (and thus the object) may be in one of
76-
// three states which are expressed by colors:
73+
// Checking the declaration of an object means determining its type
74+
// (and also its value for constants). An object (and thus its type)
75+
// may be in 1 of 3 states:
7776
//
78-
// - an object whose type is not yet known is painted white (initial color)
79-
// - an object whose type is in the process of being inferred is painted grey
80-
// - an object whose type is fully inferred is painted black
77+
// - not in Checker.objPathIdx and type == nil : type is not yet known (white)
78+
// - in Checker.objPathIdx : type is pending (grey)
79+
// - not in Checker.objPathIdx and type != nil : type is known (black)
8180
//
82-
// During type inference, an object's color changes from white to grey
83-
// to black (pre-declared objects are painted black from the start).
84-
// A black object (i.e., its type) can only depend on (refer to) other black
85-
// ones. White and grey objects may depend on white and black objects.
86-
// A dependency on a grey object indicates a cycle which may or may not be
87-
// valid.
81+
// During type-checking, an object changes from white to grey to black.
82+
// Predeclared objects start as black (their type is known without checking).
8883
//
89-
// When objects turn grey, they are pushed on the object path (a stack);
90-
// they are popped again when they turn black. Thus, if a grey object (a
91-
// cycle) is encountered, it is on the object path, and all the objects
92-
// it depends on are the remaining objects on that path. Color encoding
93-
// is such that the color value of a grey object indicates the index of
94-
// that object in the object path.
95-
96-
// During type-checking, white objects may be assigned a type without
97-
// traversing through objDecl; e.g., when initializing constants and
98-
// variables. Update the colors of those objects here (rather than
99-
// everywhere where we set the type) to satisfy the color invariants.
100-
if obj.color() == white && obj.Type() != nil {
101-
obj.setColor(black)
102-
return
103-
}
104-
105-
switch obj.color() {
106-
case white:
107-
assert(obj.Type() == nil)
108-
// All color values other than white and black are considered grey.
109-
// Because black and white are < grey, all values >= grey are grey.
110-
// Use those values to encode the object's index into the object path.
111-
obj.setColor(grey + color(check.push(obj)))
112-
defer func() {
113-
check.pop().setColor(black)
114-
}()
115-
116-
case black:
117-
assert(obj.Type() != nil)
118-
return
119-
120-
default:
121-
// Color values other than white or black are considered grey.
122-
fallthrough
123-
124-
case grey:
125-
// We have a (possibly invalid) cycle.
126-
// In the existing code, this is marked by a non-nil type
127-
// for the object except for constants and variables whose
128-
// type may be non-nil (known), or nil if it depends on the
129-
// not-yet known initialization value.
130-
// In the former case, set the type to Typ[Invalid] because
131-
// we have an initialization cycle. The cycle error will be
132-
// reported later, when determining initialization order.
133-
// TODO(gri) Report cycle here and simplify initialization
134-
// order code.
84+
// A black object may only depend on (refer to) to other black objects. White
85+
// and grey objects may depend on white or black objects. A dependency on a
86+
// grey object indicates a (possibly invalid) cycle.
87+
//
88+
// When an object is marked grey, it is pushed onto the object path (a stack)
89+
// and its index in the path is recorded in the path index map. It is popped
90+
// and removed from the map when its type is determined (and marked black).
91+
92+
// If this object is grey, we have a (possibly invalid) cycle. This is signaled
93+
// by a non-nil type for the object, except for constants and variables whose
94+
// type may be non-nil (known), or nil if it depends on a not-yet known
95+
// initialization value.
96+
//
97+
// In the former case, set the type to Typ[Invalid] because we have an
98+
// initialization cycle. The cycle error will be reported later, when
99+
// determining initialization order.
100+
//
101+
// TODO(gri) Report cycle here and simplify initialization order code.
102+
if _, ok := check.objPathIdx[obj]; ok {
135103
switch obj := obj.(type) {
136-
case *Const:
137-
if !check.validCycle(obj) || obj.typ == nil {
138-
obj.typ = Typ[Invalid]
139-
}
140-
141-
case *Var:
142-
if !check.validCycle(obj) || obj.typ == nil {
143-
obj.typ = Typ[Invalid]
104+
case *Const, *Var:
105+
if !check.validCycle(obj) || obj.Type() == nil {
106+
obj.setType(Typ[Invalid])
144107
}
145-
146108
case *TypeName:
147109
if !check.validCycle(obj) {
148-
// break cycle
149-
// (without this, calling underlying()
150-
// below may lead to an endless loop
151-
// if we have a cycle for a defined
152-
// (*Named) type)
153-
obj.typ = Typ[Invalid]
110+
obj.setType(Typ[Invalid])
154111
}
155-
156112
case *Func:
157113
if !check.validCycle(obj) {
158-
// Don't set obj.typ to Typ[Invalid] here
159-
// because plenty of code type-asserts that
160-
// functions have a *Signature type. Grey
161-
// functions have their type set to an empty
162-
// signature which makes it impossible to
114+
// Don't set type to Typ[Invalid]; plenty of code asserts that
115+
// functions have a *Signature type. Instead, leave the type
116+
// as an empty signature, which makes it impossible to
163117
// initialize a variable with the function.
164118
}
165-
166119
default:
167120
panic("unreachable")
168121
}
122+
169123
assert(obj.Type() != nil)
170124
return
171125
}
172126

127+
if obj.Type() != nil { // black, meaning it's already type-checked
128+
return
129+
}
130+
131+
// white, meaning it must be type-checked
132+
133+
check.push(obj)
134+
defer check.pop()
135+
173136
d := check.objMap[obj]
174137
if d == nil {
175138
check.dump("%v: %s should have been declared", obj.Pos(), obj)
@@ -221,8 +184,8 @@ func (check *Checker) validCycle(obj Object) (valid bool) {
221184
}
222185

223186
// Count cycle objects.
224-
assert(obj.color() >= grey)
225-
start := obj.color() - grey // index of obj in objPath
187+
start, found := check.objPathIdx[obj]
188+
assert(found)
226189
cycle := check.objPath[start:]
227190
tparCycle := false // if set, the cycle is through a type parameter list
228191
nval := 0 // number of (constant or variable) values in the cycle
@@ -764,17 +727,8 @@ func (check *Checker) funcDecl(obj *Func, decl *declInfo) {
764727
sig := new(Signature)
765728
obj.typ = sig // guard against cycles
766729

767-
// Avoid cycle error when referring to method while type-checking the signature.
768-
// This avoids a nuisance in the best case (non-parameterized receiver type) and
769-
// since the method is not a type, we get an error. If we have a parameterized
770-
// receiver type, instantiating the receiver type leads to the instantiation of
771-
// its methods, and we don't want a cycle error in that case.
772-
// TODO(gri) review if this is correct and/or whether we still need this?
773-
saved := obj.color_
774-
obj.color_ = black
775730
fdecl := decl.fdecl
776731
check.funcType(sig, fdecl.Recv, fdecl.TParamList, fdecl.Type)
777-
obj.color_ = saved
778732

779733
// Set the scope's extent to the complete "func (...) { ... }"
780734
// so that Scope.Innermost works correctly.
@@ -921,10 +875,9 @@ func (check *Checker) declStmt(list []syntax.Decl) {
921875
// the innermost containing block."
922876
scopePos := s.Name.Pos()
923877
check.declare(check.scope, s.Name, obj, scopePos)
924-
// mark and unmark type before calling typeDecl; its type is still nil (see Checker.objDecl)
925-
obj.setColor(grey + color(check.push(obj)))
878+
check.push(obj) // mark as grey
879+
defer check.pop()
926880
check.typeDecl(obj, s, nil)
927-
check.pop().setColor(black)
928881

929882
default:
930883
check.errorf(s, InvalidSyntaxTree, "unknown syntax.Decl node %T", s)

0 commit comments

Comments
 (0)