internal/core/adt: use depth counters for ancestor tracking

This change uses depth counters to be able to find ancestor
nodes that are currently being processed.

The old mechanism used the evaluatingArcs status to
determine this, as well as walking up the parent tree.

The new mechanism replaces evaluatingArcs, but also tracks
some parent nodes where the Parent field may not be set.
For instance, when computing an inline struct, the parent
node is not set (it is non rooted), but for the purpose
of cycle detection, the computing node should be seen
as a parent.

We keep the use of evaluatingArcs around for now as a
definsive mechanism and remove it in a later CL to
allow finding possible negative consequences in a
bisection.

Issue #2854

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: I958905711a8bbcdb51b862c6ee60da06f1d9972c
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202217
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Matthew Sackman <matthew@cue.works>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>

Author: mpvl

cue/testdata/benchmarks/issue1684.txtar

@@ -62,15 +62,15 @@ nestedClosed: passing: {
 	D: {id: {}} | {[string]: D}
 }
 -- out/evalalpha/stats --
-Leaks:  3341
-Freed:  794
-Reused: 794
-Allocs: 3341
+Leaks:  2135
+Freed:  530
+Reused: 530
+Allocs: 2135
 Retain: 0
 
-Unifications: 937
-Conjuncts:    6553
-Disjuncts:    1414
+Unifications: 475
+Conjuncts:    4333
+Disjuncts:    790
 -- out/evalalpha --
 (struct){
   #Secret: (#struct){
@@ -143,18 +143,18 @@ diff old new
 -Freed:  1064333
 -Reused: 1064282
 -Allocs: 51
-+Leaks:  3341
-+Freed:  794
-+Reused: 794
-+Allocs: 3341
++Leaks:  2135
++Freed:  530
++Reused: 530
++Allocs: 2135
  Retain: 0
 
 -Unifications: 792123
 -Conjuncts:    2480117
 -Disjuncts:    1064333
-+Unifications: 937
-+Conjuncts:    6553
-+Disjuncts:    1414
++Unifications: 475
++Conjuncts:    4333
++Disjuncts:    790
 -- diff/-out/evalalpha<==>+out/eval --
 diff old new
 --- old

cue/testdata/cycle/comprehension.txtar

@@ -310,10 +310,10 @@ issue2367: {
 }
 
 -- out/evalalpha/stats --
-Leaks:  782
-Freed:  60
-Reused: 60
-Allocs: 782
+Leaks:  779
+Freed:  63
+Reused: 63
+Allocs: 779
 Retain: 0
 
 Unifications: 499
@@ -913,10 +913,10 @@ diff old new
 -Reused: 1260
 -Allocs: 60
 -Retain: 145
-+Leaks:  782
-+Freed:  60
-+Reused: 60
-+Allocs: 782
++Leaks:  779
++Freed:  63
++Reused: 63
++Allocs: 779
 +Retain: 0
 
 -Unifications: 832

cue/testdata/cycle/evaluate.txtar

@@ -135,6 +135,8 @@ letCycleFail.t1.a.X: structural cycle:
     ./in.cue:33:11
 disjunctionCycle.b: cannot use 1 (type int) as list in argument 1 to and:
     ./in.cue:56:9
+b: structural cycle:
+    ./in.cue:62:6
 closeCycle.d: structural cycle:
     ./in.cue:73:11
 embedCycle: structural cycle:
@@ -226,11 +228,13 @@ Result:
     #A: (_|_){
       // [structural cycle]
       a: (_|_){
-        // [structural cycle]
+        // [structural cycle] b: structural cycle:
+        //     ./in.cue:62:6
       }
     }
     #B: (_|_){
