internal/core/adt: fix cycle bug

In the new implementation, ctx.Unify(ctx, arc, AllArcs) had to go.
The issue is that the desired evaluation state was not passed
down and AllArcs was used unconditionally. This means that for
Evaluate it would evaluate too much, causing a cycle to be
triggered.

With the new incremental algorithm, this call to Unify was also
redundant. In fact, not calling it results in more consistent error
messages and a more consistent ordering of evaluation.

It has therefore been removed.

Fixes #622

This also improves various other evaluations.

The changes in errormessages to structural.txtar are typciall for
changes in order of evaluation are fine and in spec, as long as the
same set of nodes are marked as an error (sub nodes of erroring
nodes may be removed or added from the output).

Change-Id: Icbd24891a48e421be900df40f622fd344a0bed4c
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8211
Reviewed-by: Marcel van Lohuizen <[email protected]>
Reviewed-by: CUE cueckoo <[email protected]>
Reviewed-by: Paul Jolly <[email protected]>

cue/testdata/cycle/051_resolved_self-reference_cycles_with_disjunction.txtar

@@ -181,8 +181,8 @@ Result:
   }
   xb3: (int){ |((int){ 6 }, (int){ 9 }) }
   xb4: (_|_){
-    // [cycle] cycle error:
-    //     ./in.cue:20:6
+    // [incomplete] xb2: unresolved disjunction 6 | 9 (type int):
+    //     ./in.cue:18:6
   }
   xc1: (int){ |((int){ 8 }, (int){ 9 }) }
   xc2: (int){ 8 }
@@ -230,8 +230,8 @@ Result:
   }
   xf3: (int){ |((int){ 6 }, (int){ 9 }) }
   xf4: (_|_){
-    // [cycle] cycle error:
-    //     ./in.cue:54:6
+    // [incomplete] xf2: unresolved disjunction 6 | 9 (type int):
+    //     ./in.cue:52:6
   }
   z1: (int){ |((int){ 11 }, (int){ 13 }) }
   z2: (int){ 10 }

cue/testdata/cycle/052_resolved_self-reference_cycles_with_disjunction_with_defaults.txtar

@@ -147,10 +147,7 @@ Result:
   xb1: (int){ 8 }
   xb2: (int){ 8 }
   xb3: (int){ |(*(int){ 6 }, (int){ 9 }) }
-  xb4: (_|_){
-    // [cycle] cycle error:
-    //     ./in.cue:14:6
-  }
+  xb4: (int){ 10 }
   xc1: (int){ |(*(int){ 8 }, (int){ 9 }) }
   xc2: (int){ 8 }
   xc3: (int){ 6 }

cue/testdata/cycle/expression.txtar

