internal/core/adt: do not count validators as new data

To detect cycles, we track which conjuncts are cyclic and not, and we allow cyclic conjuncts as long as non-cyclic conjuncts are added. In this CL, we do not count validators as non-cyclic, but rather inert. This has the advantage that some structural cycles are detected earlier. Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: I4a20ec2dad1f07261f8e2859885ef781286f71c2 Reviewed-on: https://cue.gerrithub.io/c/cue-lang/cue/+/1208007 Reviewed-by: Matthew Sackman <[email protected]> Unity-Result: CUE porcuepine <[email protected]> TryBot-Result: CUEcueckoo <[email protected]>
cue-lang · Jan 30, 2025 · 5b4a02e · 5b4a02e
1 parent 36174e5
commit 5b4a02e
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 70 deletions.
diff --git a/cue/testdata/cycle/builtins.txtar b/cue/testdata/cycle/builtins.txtar
@@ -209,14 +209,14 @@ selfCycle: t2: {
 -- todo/p1 --
 issue3443.noCycle: fix hang
 -- out/evalalpha/stats --
-Leaks:  352
+Leaks:  314
 Freed:  17
 Reused: 17
-Allocs: 352
+Allocs: 314
 Retain: 0
 
-Unifications: 309
-Conjuncts:    1189
+Unifications: 272
+Conjuncts:    1059
 Disjuncts:    28
 -- diff/-out/evalalpha/stats<==>+out/eval/stats --
 diff old new
@@ -228,17 +228,17 @@ diff old new
 -Reused: 357
 -Allocs: 34
 -Retain: 95
-+Leaks:  352
++Leaks:  314
 +Freed:  17
 +Reused: 17
-+Allocs: 352
++Allocs: 314
 +Retain: 0
 
 -Unifications: 363
 -Conjuncts:    645
 -Disjuncts:    454
-+Unifications: 309
-+Conjuncts:    1189
++Unifications: 272
++Conjuncts:    1059
 +Disjuncts:    28
 -- out/eval/stats --
 Leaks:  22
@@ -264,20 +264,11 @@ noCycle.t1._s.#x: invalid value {} (does not satisfy matchN): 0 matched, expecte
     ./cycle.cue:6:7
     ./cycle.cue:6:14
     ./cycle.cue:7:13
-noCycle.t1._s.#x.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1:
-    ./cycle.cue:6:7
-    ./cycle.cue:6:14
-    ./cycle.cue:7:13
 selfCycle.t1.c: invalid value {d:{}} (does not satisfy matchN): 0 matched, expected 1:
     ./cycle.cue:47:5
     ./cycle.cue:47:12
     ./cycle.cue:48:5
-selfCycle.t1.c.d: invalid value {d:{}} (does not satisfy matchN): 0 matched, expected 1:
-    ./cycle.cue:47:5
-    ./cycle.cue:47:12
-    ./cycle.cue:47:20
-    ./cycle.cue:48:8
-selfCycle.t1.c.d.d: structural cycle:
+selfCycle.t1.c.d: structural cycle:
     ./cycle.cue:47:5
 selfCycle.t2.c: invalid value {} (does not satisfy matchN): 0 matched, expected 1:
     ./cycle.cue:53:5
@@ -302,12 +293,7 @@ issue3443.cycle1.cycle.s.n.n: structural cycle:
 issue3443.cycle2.fail.#S: invalid value {n:{n:{n:_}}} (does not satisfy matchN): 0 matched, expected 1:
     ./matchn.cue:55:13
     ./matchn.cue:55:20
-issue3443.cycle2.fail.#S.n: invalid value {n:{n:{n:_}}} (does not satisfy matchN): 0 matched, expected 1:
-    ./matchn.cue:55:13
-    ./matchn.cue:55:20
-    ./matchn.cue:55:28
-    ./matchn.cue:55:40
-issue3443.cycle2.fail.#S.n.n: structural cycle:
+issue3443.cycle2.fail.#S.n: structural cycle:
     ./matchn.cue:55:13
 
 Result:
@@ -324,10 +310,6 @@ Result:
           //     ./cycle.cue:6:7
           //     ./cycle.cue:6:14
           //     ./cycle.cue:7:13
-          // noCycle.t1._s.#x.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1:
-          //     ./cycle.cue:6:7
-          //     ./cycle.cue:6:14
-          //     ./cycle.cue:7:13
         }
       }
       #x: (_|_){