-      // [structural cycle]
+      // [structural cycle] b: structural cycle:
+      //     ./in.cue:62:6
     }
   }
   letCycleWithAnd: (struct){
@@ -345,51 +349,51 @@ Result:
 diff old new
 --- old
 +++ new
-@@ -1,47 +1,47 @@
+@@ -1,47 +1,49 @@
  Errors:
 -closeCycle.a: structural cycle
 -closeCycle.b.d: structural cycle
-+printCycle.a.X: structural cycle
-+structCycle.c: structural cycle
-+letCycleOK.t2.a.X: structural cycle:
-+    ./in.cue:23:11
-+letCycleFail.t1.a.X: structural cycle:
-+    ./in.cue:33:11
-+disjunctionCycle.b: cannot use 1 (type int) as list in argument 1 to and:
-+    ./in.cue:56:9
-+closeCycle.d: structural cycle:
-+    ./in.cue:73:11
-+embedCycle: structural cycle:
-+    ./in.cue:85:12
-+listAddCycle.0.0: structural cycle:
-+    ./in.cue:91:5
-+listMulCycle.0.a.b: structural cycle:
-+    ./in.cue:97:5
- closeFail.x.b: field not allowed:
-+    ./in.cue:105:6
-     ./in.cue:104:6
+-closeFail.x.b: field not allowed:
+-    ./in.cue:104:6
 -    ./in.cue:105:12
 -    ./in.cue:106:6
-     ./in.cue:107:5
+-    ./in.cue:107:5
 -letCycleFail.t1.a.c: structural cycle
 -structCycle.a: structural cycle
 -structCycle.b.d: structural cycle
 -disjunctionCycle.a: cannot use 1 (type int) as list in argument 1 to and:
 -    ./in.cue:56:9
--disjunctionCycle.b: cannot use 1 (type int) as list in argument 1 to and:
--    ./in.cue:56:9
++printCycle.a.X: structural cycle
++structCycle.c: structural cycle
++letCycleOK.t2.a.X: structural cycle:
++    ./in.cue:23:11
++letCycleFail.t1.a.X: structural cycle:
++    ./in.cue:33:11
+ disjunctionCycle.b: cannot use 1 (type int) as list in argument 1 to and:
+     ./in.cue:56:9
 -disjunctionCycle.c: cannot use 1 (type int) as list in argument 1 to and:
 -    ./in.cue:56:9
--b: structural cycle:
--    ./in.cue:62:6
+ b: structural cycle:
+     ./in.cue:62:6
 -closeCycle.c: structural cycle:
 -    ./in.cue:73:15
 -structCycle.c: structural cycle:
 -    ./in.cue:79:14
--embedCycle: structural cycle:
++closeCycle.d: structural cycle:
++    ./in.cue:73:11
+ embedCycle: structural cycle:
 -    ./in.cue:85:11
 -printCycle.a.X.X: structural cycle:
 -    ./in.cue:113:6
++    ./in.cue:85:12
++listAddCycle.0.0: structural cycle:
++    ./in.cue:91:5
++listMulCycle.0.a.b: structural cycle:
++    ./in.cue:97:5
++closeFail.x.b: field not allowed:
++    ./in.cue:105:6
++    ./in.cue:104:6
++    ./in.cue:107:5
 
  Result:
  (_|_){
@@ -420,7 +424,7 @@ diff old new
        }
      }
    }
-@@ -56,20 +56,16 @@
+@@ -56,20 +58,16 @@
            // [structural cycle] letCycleFail.t1.a.X: structural cycle
          }
          c: (_|_){
@@ -451,7 +455,7 @@ diff old new
          }
          x: (struct){
            y: (string){ "" }
-@@ -85,15 +81,15 @@
+@@ -85,15 +83,15 @@
    disjunctionCycle: (_|_){
      // [eval]
      a: (_|_){
@@ -476,23 +480,7 @@ diff old new
        //     ./in.cue:56:9
      }
    }
-@@ -102,13 +98,11 @@
-     #A: (_|_){
-       // [structural cycle]
-       a: (_|_){
--        // [structural cycle] b: structural cycle:
--        //     ./in.cue:62:6
-+        // [structural cycle]
-       }
-     }
-     #B: (_|_){
--      // [structural cycle] b: structural cycle:
--      //     ./in.cue:62:6
-+      // [structural cycle]
-     }
-   }
-   letCycleWithAnd: (struct){
-@@ -118,41 +112,34 @@
+@@ -118,41 +116,34 @@
      }
      b: (struct){
      }
@@ -554,7 +542,7 @@ diff old new
      }
    }
    embedCycle: (_|_){
-@@ -159,39 +146,45 @@
+@@ -159,39 +150,45 @@
      // [structural cycle]
      a: (_|_){
        // [structural cycle] embedCycle: structural cycle:
@@ -623,7 +611,7 @@ diff old new
      }
    }
    closeFail: (_|_){
-@@ -201,21 +194,22 @@
+@@ -201,21 +198,22 @@
      }
      x: (_|_){
        // [eval]

cue/testdata/cycle/inline.txtar

@@ -153,14 +153,14 @@ inline: acrossFields: ok1: {
   }
 }
 -- out/evalalpha/stats --
-Leaks:  104
+Leaks:  92
 Freed:  0
 Reused: 0
-Allocs: 104
+Allocs: 92
 Retain: 0
 
-Unifications: 100
-Conjuncts:    498
+Unifications: 88
+Conjuncts:    409
 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:  104
++Leaks:  92
 +Freed:  0
 +Reused: 0
-+Allocs: 104
++Allocs: 92
 +Retain: 0
 
 -Unifications: 388
 -Conjuncts:    1307
 -Disjuncts:    707
-+Unifications: 100
-+Conjuncts:    498
++Unifications: 88
++Conjuncts:    409
 +Disjuncts:    0
 -- out/eval/stats --
 Leaks:  247
