internal/core/adt: remove cycle check specifcally for dynamic vertices

Now the cycle algorithm treats dynamic vertices as
rewrites of regular fields, it should, in theory, no longer
need to check cycles. This appears to be at least the case
for references referring to ancestors. We remove the
respective block accordingly. This fixes one spurious
cycle.

An alternative, more conservative, fix to the same issue is
to exclude reporting the error if n.hasNoCycle is set:

		if (n.node.IsDynamic || ci.Inline) && !n.hasNonCycle {
			n.reportCycleError()
			return ci, true
		}

So if this removal can be identified down the line as
breaking cycle detection, it can be reverted to that.

Note that this adds one counter error. We will address
counter errors later.

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: Ie4816869693a656e484aa132b4b126dff86484be
Dispatch-Trailer: {"type":"trybot","CL":1206384,"patchset":1,"ref":"refs/changes/84/1206384/1","targetBranch":"master"}

cue/testdata/cycle/evaluate.txtar

@@ -131,8 +131,6 @@ closeCycle.c: structural cycle
 letCycleFail.t1.a.c: structural cycle
 printCycle.a.X: structural cycle
 structCycle.c: structural cycle
-letCycleOK.t2.a.X: structural cycle:
-    ./in.cue:23:11
 disjunctionCycle.b: cannot use 1 (type int) as type list:
     ./in.cue:56:5
     ./in.cue:56:9
@@ -152,25 +150,19 @@ closeFail.x.b: field not allowed:
 Result:
 (_|_){
   // [eval]
-  letCycleOK: (_|_){
-    // [structural cycle]
+  letCycleOK: (struct){
     t1: (struct){
       b: (_){ _ }
       let X#1 = (_){ _ }
       c: (_){ _ }
     }
-    t2: (_|_){
-      // [structural cycle]
-      a: (_|_){
-        // [structural cycle]
+    t2: (struct){
+      a: (struct){
         b: (int){ 1 }
         let X#2 = (_|_){
           // [structural cycle] letCycleOK.t2.a.X: structural cycle
         }
-        c: (_|_){
-          // [structural cycle] letCycleOK.t2.a.X: structural cycle:
-          //     ./in.cue:23:11
-        }
+        c: (int){ 1 }
       }
     }
   }
@@ -348,7 +340,7 @@ Result:
 diff old new
 --- old
 +++ new
-@@ -1,50 +1,48 @@
+@@ -1,33 +1,23 @@
  Errors:
 -closeCycle.a: structural cycle
 -closeCycle.b.d: structural cycle
@@ -366,8 +358,6 @@ diff old new
 -    ./in.cue:56:9
 +printCycle.a.X: structural cycle
 +structCycle.c: structural cycle
-+letCycleOK.t2.a.X: structural cycle:
-+    ./in.cue:23:11
  disjunctionCycle.b: cannot use 1 (type int) as type list:
      ./in.cue:56:5
      ./in.cue:56:9
@@ -397,34 +387,7 @@ diff old new
 
  Result:
  (_|_){
-   // [eval]
--  letCycleOK: (struct){
-+  letCycleOK: (_|_){
-+    // [structural cycle]
-     t1: (struct){
-       b: (_){ _ }
-       let X#1 = (_){ _ }
-       c: (_){ _ }
-     }
--    t2: (struct){
--      a: (struct){
-+    t2: (_|_){
-+      // [structural cycle]
-+      a: (_|_){
-+        // [structural cycle]
-         b: (int){ 1 }
-         let X#2 = (_|_){
-           // [structural cycle] letCycleOK.t2.a.X: structural cycle
-         }
--        c: (int){ 1 }
-+        c: (_|_){
-+          // [structural cycle] letCycleOK.t2.a.X: structural cycle:
-+          //     ./in.cue:23:11
-+        }
-       }
-     }
-   }
-@@ -65,14 +63,9 @@
+@@ -65,14 +55,9 @@
      }
      t2: (struct){
        a: (struct){
@@ -442,7 +405,7 @@ diff old new
          }
          x: (struct){
            y: (string){ "" }
-@@ -88,17 +81,17 @@
+@@ -88,17 +73,17 @@
    disjunctionCycle: (_|_){
      // [eval]
      a: (_|_){
@@ -471,7 +434,7 @@ diff old new
        //     ./in.cue:56:5
        //     ./in.cue:56:9
      }
-@@ -124,80 +117,77 @@
+@@ -124,80 +109,77 @@
      }
      b: (struct){
      }
@@ -601,7 +564,7 @@ diff old new
      }
    }
    closeFail: (_|_){
-@@ -207,21 +197,22 @@
+@@ -207,21 +189,22 @@
      }
      x: (_|_){
        // [eval]
@@ -629,8 +592,6 @@ diff old new
      }
    }
  }
--- diff/todo/p1 --
-letCycleOK.t2: spurious error
 -- out/eval --
 Errors:
 closeCycle.a: structural cycle

cue/testdata/cycle/inline.txtar

@@ -153,14 +153,14 @@ inline: acrossFields: ok1: {
   }
 }
 -- out/evalalpha/stats --
-Leaks:  137
+Leaks:  148
 Freed:  0
 Reused: 0
-Allocs: 137
+Allocs: 148
 Retain: 0
 
-Unifications: 101
-Conjuncts:    509
+Unifications: 108
+Conjuncts:    552
 Disjuncts:    0
 -- diff/-out/evalalpha/stats<==>+out/eval/stats --
 diff old new
@@ -172,17 +172,17 @@ diff old new
 -Reused: 136
 -Allocs: 252
 -Retain: 834
-+Leaks:  137
++Leaks:  148
 +Freed:  0
 +Reused: 0
-+Allocs: 137
++Allocs: 148
 +Retain: 0
 
 -Unifications: 388
 -Conjuncts:    1307
 -Disjuncts:    707
-+Unifications: 101
-+Conjuncts:    509
++Unifications: 108
++Conjuncts:    552
 +Disjuncts:    0
 -- out/eval/stats --
 Leaks:  247

internal/core/adt/cycle.go

@@ -632,11 +632,6 @@ func (n *nodeContext) detectCycleV3(arc *Vertex, env *Environment, x Resolver, c
 	// we also can terminate, as this is a structural cycle.
 	// TODO: use depth or check direct ancestry.
 	if n.hasAncestorV3(arc) {
-		if n.node.IsDynamic || ci.Inline {
-			n.reportCycleError()
-			return ci, true
-		}
-
 		return n.markCyclicV3(arc, env, x, ci)
 	}
 

internal/core/adt/eval_test.go

@@ -83,6 +83,7 @@ var skipDebugDepErrors = map[string]int{
 	"comprehensions/pushdown":  2,
 	"cycle/comprehension":      2,
 	"cycle/disjunction":        4,
+	"cycle/evaluate":           1,
 	"cycle/structural":         14,
 	"disjunctions/edge":        1,
 	"disjunctions/errors":      2,