@@ -420,12 +402,7 @@ Result:
         //     ./cycle.cue:47:5
         //     ./cycle.cue:47:12
         //     ./cycle.cue:48:5
-        // selfCycle.t1.c.d: invalid value {d:{}} (does not satisfy matchN): 0 matched, expected 1:
-        //     ./cycle.cue:47:5
-        //     ./cycle.cue:47:12
-        //     ./cycle.cue:47:20
-        //     ./cycle.cue:48:8
-        // selfCycle.t1.c.d.d: structural cycle:
+        // selfCycle.t1.c.d: structural cycle:
         //     ./cycle.cue:47:5
         d: (struct){
         }
@@ -605,12 +582,7 @@ Result:
           // [eval] issue3443.cycle2.fail.#S: invalid value {n:{n:{n:_}}} (does not satisfy matchN): 0 matched, expected 1:
           //     ./matchn.cue:55:13
           //     ./matchn.cue:55:20
-          // issue3443.cycle2.fail.#S.n: invalid value {n:{n:{n:_}}} (does not satisfy matchN): 0 matched, expected 1:
-          //     ./matchn.cue:55:13
-          //     ./matchn.cue:55:20
-          //     ./matchn.cue:55:28
-          //     ./matchn.cue:55:40
-          // issue3443.cycle2.fail.#S.n.n: structural cycle:
+          // issue3443.cycle2.fail.#S.n: structural cycle:
           //     ./matchn.cue:55:13
           n: (#struct){
             n: (#struct){
@@ -651,17 +623,30 @@ Result:
 diff old new
 --- old
 +++ new
-@@ -24,29 +24,28 @@
+@@ -11,57 +11,36 @@
+     ./cycle.cue:6:7
+     ./cycle.cue:6:14
+     ./cycle.cue:7:13
+-noCycle.t1._s.#x.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1:
+-    ./cycle.cue:6:7
+-    ./cycle.cue:6:14
+-    ./cycle.cue:7:13
+ selfCycle.t1.c: invalid value {d:{}} (does not satisfy matchN): 0 matched, expected 1:
+     ./cycle.cue:47:5
      ./cycle.cue:47:12
-     ./cycle.cue:47:20
-     ./cycle.cue:48:8
+     ./cycle.cue:48:5
+-selfCycle.t1.c.d: invalid value {d:{}} (does not satisfy matchN): 0 matched, expected 1:
+-    ./cycle.cue:47:5
+-    ./cycle.cue:47:12
+-    ./cycle.cue:47:20
+-    ./cycle.cue:48:8
 -selfCycle.t1.c.d.d: invalid value {d:{}} (does not satisfy matchN): 0 matched, expected 1:
 -    ./cycle.cue:47:5
 -    ./cycle.cue:47:12
 -    ./cycle.cue:47:20
 -    ./cycle.cue:48:8
 -selfCycle.t1.c.d.d.d: structural cycle:
-+selfCycle.t1.c.d.d: structural cycle:
++selfCycle.t1.c.d: structural cycle:
      ./cycle.cue:47:5
  selfCycle.t2.c: invalid value {} (does not satisfy matchN): 0 matched, expected 1:
      ./cycle.cue:53:5
@@ -695,22 +680,34 @@ diff old new
  issue3443.cycle2.fail.#S: invalid value {n:{n:{n:_}}} (does not satisfy matchN): 0 matched, expected 1:
      ./matchn.cue:55:13
      ./matchn.cue:55:20
-@@ -55,13 +54,7 @@
-     ./matchn.cue:55:20
-     ./matchn.cue:55:28
-     ./matchn.cue:55:40
+-issue3443.cycle2.fail.#S.n: invalid value {n:{n:{n:_}}} (does not satisfy matchN): 0 matched, expected 1:
+-    ./matchn.cue:55:13
+-    ./matchn.cue:55:20
+-    ./matchn.cue:55:28
+-    ./matchn.cue:55:40
 -issue3443.cycle2.fail.#S.n.n: invalid value {n:{n:{n:_}}} (does not satisfy matchN): 0 matched, expected 1:
 -    ./matchn.cue:55:13
 -    ./matchn.cue:55:20
 -    ./matchn.cue:55:28
 -    ./matchn.cue:55:40
 -    ./matchn.cue:55:43
 -issue3443.cycle2.fail.#S.n.n.n: structural cycle:
-+issue3443.cycle2.fail.#S.n.n: structural cycle:
++issue3443.cycle2.fail.#S.n: structural cycle:
      ./matchn.cue:55:13
 
  Result:
-@@ -100,15 +93,15 @@
+@@ -78,10 +57,6 @@
+           //     ./cycle.cue:6:7
+           //     ./cycle.cue:6:14
+           //     ./cycle.cue:7:13
+-          // noCycle.t1._s.#x.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1:
+-          //     ./cycle.cue:6:7
+-          //     ./cycle.cue:6:14
+-          //     ./cycle.cue:7:13
+         }
+       }
+       #x: (_|_){
+@@ -100,15 +75,15 @@
      noCycle: (struct){
        t1: (struct){
          data: (#struct){
@@ -731,15 +728,15 @@ diff old new
                }
              }) }
          }
