]> Cypherpunks repositories - gostls13.git/commitdiff
go/types: correctly compute cycle length
authorRobert Griesemer <gri@golang.org>
Fri, 29 Jun 2018 21:11:46 +0000 (14:11 -0700)
committerIan Lance Taylor <iant@golang.org>
Tue, 10 Jul 2018 14:33:10 +0000 (14:33 +0000)
The existing algorithm assumed that the length of a cycle was simply
the number of elements in the cycle slice starting at the start object.
However, we use a special "indir" indirection object to indicate
pointer and other indirections that break the inline placement of
types in composite types. These indirection objects don't exist as
true named type objects, so don't count them anymore.

This removes an unnecessary cycle error in one of the existing tests
(testdata/issues.src:100).

Also:
- added more tracing support (only active if tracing is enabled)
- better documentation in parts
- r/check.typ/check.typExpr/ in a few of places where we don't
  need to record a type indirection

Found while investigating #26124.

Change-Id: I45341743225d979a72af3fbecfa05012b32fab67
Reviewed-on: https://go-review.googlesource.com/121755
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
src/go/types/check.go
src/go/types/decl.go
src/go/types/expr.go
src/go/types/testdata/issues.src

index 286b1f36a95784338a1b30b72629d2d01bed98eb..76d9c8917cb76c66358381b46b8408b54d081be3 100644 (file)
@@ -162,14 +162,7 @@ func (check *Checker) pop() Object {
 
 // pathString returns a string of the form a->b-> ... ->g for an object path [a, b, ... g].
 func (check *Checker) pathString() string {
-       var s string
-       for i, p := range check.objPath {
-               if i > 0 {
-                       s += "->"
-               }
-               s += p.Name()
-       }
-       return s
+       return objPathString(check.objPath)
 }
 
 // NewChecker returns a new Checker instance for a given package.
index e8e01541a3e95c366214e69f120295a513687ebf..763287adbe453dcdd6b4d93e3ea2ca81c97612b8 100644 (file)
@@ -38,6 +38,8 @@ func (check *Checker) declare(scope *Scope, id *ast.Ident, obj Object, pos token
 }
 
 // pathString returns a string of the form a->b-> ... ->g for a path [a, b, ... g].
+// TODO(gri) remove once we don't need the old cycle detection (explicitly passed
+//           []*TypeName path) anymore
 func pathString(path []*TypeName) string {
        var s string
        for i, p := range path {
@@ -49,6 +51,19 @@ func pathString(path []*TypeName) string {
        return s
 }
 
+// objPathString returns a string of the form a->b-> ... ->g for a path [a, b, ... g].
+// TODO(gri) s/objPathString/pathString/ once we got rid of pathString above
+func objPathString(path []Object) string {
+       var s string
+       for i, p := range path {
+               if i > 0 {
+                       s += "->"
+               }
+               s += p.Name()
+       }
+       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
@@ -224,16 +239,18 @@ var indir = NewTypeName(token.NoPos, nil, "*", nil)
 // 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) bool {
+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()
        }
 
-       // We distinguish between cycles involving only constants and variables
-       // (nval = len(cycle)), cycles involving types (and functions) only
-       // (nval == 0), and mixed cycles (nval != 0 && nval != len(cycle)).
+       // Given the number of constants and variables (nval) in the cycle
+       // and the cycle length (ncycle = number of named objects in the cycle),
+       // we distinguish between cycles involving only constants and variables
+       // (nval = ncycle), cycles involving types (and functions) only
+       // (nval == 0), and mixed cycles (nval != 0 && nval != ncycle).
        // We ignore functions at the moment (taking them into account correctly
        // is complicated and it doesn't improve error reporting significantly).
        //
@@ -242,17 +259,19 @@ func (check *Checker) typeCycle(obj Object) bool {
        // cannot be computed (it's either infinite or 0); if there is no type
        // definition, we have a sequence of alias type names which will expand
        // ad infinitum.
-       var nval int
+       var nval, ncycle int
        var hasIndir, hasTDef bool
        assert(obj.color() >= grey)
        start := obj.color() - grey // index of obj in objPath
        cycle := check.objPath[start:]
+       ncycle = len(cycle) // including indirections
        for _, obj := range cycle {
                switch obj := obj.(type) {
                case *Const, *Var:
                        nval++
                case *TypeName:
                        if obj == indir {
+                               ncycle-- // don't count (indirections are not objects)
                                hasIndir = true
                        } else if !check.objMap[obj].alias {
                                hasTDef = true
@@ -264,10 +283,20 @@ func (check *Checker) typeCycle(obj Object) bool {
                }
        }
 
+       if trace {
+               check.trace(obj.Pos(), "## cycle detected: objPath = %s->%s (len = %d)", objPathString(cycle), obj.Name(), ncycle)
+               check.trace(obj.Pos(), "## cycle contains: %d values, has indirection = %v, has type definition = %v", nval, hasIndir, hasTDef)
+               defer func() {
+                       if isCycle {
+                               check.trace(obj.Pos(), "=> error: cycle is invalid")
+                       }
+               }()
+       }
+
        // A cycle involving only constants and variables is invalid but we
        // ignore them here because they are reported via the initialization
        // cycle check.
-       if nval == len(cycle) {
+       if nval == ncycle {
                return false
        }
 
@@ -305,7 +334,7 @@ func (check *Checker) constDecl(obj *Const, typ, init ast.Expr) {
 
        // determine type, if any
        if typ != nil {
-               t := check.typ(typ)
+               t := check.typExpr(typ, nil, nil)
                if !isConstType(t) {
                        // don't report an error if the type is an invalid C (defined) type
                        // (issue #22090)
@@ -331,7 +360,7 @@ func (check *Checker) varDecl(obj *Var, lhs []*Var, typ, init ast.Expr) {
 
        // determine type, if any
        if typ != nil {
-               obj.typ = check.typ(typ)
+               obj.typ = check.typExpr(typ, nil, nil)
                // We cannot spread the type to all lhs variables if there
                // are more than one since that would mark them as checked
                // (see Checker.objDecl) and the assignment of init exprs,
index 3f3c4f83c6489ec0a140273e00a6be0f2d707c0d..39ee6bcca3573d92c58ea13faf572414c82051e7 100644 (file)
@@ -1431,7 +1431,7 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind {
                        check.invalidAST(e.Pos(), "use of .(type) outside type switch")
                        goto Error
                }
-               T := check.typ(e.Type)
+               T := check.typExpr(e.Type, nil, nil)
                if T == Typ[Invalid] {
                        goto Error
                }
index d727c3b3e23f1eaa75cf19ca874b5b60b335f3ce..9750bdc2e22b94fb3df4c25953cfa44faf0ce0dd 100644 (file)
@@ -97,7 +97,7 @@ func issue10979() {
 
 // issue11347
 // These should not crash.
-var a1, b1 /* ERROR cycle */ /* ERROR cycle */ , c1 /* ERROR cycle */ b1 = 0 > 0<<""[""[c1]]>c1
+var a1, b1 /* ERROR cycle */ , c1 /* ERROR cycle */ b1 = 0 > 0<<""[""[c1]]>c1
 var a2, b2 /* ERROR cycle */ = 0 /* ERROR cannot initialize */ /* ERROR cannot initialize */ > 0<<""[b2]
 var a3, b3 /* ERROR cycle */ = int /* ERROR cannot initialize */ /* ERROR cannot initialize */ (1<<""[b3])