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>

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 {

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]

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.