internal/core/adt: remove further evaluatingArcs logic

The logic is now only used for detecting "mutual dependencies". This construct resulted in spurious cycle detections, which caused builtins to remain unevaluated. To make this work, we now copy the "postChecks logic of the old evaluator. This harmonizes error messages in circular dependencies in comprehensions. Note that before we only conversed over each arc of a node once (using arcPos). However, this prevents us from detecting structural cycles if a node was shared now that the evaluatingArcs is moreved. We therefore removed this logic. We still use an index, instead of range in case an arc is added later. Fixes various tests, including one counter test. Previously, when comparing to bottom, Finalize might be called recursively. This caused a node to be finalized while comprehensions were running, resulting in a mismatch of counters. Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I907e528894abd154eef685199cf5681230a222c8 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202837 Reviewed-by: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
cue-lang · Oct 22, 2024 · 8f42ad0 · 8f42ad0
1 parent 9b07aba
commit 8f42ad0
Show file tree

Hide file tree

Showing 9 changed files with 168 additions and 503 deletions.
diff --git a/cue/testdata/cycle/chain.txtar b/cue/testdata/cycle/chain.txtar
@@ -208,15 +208,15 @@ issue2052: full: {
 	d: #Depth & {#in: tree}
 }
 -- out/evalalpha/stats --
-Leaks:  12701
-Freed:  1308
-Reused: 1307
-Allocs: 12702
+Leaks:  12801
+Freed:  1484
+Reused: 1483
+Allocs: 12802
 Retain: 0
 
-Unifications: 5010
-Conjuncts:    66031
-Disjuncts:    8699
+Unifications: 5066
+Conjuncts:    67167
+Disjuncts:    8919
 -- out/evalalpha --
 Errors:
 issue2052.t1.#Depth: adding field #basic not allowed as field set was already referenced:
@@ -729,18 +729,18 @@ diff old new
 -Reused: 1816
 -Allocs: 70
 -Retain: 169
-+Leaks:  12701
-+Freed:  1308
-+Reused: 1307
-+Allocs: 12702
++Leaks:  12801
++Freed:  1484
++Reused: 1483
++Allocs: 12802
 +Retain: 0
 
 -Unifications: 801
 -Conjuncts:    3177
 -Disjuncts:    1979
-+Unifications: 5010
-+Conjuncts:    66031
-+Disjuncts:    8699
++Unifications: 5066
++Conjuncts:    67167
++Disjuncts:    8919
 -- diff/-out/evalalpha<==>+out/eval --
 diff old new
 --- old