@@ -0,0 +1,272 @@
+-- in.cue --
+t1: a: {
+    a1: a0 + 2
+    b1: a1 - 2
+    a0: X
+    X: b1
+    X: 5.0
+}
+
+t1: b: {
+    X: 5.0
+    a1: a0 + 2
+    b1: a1 - 2
+    a0: X
+    X: b1
+}
+
+t1: c: {
+    a1: a0 + 2
+    b1: a1 - 2
+    X: 5.0
+    a0: X
+    X: b1
+}
+
+t1: c: {
+    b1: a1 - 2
+    a1: a0 + 2
+    X: 5.0
+    a0: X
+    X: b1
+}
+
+t1: c: {
+    b1: a1 - 2
+    X: 5.0
+    a1: a0 + 2
+    a0: X
+    X: b1
+}
+
+t1total: {
+    for _, v in t1 { v }
+}
+
+// Issue #622
+t2: a: {
+    a0: X
+    a1: a0 * 2
+    Y: a1
+
+    b0: Y
+    b1: b0 / 2
+    X: b1
+
+    X: 5.0
+}
+
+t2: b: {
+    b0: Y
+    b1: b0 / 2
+    X: b1
+
+    a0: X
+    a1: a0 * 2
+    Y: a1
+
+    X: 5.0
+}
+
+t2: c: {
+    b0: Y
+    b1: b0 / 2
+    X: b1
+
+    X: 5.0
+
+    a0: X
+    a1: a0 * 2
+    Y: a1
+}
+
+t2: d: {
+    b0: Y
+    b1: b0 / 2
+    X: b1
+
+    a0: X
+    a1: a0 * 2
+    Y: a1
+
+    X: 5.0
+}
+
+t2total: {
+    for _, v in t2 { v }
+}
+
+-- out/eval --
+(struct){
+  t1: (struct){
+    a: (struct){
+      a1: (float){ 7.0 }
+      b1: (float){ 5.0 }
+      a0: (float){ 5.0 }
+      X: (float){ 5.0 }
+    }
+    b: (struct){
+      X: (float){ 5.0 }
+      a1: (float){ 7.0 }
+      b1: (float){ 5.0 }
+      a0: (float){ 5.0 }
+    }
+    c: (struct){
+      a1: (float){ 7.0 }
+      b1: (float){ 5.0 }
+      X: (float){ 5.0 }
+      a0: (float){ 5.0 }
+    }
+  }
+  t1total: (struct){
+    a1: (float){ 7.0 }
+    b1: (float){ 5.0 }
+    a0: (float){ 5.0 }
+    X: (float){ 5.0 }
+  }
+  t2: (struct){
+    a: (struct){
+      a0: (float){ 5.0 }
+      a1: (float){ 10.0 }
+      Y: (float){ 10.0 }
+      b0: (float){ 10.0 }
+      b1: (float){ 5.0 }
+      X: (float){ 5.0 }
+    }
+    b: (struct){
+      b0: (float){ 10.0 }
+      b1: (float){ 5.0 }
+      X: (float){ 5.0 }
+      a0: (float){ 5.0 }
+      a1: (float){ 10.0 }
+      Y: (float){ 10.0 }
+    }
+    c: (struct){
+      b0: (float){ 10.0 }
+      b1: (float){ 5.0 }
+      X: (float){ 5.0 }
+      a0: (float){ 5.0 }
+      a1: (float){ 10.0 }
+      Y: (float){ 10.0 }
+    }
+    d: (struct){
+      b0: (float){ 10.0 }
+      b1: (float){ 5.0 }
+      X: (float){ 5.0 }
+      a0: (float){ 5.0 }
+      a1: (float){ 10.0 }
+      Y: (float){ 10.0 }
+    }
+  }
+  t2total: (struct){
+    a0: (float){ 5.0 }
+    a1: (float){ 10.0 }
+    Y: (float){ 10.0 }
+    b0: (float){ 10.0 }
+    b1: (float){ 5.0 }
+    X: (float){ 5.0 }
+  }
+}
+-- out/compile --
+--- in.cue
+{
+  t1: {
+    a: {
+      a1: (〈0;a0〉 + 2)
+      b1: (〈0;a1〉 - 2)
+      a0: 〈0;X〉
+      X: 〈0;b1〉
+      X: 5.0
+    }
+  }
+  t1: {
+    b: {
+      X: 5.0
+      a1: (〈0;a0〉 + 2)
+      b1: (〈0;a1〉 - 2)
+      a0: 〈0;X〉
+      X: 〈0;b1〉
+    }
+  }
+  t1: {
+    c: {
+      a1: (〈0;a0〉 + 2)
+      b1: (〈0;a1〉 - 2)
+      X: 5.0
+      a0: 〈0;X〉
+      X: 〈0;b1〉
+    }
+  }
+  t1: {
+    c: {
+      b1: (〈0;a1〉 - 2)
+      a1: (〈0;a0〉 + 2)
+      X: 5.0
+      a0: 〈0;X〉
+      X: 〈0;b1〉
+    }
+  }
+  t1: {
+    c: {
+      b1: (〈0;a1〉 - 2)
+      X: 5.0
+      a1: (〈0;a0〉 + 2)
+      a0: 〈0;X〉
+      X: 〈0;b1〉
+    }
+  }
+  t1total: {
+    for _, v in 〈1;t1〉 {
+      〈1;v〉
+    }
+  }
+  t2: {
+    a: {
+      a0: 〈0;X〉
+      a1: (〈0;a0〉 * 2)
+      Y: 〈0;a1〉
+      b0: 〈0;Y〉
+      b1: (〈0;b0〉 / 2)
+      X: 〈0;b1〉
+      X: 5.0
+    }
+  }
+  t2: {
+    b: {
+      b0: 〈0;Y〉
+      b1: (〈0;b0〉 / 2)
+      X: 〈0;b1〉
+      a0: 〈0;X〉
+      a1: (〈0;a0〉 * 2)
+      Y: 〈0;a1〉
+      X: 5.0
+    }
+  }
+  t2: {
+    c: {
+      b0: 〈0;Y〉
+      b1: (〈0;b0〉 / 2)
+      X: 〈0;b1〉
+      X: 5.0
+      a0: 〈0;X〉
+      a1: (〈0;a0〉 * 2)
+      Y: 〈0;a1〉
+    }
+  }
+  t2: {
+    d: {
+      b0: 〈0;Y〉
+      b1: (〈0;b0〉 / 2)
+      X: 〈0;b1〉
+      a0: 〈0;X〉
+      a1: (〈0;a0〉 * 2)
+      Y: 〈0;a1〉
+      X: 5.0
+    }
+  }
+  t2total: {
+    for _, v in 〈1;t2〉 {
+      〈1;v〉
+    }
+  }
+}

