go/types: simplify some code and remove TODOs (cleanup)

- remove Checker.cycle in favor of using a "seen" map
- rename Checker.typeCycle -> Checker.cycle
- remove TODO in api.go since the API is frozen

Change-Id: I182a8215978dad54e9c6e79c21c5ec88ec802349
Reviewed-on: https://go-review.googlesource.com/c/go/+/198042
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

diff --git a/src/go/types/api.go b/src/go/types/api.go
index 6699231fce1072d1f98623e867a215d66735442d..2a21ad0c53f2e36c9ff5e1822424a112ee0c63d1 100644 (file)
--- a/src/go/types/api.go
+++ b/src/go/types/api.go
@@ -259,9 +259,6 @@ type TypeAndValue struct {
        Value constant.Value
 }
 
-// TODO(gri) Consider eliminating the IsVoid predicate. Instead, report
-// "void" values as regular values but with the empty tuple type.
-
 // IsVoid reports whether the corresponding expression
 // is a function call without results.
 func (tv TypeAndValue) IsVoid() bool {

diff --git a/src/go/types/decl.go b/src/go/types/decl.go
index 4485ea8c71a4707cd76ce93bbfb61a96213afa6e..d0027aeb8e8fa0fd73754ed87c44d7a45c178a8d 100644 (file)
--- a/src/go/types/decl.go
+++ b/src/go/types/decl.go
@@ -125,17 +125,17 @@ func (check *Checker) objDecl(obj Object, def *Named) {
                // order code.
                switch obj := obj.(type) {
                case *Const:
-                       if check.typeCycle(obj) || obj.typ == nil {
+                       if check.cycle(obj) || obj.typ == nil {
                                obj.typ = Typ[Invalid]
                        }
 
                case *Var:
-                       if check.typeCycle(obj) || obj.typ == nil {
+                       if check.cycle(obj) || obj.typ == nil {
                                obj.typ = Typ[Invalid]
                        }
 
                case *TypeName:
-                       if check.typeCycle(obj) {
+                       if check.cycle(obj) {
                                // break cycle
                                // (without this, calling underlying()
                                // below may lead to an endless loop
@@ -145,7 +145,7 @@ func (check *Checker) objDecl(obj Object, def *Named) {
                        }
 
                case *Func:
-                       if check.typeCycle(obj) {
+                       if check.cycle(obj) {
                                // Don't set obj.typ to Typ[Invalid] here
                                // because plenty of code type-asserts that
                                // functions have a *Signature type. Grey
@@ -198,11 +198,9 @@ func (check *Checker) objDecl(obj Object, def *Named) {
        }
 }
 
-// typeCycle checks if the cycle starting with obj is valid and
+// cycle checks if the cycle starting with obj is valid and
 // reports an error if it is not.
-// TODO(gri) rename s/typeCycle/cycle/ once we don't need the other
-// cycle method anymore.
-func (check *Checker) typeCycle(obj Object) (isCycle bool) {
+func (check *Checker) cycle(obj Object) (isCycle bool) {
        // The object map contains the package scope objects and the non-interface methods.
        if debug {
                info := check.objMap[obj]

diff --git a/src/go/types/resolver.go b/src/go/types/resolver.go
index 93de63b059346730fab27a02653c3f43a2840551..20730340edd2ec14e301ef9a1d986c2fe850bc6d 100644 (file)
--- a/src/go/types/resolver.go
+++ b/src/go/types/resolver.go
@@ -482,7 +482,7 @@ func (check *Checker) resolveBaseTypeName(typ ast.Expr) (ptr bool, base *TypeNam
        // non-alias type name. If we encounter anything but pointer types or
        // parentheses we're done. If we encounter more than one pointer type
        // we're done.
-       var path []*TypeName
+       var seen map[*TypeName]bool
        for {
                typ = unparen(typ)
 
@@ -496,7 +496,7 @@ func (check *Checker) resolveBaseTypeName(typ ast.Expr) (ptr bool, base *TypeNam
                        typ = unparen(pexpr.X) // continue with pointer base type
                }
 
-               // typ must be the name
+               // typ must be a name
                name, _ := typ.(*ast.Ident)
                if name == nil {
                        return false, nil
@@ -516,7 +516,7 @@ func (check *Checker) resolveBaseTypeName(typ ast.Expr) (ptr bool, base *TypeNam
                }
 
                // ... which we have not seen before
-               if check.cycle(tname, path, false) {
+               if seen[tname] {
                        return false, nil
                }
 
@@ -529,28 +529,11 @@ func (check *Checker) resolveBaseTypeName(typ ast.Expr) (ptr bool, base *TypeNam
 
                // otherwise, continue resolving
                typ = tdecl.typ
-               path = append(path, tname)
-       }
-}
-
-// cycle reports whether obj appears in path or not.
-// If it does, and report is set, it also reports a cycle error.
-func (check *Checker) cycle(obj *TypeName, path []*TypeName, report bool) bool {
-       // (it's ok to iterate forward because each named type appears at most once in path)
-       for i, prev := range path {
-               if prev == obj {
-                       if report {
-                               check.errorf(obj.pos, "illegal cycle in declaration of %s", obj.name)
-                               // print cycle
-                               for _, obj := range path[i:] {
-                                       check.errorf(obj.Pos(), "\t%s refers to", obj.Name()) // secondary error, \t indented
-                               }
-                               check.errorf(obj.Pos(), "\t%s", obj.Name())
-                       }
-                       return true
+               if seen == nil {
+                       seen = make(map[*TypeName]bool)
                }
+               seen[tname] = true
        }
-       return false
 }
 
 // packageObjects typechecks all package objects, but not function bodies.