-@@ -115,7 +108,6 @@
+@@ -115,7 +90,6 @@
        }
        t2: (struct){
          x: (struct){
 -          b?: (string){ string }
            y: (struct){
              a: (struct){
                d: (struct){
-@@ -123,14 +115,15 @@
+@@ -123,14 +97,15 @@
                }
              }
            }
@@ -757,7 +754,7 @@ diff old new
                  }
                }) }
            }
-@@ -140,15 +133,15 @@
+@@ -140,15 +115,15 @@
      cycle: (struct){
        t1: (struct){
          data: (#struct){
@@ -778,7 +775,7 @@ diff old new
                }
              }) }
          }
-@@ -161,8 +154,8 @@
+@@ -161,8 +136,8 @@
        a?: ((string|bytes)){ "encoding/yaml".Validate(yamlNoCycle.#c) }
      }
      data: (#struct){
@@ -788,21 +785,26 @@ diff old new
      }
    }
    selfCycle: (_|_){
-@@ -179,12 +172,7 @@
+@@ -174,17 +149,7 @@
+         //     ./cycle.cue:47:5
          //     ./cycle.cue:47:12
-         //     ./cycle.cue:47:20
-         //     ./cycle.cue:48:8
+         //     ./cycle.cue:48:5
+-        // selfCycle.t1.c.d: invalid value {d:{}} (does not satisfy matchN): 0 matched, expected 1:
+-        //     ./cycle.cue:47:5
+-        //     ./cycle.cue:47:12
+-        //     ./cycle.cue:47:20
+-        //     ./cycle.cue:48:8
 -        // selfCycle.t1.c.d.d: invalid value {d:{}} (does not satisfy matchN): 0 matched, expected 1:
 -        //     ./cycle.cue:47:5
 -        //     ./cycle.cue:47:12
 -        //     ./cycle.cue:47:20
 -        //     ./cycle.cue:48:8
 -        // selfCycle.t1.c.d.d.d: structural cycle:
-+        // selfCycle.t1.c.d.d: structural cycle:
++        // selfCycle.t1.c.d: structural cycle:
          //     ./cycle.cue:47:5
          d: (struct){
          }
-@@ -197,15 +185,7 @@
+@@ -197,15 +162,7 @@
          //     ./cycle.cue:53:5
          //     ./cycle.cue:53:12
          //     ./cycle.cue:54:5
@@ -819,7 +821,7 @@ diff old new
          //     ./cycle.cue:53:5
        }
      }
-@@ -267,20 +247,20 @@
+@@ -267,20 +224,20 @@
    issue3410: (struct){
      _s: (struct){
        #x: (_){ matchN(1, (#list){
@@ -848,7 +850,7 @@ diff old new
          }
        }) }
    }
-@@ -287,9 +267,9 @@
+@@ -287,9 +244,9 @@
    issue3420: (struct){
      matches1: (struct){
        #S: (_){ matchN(1, (#list){
@@ -861,7 +863,7 @@ diff old new
            }
          }) }
        s: (int){ 2 }
