internal/core/adt: prevent recursion in calls

Expressions that contain a call may end up in an infinite recursion if we do not ensure that there is non-cyclic data to propagate the evaluation. This fixes some tests, but we found it also makes it more robust against infinite recursion when developing alternative evaluation strategies. Issue #3649 Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: I7d6fecb886cf50f34589607dae606de1fea5ce93 Reviewed-on: https://cue.gerrithub.io/c/cue-lang/cue/+/1208008 Unity-Result: CUE porcuepine <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Reviewed-by: Matthew Sackman <[email protected]>
cue-lang · Jan 30, 2025 · 16e3250 · 16e3250
1 parent 5b4a02e
commit 16e3250
Show file tree

Hide file tree

Showing 3 changed files with 122 additions and 42 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:  314
+Leaks:  302
 Freed:  17
 Reused: 17
-Allocs: 314
+Allocs: 302
 Retain: 0
 
-Unifications: 272
-Conjuncts:    1059
+Unifications: 262
+Conjuncts:    1037
 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:  314
++Leaks:  302
 +Freed:  17
 +Reused: 17
-+Allocs: 314
++Allocs: 302
 +Retain: 0
 
 -Unifications: 363
 -Conjuncts:    645
 -Disjuncts:    454
-+Unifications: 272
-+Conjuncts:    1059
++Unifications: 262
++Conjuncts:    1037
 +Disjuncts:    28
 -- out/eval/stats --
 Leaks:  22
@@ -256,14 +256,18 @@ noCycle.t1.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1
     ./cycle.cue:6:7
     ./cycle.cue:6:14
     ./cycle.cue:7:13
-noCycle.t1.#x.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1:
+noCycle.t1.#x.#x: structural cycle:
     ./cycle.cue:6:7
-    ./cycle.cue:6:14
-    ./cycle.cue:7:13
 noCycle.t1._s.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1:
     ./cycle.cue:6:7
     ./cycle.cue:6:14
     ./cycle.cue:7:13
+issue3649.cycle.t1.data.a: invalid value {b:"foo"} (does not satisfy matchN): 0 matched, expected 1:
+    ./cycle.cue:33:6
+    ./cycle.cue:30:11
+    ./cycle.cue:33:13
+issue3649.cycle.t1.data.a.a: structural cycle:
+    ./cycle.cue:33:6
 selfCycle.t1.c: invalid value {d:{}} (does not satisfy matchN): 0 matched, expected 1:
     ./cycle.cue:47:5
     ./cycle.cue:47:12
@@ -317,14 +321,13 @@ Result:
         //     ./cycle.cue:6:7
         //     ./cycle.cue:6:14
         //     ./cycle.cue:7:13
-        // noCycle.t1.#x.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1:
+        // noCycle.t1.#x.#x: structural cycle:
         //     ./cycle.cue:6:7
-        //     ./cycle.cue:6:14
-        //     ./cycle.cue:7:13
       }
     }
   }