diff --git a/cue/testdata/cycle/compbottom2.txtar b/cue/testdata/cycle/compbottom2.txtar
@@ -469,37 +469,23 @@ Disjuncts:    204
         }
       }
       t2: (struct){
-        p1: (struct){
-          raises: (_|_){
-            // [cycle] sameStruct.cycleFail.t2.p1.raises: mutual dependency
-          }
-          ret: (struct){
-          }
+        p1: (_|_){
+          // [cycle] sameStruct.cycleFail.t2.p1: circular dependency in evaluation of conditionals: raises changed after evaluation:
+          //     ./mutualsamestruct.cue:54:6
         }
-        p2: (struct){
-          ret: (_|_){
-            // [cycle] sameStruct.cycleFail.t2.p2.ret: mutual dependency
-          }
-          raises: (struct){
-            a: (int){ 1 }
-          }
+        p2: (_|_){
+          // [cycle] sameStruct.cycleFail.t2.p2: circular dependency in evaluation of conditionals: ret changed after evaluation:
+          //     ./mutualsamestruct.cue:69:6
         }
       }
       t3: (struct){
-        p1: (struct){
-          raises: (_|_){
-            // [cycle] sameStruct.cycleFail.t3.p1.raises: mutual dependency
-          }
-          ret: (struct){
-          }
+        p1: (_|_){
+          // [cycle] sameStruct.cycleFail.t3.p1: circular dependency in evaluation of conditionals: raises changed after evaluation:
+          //     ./mutualsamestruct.cue:83:6
         }
-        p2: (struct){
-          ret: (_|_){
-            // [cycle] sameStruct.cycleFail.t3.p2.ret: mutual dependency
-          }
-          raises: (struct){
-            a: (int){ 1 }
-          }
+        p2: (_|_){
+          // [cycle] sameStruct.cycleFail.t3.p2: circular dependency in evaluation of conditionals: ret changed after evaluation:
+          //     ./mutualsamestruct.cue:94:6
         }
       }
     }
@@ -613,7 +599,7 @@ diff old new
        }
      }
      brokenCycleSuccess: (struct){
-@@ -151,33 +153,60 @@
+@@ -151,13 +153,26 @@
      }
      cycleFail: (struct){
        t1: (struct){
@@ -624,16 +610,6 @@ diff old new
 -        p2: (_|_){
 -          // [cycle] sameStruct.cycleFail.t1.p2: circular dependency in evaluation of conditionals: ret changed after evaluation:
 -          //     ./mutualsamestruct.cue:37:6
--        }
--      }
--      t2: (struct){
--        p1: (_|_){
--          // [cycle] sameStruct.cycleFail.t2.p1: circular dependency in evaluation of conditionals: raises changed after evaluation:
--          //     ./mutualsamestruct.cue:54:6
--        }
--        p2: (_|_){
--          // [cycle] sameStruct.cycleFail.t2.p2: circular dependency in evaluation of conditionals: ret changed after evaluation:
--          //     ./mutualsamestruct.cue:69:6
 +        p1: (struct){
 +          raises?: (struct){
 +            a: (_|_){
@@ -653,52 +629,11 @@ diff old new
 +            }
 +          }
 +          raises?: (struct){
-+          }
-+        }
-+      }
-+      t2: (struct){
-+        p1: (struct){
-+          raises: (_|_){
-+            // [cycle] sameStruct.cycleFail.t2.p1.raises: mutual dependency
-+          }
-+          ret: (struct){
-+          }
-+        }
-+        p2: (struct){
-+          ret: (_|_){
-+            // [cycle] sameStruct.cycleFail.t2.p2.ret: mutual dependency
-+          }
-+          raises: (struct){
-+            a: (int){ 1 }
 +          }
          }
        }
-       t3: (struct){
--        p1: (_|_){
--          // [cycle] sameStruct.cycleFail.t3.p1: circular dependency in evaluation of conditionals: raises changed after evaluation:
--          //     ./mutualsamestruct.cue:83:6
--        }
--        p2: (_|_){
--          // [cycle] sameStruct.cycleFail.t3.p2: circular dependency in evaluation of conditionals: ret changed after evaluation:
--          //     ./mutualsamestruct.cue:94:6
-+        p1: (struct){
-+          raises: (_|_){
-+            // [cycle] sameStruct.cycleFail.t3.p1.raises: mutual dependency
-+          }
-+          ret: (struct){
-+          }
-+        }
-+        p2: (struct){
-+          ret: (_|_){
-+            // [cycle] sameStruct.cycleFail.t3.p2.ret: mutual dependency
-+          }
-+          raises: (struct){
-+            a: (int){ 1 }
-+          }
-         }
-       }
-     }
-@@ -189,17 +218,19 @@
+       t2: (struct){
+@@ -189,17 +204,19 @@
          ret?: (_){ _ }
        }
        expr: (#struct){
@@ -723,7 +658,7 @@ diff old new
      }
      brokenCycleSuccess: (struct){
        #E: (#struct){
-@@ -208,8 +239,12 @@
+@@ -208,8 +225,12 @@
        }
      }
      doubleAddfail: (_|_){
@@ -742,7 +677,6 @@ diff old new
 self.isConcreteFail: t1: int value is not an error.
 self.isNotConcrete: t1: int value is not an error.
 -- diff/todo/p3 --
-sameStruct.cycleFail: Harmonize and improve cycle errors
 cycleFail.t1.p1.raises?.a: error needs to be reported at parent node.
 -- out/eval --
 (struct){