-@@ -306,13 +286,13 @@
+@@ -306,13 +263,13 @@
      }
      noCycle: (struct){
        #S: (_){ matchN(1, (#list){
@@ -878,7 +880,7 @@ diff old new
            n: (struct){
              n: (_){ _ }
            }
-@@ -321,29 +301,43 @@
+@@ -321,29 +278,43 @@
      }
      noCycle2: (struct){
        #S: (_){ matchN(1, (#list){
@@ -935,22 +937,27 @@ diff old new
            n: (struct){
              n: (_){ _ }
            }
-@@ -363,13 +357,7 @@
+@@ -358,18 +329,7 @@
+           // [eval] issue3443.cycle2.fail.#S: invalid value {n:{n:{n:_}}} (does not satisfy matchN): 0 matched, expected 1:
+           //     ./matchn.cue:55:13
            //     ./matchn.cue:55:20
-           //     ./matchn.cue:55:28
-           //     ./matchn.cue:55:40
+-          // issue3443.cycle2.fail.#S.n: invalid value {n:{n:{n:_}}} (does not satisfy matchN): 0 matched, expected 1:
+-          //     ./matchn.cue:55:13
+-          //     ./matchn.cue:55:20
+-          //     ./matchn.cue:55:28
+-          //     ./matchn.cue:55:40
 -          // issue3443.cycle2.fail.#S.n.n: invalid value {n:{n:{n:_}}} (does not satisfy matchN): 0 matched, expected 1:
 -          //     ./matchn.cue:55:13
 -          //     ./matchn.cue:55:20
 -          //     ./matchn.cue:55:28
 -          //     ./matchn.cue:55:40
 -          //     ./matchn.cue:55:43
 -          // issue3443.cycle2.fail.#S.n.n.n: structural cycle:
-+          // issue3443.cycle2.fail.#S.n.n: structural cycle:
++          // issue3443.cycle2.fail.#S.n: structural cycle:
            //     ./matchn.cue:55:13
            n: (#struct){
              n: (#struct){
-@@ -396,11 +384,11 @@
+@@ -396,11 +356,11 @@
          //     ./matchn.cue:63:29
        }
        #s: (_){ matchN(1, (#list){

diff --git a/internal/core/adt/conjunct.go b/internal/core/adt/conjunct.go
@@ -574,8 +574,6 @@ var NoShareSentinel = &Bottom{
 }
 
 func (n *nodeContext) insertValueConjunct(env *Environment, v Value, id CloseInfo) {
-	n.updateCyclicStatusV3(id)
-
 	ctx := n.ctx
 
 	switch x := v.(type) {
@@ -599,6 +597,8 @@ func (n *nodeContext) insertValueConjunct(env *Environment, v Value, id CloseInf
 		}
 
 		if !x.IsData() {
+			n.updateCyclicStatusV3(id)
+
 			c := MakeConjunct(env, x, id)
 			n.scheduleVertexConjuncts(c, x, id)
 			return
@@ -656,6 +656,8 @@ func (n *nodeContext) insertValueConjunct(env *Environment, v Value, id CloseInf
 
 	switch x := v.(type) {
 	case *Disjunction:
+		n.updateCyclicStatusV3(id)
+
 		// TODO(perf): reuse envDisjunct values so that we can also reuse the
 		// disjunct slice.
 		id := id
@@ -687,13 +689,19 @@ func (n *nodeContext) insertValueConjunct(env *Environment, v Value, id CloseInf
 		}
 
 	case *Top:
+		n.updateCyclicStatusV3(id)
+
 		n.hasTop = true
 		id.cc.hasTop = true
 
 	case *BasicType:
+		n.updateCyclicStatusV3(id)
+
 		id.cc.hasNonTop = true
 
 	case *BoundValue:
+		n.updateCyclicStatusV3(id)
+
 		id.cc.hasNonTop = true
 		switch x.Op {
 		case LessThanOp, LessEqualOp:
@@ -784,6 +792,8 @@ func (n *nodeContext) insertValueConjunct(env *Environment, v Value, id CloseInf
 	// handled above.
 
 	case Value: // *NullLit, *BoolLit, *NumLit, *StringLit, *BytesLit, *Builtin
+		n.updateCyclicStatusV3(id)
+
 		if y := n.scalar; y != nil {
 			if b, ok := BinOp(ctx, EqualOp, x, y).(*Bool); !ok || !b.B {
 				n.reportConflict(x, y, x.Kind(), y.Kind(), n.scalarID, id)