-  issue3649: (struct){
+  issue3649: (_|_){
+    // [eval]
     noCycle: (struct){
       t1: (struct){
         data: (#struct){
@@ -365,10 +368,19 @@ Result:
         }
       }
     }
-    cycle: (struct){
-      t1: (struct){
-        data: (#struct){
-          a: (#struct){
+    cycle: (_|_){
+      // [eval]
+      t1: (_|_){
+        // [eval]
+        data: (_|_){
+          // [eval]
+          a: (_|_){
+            // [eval] issue3649.cycle.t1.data.a: invalid value {b:"foo"} (does not satisfy matchN): 0 matched, expected 1:
+            //     ./cycle.cue:33:6
+            //     ./cycle.cue:30:11
+            //     ./cycle.cue:33:13
+            // issue3649.cycle.t1.data.a.a: structural cycle:
+            //     ./cycle.cue:33:6
             b: (string){ "foo" }
           }
           b: (string){ string }
@@ -623,14 +635,30 @@ Result:
 diff old new
 --- old
 +++ new
-@@ -11,57 +11,36 @@
+@@ -3,65 +3,48 @@
+     ./cycle.cue:6:7
+     ./cycle.cue:6:14
+     ./cycle.cue:7:13
+-noCycle.t1.#x.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1:
+-    ./cycle.cue:6:7
+-    ./cycle.cue:6:14
+-    ./cycle.cue:7:13
++noCycle.t1.#x.#x: structural cycle:
++    ./cycle.cue:6:7
+ noCycle.t1._s.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1:
      ./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
++issue3649.cycle.t1.data.a: invalid value {b:"foo"} (does not satisfy matchN): 0 matched, expected 1:
++    ./cycle.cue:33:6
++    ./cycle.cue:30:11
++    ./cycle.cue:33:13
++issue3649.cycle.t1.data.a.a: structural cycle:
++    ./cycle.cue:33:6
  selfCycle.t1.c: invalid value {d:{}} (does not satisfy matchN): 0 matched, expected 1:
      ./cycle.cue:47:5
      ./cycle.cue:47:12
@@ -696,7 +724,7 @@ diff old new
      ./matchn.cue:55:13
 
  Result:
-@@ -78,10 +57,6 @@
+@@ -78,10 +61,6 @@
            //     ./cycle.cue:6:7
            //     ./cycle.cue:6:14
            //     ./cycle.cue:7:13
@@ -707,14 +735,35 @@ diff old new
          }
        }
        #x: (_|_){
-@@ -100,15 +75,15 @@
-     noCycle: (struct){
-       t1: (struct){
-         data: (#struct){
+@@ -89,26 +68,25 @@
+         //     ./cycle.cue:6:7
+         //     ./cycle.cue:6:14
+         //     ./cycle.cue:7:13
+-        // noCycle.t1.#x.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1:
+-        //     ./cycle.cue:6:7
+-        //     ./cycle.cue:6:14
+-        //     ./cycle.cue:7:13
+-      }
+-    }
+-  }
+-  issue3649: (struct){
+-    noCycle: (struct){
+-      t1: (struct){
+-        data: (#struct){
 -          b?: (string){ string }
 -          a: (struct){
 -            b: (string){ "foo" }
 -          }
++        // noCycle.t1.#x.#x: structural cycle:
++        //     ./cycle.cue:6:7
++      }
++    }
++  }
++  issue3649: (_|_){
++    // [eval]
++    noCycle: (struct){
++      t1: (struct){
++        data: (#struct){
 +          a: (#struct){
 +            b: (string){ "foo" }
 +          }
@@ -728,15 +777,15 @@ diff old new
                }
              }) }
          }
-@@ -115,7 +90,6 @@
+@@ -115,7 +93,6 @@
        }
        t2: (struct){
          x: (struct){
 -          b?: (string){ string }
            y: (struct){
              a: (struct){
                d: (struct){
-@@ -123,14 +97,15 @@
+@@ -123,14 +100,15 @@
                }
              }
            }
@@ -754,15 +803,30 @@ diff old new
                  }
                }) }
            }
-@@ -140,15 +115,15 @@
-     cycle: (struct){
-       t1: (struct){
-         data: (#struct){
+@@ -137,18 +115,27 @@
+         }
+       }
+     }
+-    cycle: (struct){
+-      t1: (struct){
+-        data: (#struct){
 -          b: (string){ string }
 -          a: (struct){
 -            b: (string){ "foo" }
 -          }
-+          a: (#struct){
++    cycle: (_|_){
++      // [eval]
++      t1: (_|_){
++        // [eval]
++        data: (_|_){
++          // [eval]
++          a: (_|_){
++            // [eval] issue3649.cycle.t1.data.a: invalid value {b:"foo"} (does not satisfy matchN): 0 matched, expected 1:
++            //     ./cycle.cue:33:6
++            //     ./cycle.cue:30:11
++            //     ./cycle.cue:33:13
++            // issue3649.cycle.t1.data.a.a: structural cycle:
++            //     ./cycle.cue:33:6
 +            b: (string){ "foo" }
 +          }
 +          b: (string){ string }
@@ -775,7 +839,7 @@ diff old new
                }
              }) }
          }
-@@ -161,8 +136,8 @@
+@@ -161,8 +148,8 @@
        a?: ((string|bytes)){ "encoding/yaml".Validate(yamlNoCycle.#c) }
      }
      data: (#struct){
@@ -785,7 +849,7 @@ diff old new
      }
    }
    selfCycle: (_|_){
-@@ -174,17 +149,7 @@
+@@ -174,17 +161,7 @@
          //     ./cycle.cue:47:5
          //     ./cycle.cue:47:12
          //     ./cycle.cue:48:5
@@ -804,7 +868,7 @@ diff old new
          //     ./cycle.cue:47:5
          d: (struct){
          }
-@@ -197,15 +162,7 @@
+@@ -197,15 +174,7 @@
          //     ./cycle.cue:53:5
          //     ./cycle.cue:53:12
          //     ./cycle.cue:54:5
@@ -821,7 +885,7 @@ diff old new
          //     ./cycle.cue:53:5
        }
      }
-@@ -267,20 +224,20 @@
+@@ -267,20 +236,20 @@
    issue3410: (struct){
      _s: (struct){
        #x: (_){ matchN(1, (#list){
@@ -850,7 +914,7 @@ diff old new
          }
        }) }
    }
-@@ -287,9 +244,9 @@
+@@ -287,9 +256,9 @@
    issue3420: (struct){
      matches1: (struct){
        #S: (_){ matchN(1, (#list){
@@ -863,7 +927,7 @@ diff old new
            }
          }) }
        s: (int){ 2 }