cue/testdata/cycle/structural.txtar

@@ -428,7 +428,6 @@ b6.x.a.0: structural cycle
 b7.a.0: structural cycle
 c1.a.c.c: structural cycle
 d1.a.b.c.d.t: structural cycle
-d1.r: structural cycle
 d2.r.c.d.t: structural cycle
 d2.x.d.t.c.d.t: structural cycle
 e1.a.c: structural cycle
@@ -490,7 +489,6 @@ p3.#U.#T.a.b.link: structural cycle
 p5.#T.a.0.link: structural cycle
 p6.#U.#T.a.0.link: structural cycle
 z1.z.f.h.h: structural cycle
-z1.z.g.h: structural cycle
 cycle error:
     ./in.cue:144:10
 0: structural cycle:
@@ -1074,14 +1072,7 @@ Result:
       }
     }
     r: (_|_){
-      // [structural cycle] d1.r: structural cycle
-      c: (_|_){// {
-        //   d: {
-        //     h: int
-        //     t: 〈4;r〉
-        //   }
-        // }
-      }
+      // [structural cycle] d1.a.b.c.d.t: structural cycle
     }
     x: (_|_){
       // [structural cycle] d1.a.b.c.d.t: structural cycle
@@ -1412,9 +1403,6 @@ Result:
       }
       g: (_|_){
         // [structural cycle]
-        h: (_|_){
-          // [structural cycle] z1.z.g.h: structural cycle
-        }
       }
     }
   }

internal/core/adt/eval.go

@@ -1213,13 +1213,6 @@ func (n *nodeContext) addVertexConjuncts(env *Environment, closeInfo CloseInfo,
 
 	closeInfo = closeInfo.SpawnRef(arc, IsDef(x), x)
 
-	// TODO: uncommenting the following almost works, but causes some
-	// faulty results in complex cycle handling between disjunctions.
-	// The reason is that disjunctions must be eliminated if checks in
-	// values on which they depend fail.
-	ctx := n.ctx
-	ctx.Unify(ctx, arc, AllArcs)
-
 	for _, c := range arc.Conjuncts {
 		var a []*Vertex
 		if env != nil {