go/types: track local cycles using same mechanism as for global objects
For Go 1.11, cycle tracking of global (package-level) objects was changed
to use a Checker-level object path rather than relying on the explicit
path parameter that is passed around to some (but not all) type-checker
functions.
This change now uses the same mechanism for the detection of local
type cycles (local non-type objects cannot create cycles by definition
of the spec).
As a result, local alias cycles are now correctly detected as well
(issue #27106).
The path parameter that is explicitly passed around to some type-checker
methods is still present and will be removed in a follow-up CL.
Also:
- removed useCycleMarking flag and respective dead code
- added a couple more tests
- improved documentation
Fixes #27106.
Updates #25773.
Change-Id: I7cbf304bceb43a8d52e6483dcd0fa9ef7e1ea71c
Reviewed-on: https://go-review.googlesource.com/130455
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
diff --git a/src/go/types/check.go b/src/go/types/check.go
index 76d9c89..5b796be 100644
--- a/src/go/types/check.go
+++ b/src/go/types/check.go
@@ -76,7 +76,7 @@
fset *token.FileSet
pkg *Package
*Info
- objMap map[Object]*declInfo // maps package-level object to declaration info
+ objMap map[Object]*declInfo // maps package-level objects and (non-interface) methods to declaration info
impMap map[importKey]*Package // maps (import path, source directory) to (complete or fake) package
// information collected during type-checking of a set of package files
diff --git a/src/go/types/decl.go b/src/go/types/decl.go
index 11b6858..d845789 100644
--- a/src/go/types/decl.go
+++ b/src/go/types/decl.go
@@ -64,13 +64,6 @@
return s
}
-// useCycleMarking enables the new coloring-based cycle marking scheme
-// for package-level objects. Set this flag to false to disable this
-// code quickly and revert to the existing mechanism (and comment out
-// some of the new tests in cycles5.src that will fail again).
-// TODO(gri) remove this for Go 1.12
-const useCycleMarking = true
-
// objDecl type-checks the declaration of obj in its respective (file) context.
// See check.typ for the details on def and path.
func (check *Checker) objDecl(obj Object, def *Named, path []*TypeName) {
@@ -147,7 +140,7 @@
// order code.
switch obj := obj.(type) {
case *Const:
- if useCycleMarking && check.typeCycle(obj) {
+ if check.typeCycle(obj) {
obj.typ = Typ[Invalid]
break
}
@@ -156,7 +149,7 @@
}
case *Var:
- if useCycleMarking && check.typeCycle(obj) {
+ if check.typeCycle(obj) {
obj.typ = Typ[Invalid]
break
}
@@ -190,7 +183,7 @@
}
}
- if useCycleMarking && check.typeCycle(obj) {
+ if check.typeCycle(obj) {
// break cycle
// (without this, calling underlying()
// below may lead to an endless loop
@@ -200,7 +193,7 @@
}
case *Func:
- if useCycleMarking && check.typeCycle(obj) {
+ if check.typeCycle(obj) {
// Don't set obj.typ to Typ[Invalid] here
// because plenty of code type-asserts that
// functions have a *Signature type. Grey
@@ -273,10 +266,15 @@
// TODO(gri) rename s/typeCycle/cycle/ once we don't need the other
// cycle method anymore.
func (check *Checker) typeCycle(obj Object) (isCycle bool) {
- d := check.objMap[obj]
- if d == nil {
- check.dump("%v: %s should have been declared", obj.Pos(), obj)
- unreachable()
+ // The object map contains the package scope objects and the non-interface methods.
+ if debug {
+ info := check.objMap[obj]
+ inObjMap := info != nil && (info.fdecl == nil || info.fdecl.Recv == nil) // exclude methods
+ isPkgObj := obj.Parent() == check.pkg.scope
+ if isPkgObj != inObjMap {
+ check.dump("%v: inconsistent object map for %s (isPkgObj = %v, inObjMap = %v)", obj.Pos(), obj, isPkgObj, inObjMap)
+ unreachable()
+ }
}
// Given the number of constants and variables (nval) in the cycle
@@ -312,8 +310,25 @@
// that we type-check methods when we type-check their
// receiver base types.
return false
- case !check.objMap[obj].alias:
- hasTDef = true
+ default:
+ // Determine if the type name is an alias or not. For
+ // package-level objects, use the object map which
+ // provides syntactic information (which doesn't rely
+ // on the order in which the objects are set up). For
+ // local objects, we can rely on the order, so use
+ // the object's predicate.
+ // TODO(gri) It would be less fragile to always access
+ // the syntactic information. We should consider storing
+ // this information explicitly in the object.
+ var alias bool
+ if d := check.objMap[obj]; d != nil {
+ alias = d.alias // package-level object
+ } else {
+ alias = obj.IsAlias() // function local object
+ }
+ if !alias {
+ hasTDef = true
+ }
}
case *Func:
// ignored for now
@@ -552,15 +567,13 @@
}
}
- if useCycleMarking {
- // Suppress detection of type cycles occurring through method
- // declarations - they wouldn't exist if methods were type-
- // checked separately from their receiver base types. See also
- // comment at the end of Checker.typeDecl.
- // TODO(gri) Remove this once methods are type-checked separately.
- check.push(cutCycle)
- defer check.pop()
- }
+ // Suppress detection of type cycles occurring through method
+ // declarations - they wouldn't exist if methods were type-
+ // checked separately from their receiver base types. See also
+ // comment at the end of Checker.typeDecl.
+ // TODO(gri) Remove this once methods are type-checked separately.
+ check.push(cutCycle)
+ defer check.pop()
// type-check methods
for _, m := range methods {
@@ -730,8 +743,10 @@
// the innermost containing block."
scopePos := s.Name.Pos()
check.declare(check.scope, s.Name, obj, scopePos)
+ // mark and unmark type before calling typeDecl; its type is still nil (see Checker.objDecl)
+ obj.setColor(grey + color(check.push(obj)))
check.typeDecl(obj, s.Type, nil, nil, s.Assign.IsValid())
-
+ check.pop().setColor(black)
default:
check.invalidAST(s.Pos(), "const, type, or var declaration expected")
}
diff --git a/src/go/types/resolver.go b/src/go/types/resolver.go
index 5cbaba1..a462912 100644
--- a/src/go/types/resolver.go
+++ b/src/go/types/resolver.go
@@ -420,6 +420,10 @@
check.recordDef(d.Name, obj)
}
info := &declInfo{file: fileScope, fdecl: d}
+ // Methods are not package-level objects but we still track them in the
+ // object map so that we can handle them like regular functions (if the
+ // receiver is invalid); also we need their fdecl info when associating
+ // them with their receiver base type, below.
check.objMap[obj] = info
obj.setOrder(uint32(len(check.objMap)))
diff --git a/src/go/types/testdata/cycles.src b/src/go/types/testdata/cycles.src
index 59f112d..a9af46a 100644
--- a/src/go/types/testdata/cycles.src
+++ b/src/go/types/testdata/cycles.src
@@ -158,6 +158,8 @@
type T22 = chan [unsafe.Sizeof(func(ch T20){ _ = <-ch })]byte
func _() {
- type T1 chan [unsafe.Sizeof(func(ch T1){ _ = <-ch })]byte
- type T2 = chan [unsafe.Sizeof(func(ch T2){ _ = <-ch })]byte
+ type T0 func(T0)
+ type T1 /* ERROR cycle */ = func(T1)
+ type T2 chan [unsafe.Sizeof(func(ch T2){ _ = <-ch })]byte
+ type T3 /* ERROR cycle */ = chan [unsafe.Sizeof(func(ch T3){ _ = <-ch })]byte
}
diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go
index 45ada58..1da1f01 100644
--- a/src/go/types/typexpr.go
+++ b/src/go/types/typexpr.go
@@ -71,17 +71,6 @@
case *TypeName:
x.mode = typexpr
- // package-level alias cycles are now checked by Checker.objDecl
- if useCycleMarking {
- if check.objMap[obj] != nil {
- break
- }
- }
- if check.cycle(obj, path, true) {
- // maintain x.mode == typexpr despite error
- typ = Typ[Invalid]
- break
- }
case *Var:
// It's ok to mark non-local variables, but ignore variables
@@ -169,10 +158,8 @@
func (check *Checker) typ(e ast.Expr) Type {
// typExpr is called with a nil path indicating an indirection:
// push indir sentinel on object path
- if useCycleMarking {
- check.push(indir)
- defer check.pop()
- }
+ check.push(indir)
+ defer check.pop()
return check.typExpr(e, nil, nil)
}