-@@ -306,13 +263,13 @@
+@@ -306,13 +275,13 @@
      }
      noCycle: (struct){
        #S: (_){ matchN(1, (#list){
@@ -880,7 +944,7 @@ diff old new
            n: (struct){
              n: (_){ _ }
            }
-@@ -321,29 +278,43 @@
+@@ -321,29 +290,43 @@
      }
      noCycle2: (struct){
        #S: (_){ matchN(1, (#list){
@@ -937,7 +1001,7 @@ diff old new
            n: (struct){
              n: (_){ _ }
            }
-@@ -358,18 +329,7 @@
+@@ -358,18 +341,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
@@ -957,7 +1021,7 @@ diff old new
            //     ./matchn.cue:55:13
            n: (#struct){
              n: (#struct){
-@@ -396,11 +356,11 @@
+@@ -396,11 +368,11 @@
          //     ./matchn.cue:63:29
        }
        #s: (_){ matchN(1, (#list){

diff --git a/internal/core/adt/conjunct.go b/internal/core/adt/conjunct.go
@@ -228,6 +228,18 @@ func (n *nodeContext) scheduleConjunct(c Conjunct, id CloseInfo) {
 
 	case Evaluator:
 		n.unshare()
+
+		// Expressions that contain a call may end up in an infinite recursion
+		// here if we do not ensure that there is non-cyclic data to propagate
+		// the evaluation. We therefore postpone expressions until we have
+		// evidence that such non-cyclic conjuncts exist.
+		if id.CycleType == IsCyclic && !n.hasNonCycle && !n.hasNonCyclic {
+			n.hasAncestorCycle = true
+			n.cyclicConjuncts = append(n.cyclicConjuncts, cyclicConjunct{c: c})
+			n.node.cc().incDependent(n.ctx, DEFER, nil)
+			return
+		}
+
 		// Interpolation, UnaryExpr, CallExpr
 		n.scheduleTask(handleExpr, env, x, id)
 

diff --git a/internal/core/adt/cycle.go b/internal/core/adt/cycle.go
@@ -1103,7 +1103,11 @@ func (n *nodeContext) updateCyclicStatusV3(c CloseInfo) {
 		for _, c := range n.cyclicConjuncts {
 			ci := c.c.CloseInfo
 			ci.cc = n.node.rootCloseContext(n.ctx)
-			n.scheduleVertexConjuncts(c.c, c.arc, ci)
+			if c.arc != nil {
+				n.scheduleVertexConjuncts(c.c, c.arc, ci)
+			} else {
+				n.scheduleConjunct(c.c, ci)
+			}
 			n.node.cc().decDependent(n.ctx, DEFER, nil)
 		}
 		n.cyclicConjuncts = n.cyclicConjuncts[:0]