@@ -304,9 +304,9 @@ structural cycle:
 structural cycle:
     ./x.cue:20:9
 in: structural cycle:
-    ./x.cue:38:17
+    ./x.cue:34:17
 in: structural cycle:
-    ./x.cue:54:17
+    ./x.cue:49:17
 
 Result:
 (_|_){
@@ -372,15 +372,15 @@ Result:
         k00: (int){ 0 }
         k10: (_|_){
           // [structural cycle] in: structural cycle:
-          //     ./x.cue:38:17
+          //     ./x.cue:34:17
         }
         k20: (_|_){
           // [structural cycle] in: structural cycle:
-          //     ./x.cue:38:17
+          //     ./x.cue:34:17
         }
         k30: (_|_){
           // [structural cycle] in: structural cycle:
-          //     ./x.cue:38:17
+          //     ./x.cue:34:17
         }
       }
       ok1: (_|_){
@@ -392,15 +392,15 @@ Result:
         k00: (int){ 0 }
         k10: (_|_){
           // [structural cycle] in: structural cycle:
-          //     ./x.cue:54:17
+          //     ./x.cue:49:17
         }
         k20: (_|_){
           // [structural cycle] in: structural cycle:
-          //     ./x.cue:54:17
+          //     ./x.cue:49:17
         }
         k30: (_|_){
           // [structural cycle] in: structural cycle:
-          //     ./x.cue:54:17
+          //     ./x.cue:49:17
         }
       }
     }
@@ -421,9 +421,9 @@ diff old new
      ./x.cue:20:9
  in: structural cycle:
 -    ./x.cue:30:8
-+    ./x.cue:38:17
++    ./x.cue:34:17
 +in: structural cycle:
-+    ./x.cue:54:17
++    ./x.cue:49:17
 
  Result:
  (_|_){
@@ -452,15 +452,15 @@ diff old new
 -        }
 -      }
 -      ok1: (struct){
-+          //     ./x.cue:38:17
++          //     ./x.cue:34:17
 +        }
 +        k20: (_|_){
 +          // [structural cycle] in: structural cycle:
-+          //     ./x.cue:38:17
++          //     ./x.cue:34:17
 +        }
 +        k30: (_|_){
 +          // [structural cycle] in: structural cycle:
-+          //     ./x.cue:38:17
++          //     ./x.cue:34:17
 +        }
 +      }
 +      ok1: (_|_){
@@ -475,15 +475,15 @@ diff old new
 -        k30: (int){ 0 }
 +        k10: (_|_){
 +          // [structural cycle] in: structural cycle:
-+          //     ./x.cue:54:17
++          //     ./x.cue:49:17
 +        }
 +        k20: (_|_){
 +          // [structural cycle] in: structural cycle:
-+          //     ./x.cue:54:17
++          //     ./x.cue:49:17
 +        }
 +        k30: (_|_){
 +          // [structural cycle] in: structural cycle:
-+          //     ./x.cue:54:17
++          //     ./x.cue:49:17
 +        }
        }
      }

cue/testdata/cycle/structural.txtar

@@ -1329,7 +1329,7 @@ Result:
       cheeseburger: (float){ 3.00 }
       fries: (float){ 2.00 }
       sprite: (float){ 1.00 }
-      total: (int){ 18 }
+      total: (int){ 12 }
     }
   }
   listOptOK: (struct){
@@ -2206,6 +2206,15 @@ diff old new
      }
    }
    fieldsSumInfinite: (struct){
+@@ -753,7 +731,7 @@
+       cheeseburger: (float){ 3.00 }
+       fries: (float){ 2.00 }
+       sprite: (float){ 1.00 }
+-      total: (int){ 18 }
++      total: (int){ 12 }
+     }
+   }
+   listOptOK: (struct){
 @@ -767,9 +745,7 @@
        head: (int){ 3 }
        tail: (struct){

internal/core/adt/cycle.go

@@ -693,6 +693,20 @@ func (n *nodeContext) hasAncestorV3(arc *Vertex) bool {
 		return true
 	}
 
+	// TODO(evalv3): replace the use of the old status mechanism.
+	// the depth counters is an alternative mechanism to the status used in
+	// the v2 evaluator. It is slightly more precise:
+	//    - it allows detecting reference cycles as well (state evaluating is
+	//      no longer used in v3)
+	//    - it can capture cycles across inline structs, which do not have
+	//      Parent set.
+	// TODO: ensure that evalDepth is cleared when a node is finalized.
+	if s := arc.state; s != nil && arc.status != finalized {
+		if s.evalDepth > 0 && s.evalDepth < n.ctx.evalDepth {
+			return true
+		}
+	}
+
 	// 	TODO: insert test conditions for Bloom filter that guarantee that all
 	// 	parent nodes have been marked as "hot", in which case we can avoid this
 	// 	traversal.