internal/core/adt: fix sharing-related counter issue

In case of a structural cycle, the conjunct was neither
marked as shared, nor as non-shared. When then
later another reference is unified, this may stick as
a shared node.

As a result, the second conjunct is now recognized
as a "new" value, and thus the structural cycle is
allowed to descend one more level.

This brings V3 more in line with V2.

The added tests do not expose a bug per se, but
expose a counter error. As they do not change
anything except a counter, which are reduced here
anyway, they are not added in a separate test.

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: Ia73090d73fa292415aaccdcd6c0d49c3b0686948
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1207749
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Matthew Sackman <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>

Author: mpvl

cue/testdata/cycle/disjunction.txtar

@@ -213,9 +213,7 @@ Result:
     // [eval]
     #foo: (string){ =~"^foo" }
     #nesting: ((string|list|struct)){ |((string){ string }, (#struct){
-        name?: (_|_){
-          // [structural cycle] issue3042.#nesting.name: structural cycle
-        }
+        name?: (string){ =~"^foo" }
       }, (list){
       }) }
     data: (_|_){
@@ -258,7 +256,7 @@ diff old new
    cycle: (_|_){
      // [structural cycle]
      a: (_|_){
-@@ -58,22 +67,28 @@
+@@ -58,22 +67,26 @@
      x: (#list){
      }
    }
@@ -267,10 +265,7 @@ diff old new
 +    // [eval]
      #foo: (string){ =~"^foo" }
      #nesting: ((string|list|struct)){ |((string){ string }, (#struct){
--        name?: (string){ =~"^foo" }
-+        name?: (_|_){
-+          // [structural cycle] issue3042.#nesting.name: structural cycle
-+        }
+         name?: (string){ =~"^foo" }
        }, (list){
        }) }
 -    data: (#struct){

cue/testdata/cycle/issue990.txtar

@@ -80,14 +80,14 @@ out: #sub & {#p: _test.s1}
 -- diff/todo/p3 --
 Reordering
 -- out/evalalpha/stats --
-Leaks:  1182
+Leaks:  1184
 Freed:  81
 Reused: 81
-Allocs: 1182
+Allocs: 1184
 Retain: 0
 
 Unifications: 243
-Conjuncts:    4270
+Conjuncts:    4276
 Disjuncts:    204
 -- out/evalalpha --
 (struct){
@@ -328,17 +328,17 @@ diff old new
 -Reused: 3213
 -Allocs: 25
 -Retain: 26
-+Leaks:  1182
++Leaks:  1184
 +Freed:  81
 +Reused: 81
-+Allocs: 1182
++Allocs: 1184
 +Retain: 0
 
 -Unifications: 2588
 -Conjuncts:    12056
 -Disjuncts:    3258
 +Unifications: 243
-+Conjuncts:    4270
++Conjuncts:    4276
 +Disjuncts:    204
 -- diff/-out/evalalpha<==>+out/eval --
 diff old new

cue/testdata/cycle/structural.txtar

@@ -631,7 +631,7 @@ b7.b.0.0: conflicting values 1 and [a] (mismatched types int and list):
     ./in.cue:99:11
     ./in.cue:100:5
 b7.b.0.0.0: structural cycle
-c1.a.c: structural cycle
+c1.a.c.c: structural cycle
 d1.r: structural cycle
 d2.a.b.c.d.t: structural cycle
 d3.x.indirect: structural cycle
@@ -1237,7 +1237,12 @@ Result:
       b: (struct){
       }
       c: (_|_){
-        // [structural cycle] c1.a.c: structural cycle
+        // [structural cycle]
+        b: (struct){
+        }
+        c: (_|_){
+          // [structural cycle] c1.a.c.c: structural cycle
+        }
       }
     }
   }
@@ -1850,13 +1855,12 @@ diff old new
  b6.b.a.0.0: structural cycle
  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
 +b7.b.0.0: conflicting values 1 and [a] (mismatched types int and list):
 +    ./in.cue:99:11
 +    ./in.cue:100:5
 +b7.b.0.0.0: structural cycle
-+c1.a.c: structural cycle
+ c1.a.c.c: structural cycle
+-d1.a.b.c.d.t: structural cycle
 +d1.r: structural cycle
  d2.a.b.c.d.t: structural cycle
 -d2.r.c.d.t: structural cycle
@@ -2143,21 +2147,7 @@ diff old new
                }
              }
            }
-@@ -579,12 +627,7 @@
-       b: (struct){
-       }
-       c: (_|_){
--        // [structural cycle]
--        b: (struct){
--        }
--        c: (_|_){
--          // [structural cycle] c1.a.c.c: structural cycle
--        }
-+        // [structural cycle] c1.a.c: structural cycle
-       }
-     }
-   }
-@@ -600,47 +643,27 @@
+@@ -600,47 +648,27 @@
              // [structural cycle]
              h: (int){ int }
              t: (_|_){
@@ -2222,7 +2212,7 @@ diff old new
          c: (_|_){
            // [structural cycle]
            d: (_|_){
-@@ -667,28 +690,25 @@
+@@ -667,28 +695,25 @@
      }
      x: (_|_){
        // [structural cycle]
@@ -2256,7 +2246,7 @@ diff old new
        #List: (#struct){
          Next: (null){ null }
        }
-@@ -697,9 +717,7 @@
+@@ -697,9 +722,7 @@
        // [structural cycle]
        t1: (struct){
          #Foo: (#struct){
@@ -2267,7 +2257,7 @@ diff old new
          }
        }
        t2: (_|_){
-@@ -707,10 +725,7 @@
+@@ -707,10 +730,7 @@
          Foo: (_|_){
            // [structural cycle]
            ref: (_|_){
@@ -2279,7 +2269,7 @@ diff old new
            }
          }
        }
-@@ -717,9 +732,7 @@
+@@ -717,9 +737,7 @@
      }
      comprehension: (struct){
        #list: (#struct){
@@ -2290,7 +2280,7 @@ diff old new
        }
      }
    }
-@@ -745,8 +758,7 @@
+@@ -745,8 +763,7 @@
        }
      }
      let _schema_1#1 = (_|_){
@@ -2300,7 +2290,7 @@ diff old new
      }
    }
    fieldsSumInfinite: (_|_){
-@@ -757,7 +769,8 @@
+@@ -757,7 +774,8 @@
        fries: (float){ 2.00 }
        sprite: (float){ 1.00 }
        total: (_|_){
@@ -2310,7 +2300,7 @@ diff old new
        }
      }
    }
-@@ -772,27 +785,16 @@
+@@ -772,27 +790,16 @@
        head: (int){ 3 }
        tail: (struct){
          head: (int){ 2 }
@@ -2347,7 +2337,7 @@ diff old new
      }
    }
    e1: (_|_){
-@@ -831,11 +833,12 @@
+@@ -831,11 +838,12 @@
        // [eval] e3.a: conflicting values [a] and {c:a} (mismatched types list and struct):
        //     ./in.cue:412:5
        //     ./in.cue:413:5
@@ -2365,7 +2355,7 @@ diff old new
        }
      }
      b: (_|_){
-@@ -842,11 +845,12 @@
+@@ -842,11 +850,12 @@
        // [eval] e3.b: conflicting values [b] and {c:b} (mismatched types list and struct):
        //     ./in.cue:415:5
        //     ./in.cue:416:5
@@ -2383,7 +2373,7 @@ diff old new
        }
      }
    }
-@@ -855,38 +859,72 @@
+@@ -855,38 +864,72 @@
      a: (_|_){
        // [eval]
        0: (_|_){
@@ -2478,7 +2468,7 @@ diff old new
          0: (struct){
            c: (int){ 1 }
          }
-@@ -913,17 +951,16 @@
+@@ -913,17 +956,16 @@
          // [eval]
          0: (_|_){
            // [eval] nestedList.v1e.y.0: 4 errors in empty disjunction:
@@ -2501,7 +2491,7 @@ diff old new
            1: (int){ 1 }
          }
          1: (int){ 1 }
-@@ -935,17 +972,16 @@
+@@ -935,17 +977,16 @@
          // [eval]
          0: (_|_){
            // [eval] nestedList.v2e.y.0: 4 errors in empty disjunction:
@@ -2524,7 +2514,7 @@ diff old new
            1: (int){ 1 }
          }
          1: (int){ 1 }
-@@ -999,7 +1035,10 @@
+@@ -999,7 +1040,10 @@
          head: (int){ 3 }
          tail: (struct){
            head: (int){ 4 }
@@ -2536,7 +2526,7 @@ diff old new
          }
        }
      }
-@@ -1013,7 +1052,10 @@
+@@ -1013,7 +1057,10 @@
        head: (int){ 2 }
        tail: (struct){
          head: (int){ 3 }
@@ -2548,7 +2538,7 @@ diff old new
        }
      }
    }
-@@ -1027,8 +1069,12 @@
+@@ -1027,8 +1074,12 @@
        head: (int){ 2 }
        tail: (struct){ |((struct){
            head: (int){ 3 }
@@ -2563,7 +2553,7 @@ diff old new
          }, (struct){
            head: (int){ 3 }
          }) }
-@@ -1050,9 +1096,7 @@
+@@ -1050,9 +1101,7 @@
        // [structural cycle]
        f: (_|_){
          // [structural cycle]
@@ -2574,7 +2564,7 @@ diff old new
        }
        g: (_|_){
          // [structural cycle]
-@@ -1073,10 +1117,7 @@
+@@ -1073,10 +1122,7 @@
            x: (_){ _ }
            y: (_){ _ }
          }
@@ -2586,7 +2576,7 @@ diff old new
        }
      }
      t2: (struct){
-@@ -1089,10 +1130,7 @@
+@@ -1089,10 +1135,7 @@
            x: (_){ _ }
            y: (_){ _ }
          }
@@ -2598,7 +2588,7 @@ diff old new
        }
      }
      t3: (struct){
-@@ -1107,16 +1145,8 @@
+@@ -1107,16 +1150,8 @@
            y: (_){ _ }
            z: (_){ _ }
          }
@@ -2617,7 +2607,7 @@ diff old new
        }
      }
      t4: (struct){
-@@ -1132,51 +1162,11 @@
+@@ -1132,51 +1167,11 @@
              y: (_){ _ }
              z: (_){ _ }
            }
@@ -2674,7 +2664,7 @@ diff old new
        }
      }
      t5: (struct){
-@@ -1187,18 +1177,8 @@
+@@ -1187,18 +1182,8 @@
          }
        }
        C: (struct){
@@ -2695,7 +2685,7 @@ diff old new
        }
      }
    }
-@@ -1221,19 +1201,19 @@
+@@ -1221,19 +1206,19 @@
      }
    }
    n4: (struct){

cue/testdata/eval/sharing.txtar

@@ -121,6 +121,25 @@ issue3546: reduced: {
 	out: #Network
 	out: val
 }
+-- sharecycle.cue --
+shareCycle: t1: {
+	#X: {}
+	Y: {
+		x: #X & Y
+	}
+}
+shareCycle: t2: {
+	#X: int
+	Y: {
+		x: #X & Y
+	}
+}
+shareCycle: t3: {
+	#X: {}
+	Y: {
+		x: Y & #X
+	}
+}
 -- acrossdisjunction.cue --
 import "list"
 
@@ -133,16 +152,32 @@ issue3679: {
 }
 -- out/eval/stats --
 Leaks:  3
-Freed:  158
-Reused: 151
+Freed:  174
+Reused: 167
 Allocs: 10
 Retain: 21
 
-Unifications: 159
-Conjuncts:    331
-Disjuncts:    177
+Unifications: 175
+Conjuncts:    365
+Disjuncts:    193
 -- out/evalalpha --
-(struct){
+Errors:
+shareCycle.t2.Y.x: conflicting values int and {x:(#X & Y)} (mismatched types int and struct):
+    ./sharecycle.cue:8:6
+    ./sharecycle.cue:9:5
+shareCycle.t1.Y.x.x: field not allowed:
+    ./sharecycle.cue:4:11
+    ./sharecycle.cue:4:3
+shareCycle.t2.Y.x.x: field not allowed:
+    ./sharecycle.cue:10:11
+    ./sharecycle.cue:10:3
+shareCycle.t3.Y.x.x: field not allowed:
+    ./sharecycle.cue:16:10
+    ./sharecycle.cue:16:3
+
+Result:
+(_|_){
+  // [eval]
   issue3679: (struct){
     b: (#list){
       0: (string){ "bar" }
@@ -369,12 +404,87 @@ Disjuncts:    177
       }
     }
   }
+  shareCycle: (_|_){
+    // [eval]
+    t1: (_|_){
+      // [eval]
+      #X: (#struct){
+      }
+      Y: (_|_){
+        // [eval]
+        x: (_|_){
+          // [eval]
+          x: (_|_){
+            // [eval] shareCycle.t1.Y.x.x: field not allowed:
+            //     ./sharecycle.cue:4:11
+            //     ./sharecycle.cue:4:3
+          }
+        }
+      }
+    }
+    t2: (_|_){
+      // [eval]
+      #X: (int){ int }
+      Y: (_|_){
+        // [eval]
+        x: (_|_){
+          // [eval] shareCycle.t2.Y.x: conflicting values int and {x:(#X & Y)} (mismatched types int and struct):
+          //     ./sharecycle.cue:8:6
+          //     ./sharecycle.cue:9:5
+          // shareCycle.t2.Y.x.x: field not allowed:
+          //     ./sharecycle.cue:10:11
+          //     ./sharecycle.cue:10:3
+          x: (_|_){
+            // [eval] shareCycle.t2.Y.x.x: field not allowed:
+            //     ./sharecycle.cue:10:11
+            //     ./sharecycle.cue:10:3
+          }
+        }
+      }
+    }
+    t3: (_|_){
+      // [eval]
+      #X: (#struct){
+      }
+      Y: (_|_){
+        // [eval]
+        x: (_|_){
+          // [eval]
+          x: (_|_){
+            // [eval] shareCycle.t3.Y.x.x: field not allowed:
+            //     ./sharecycle.cue:16:10
+            //     ./sharecycle.cue:16:3
+          }
+        }
+      }
+    }
+  }
 }
 -- diff/-out/evalalpha<==>+out/eval --
 diff old new
 --- old
 +++ new
-@@ -16,102 +16,75 @@
+@@ -1,11 +1,16 @@
+ Errors:
+-shareCycle.t1.Y.x.x: structural cycle
+ shareCycle.t2.Y.x: conflicting values int and {x:(#X & Y)} (mismatched types int and struct):
+     ./sharecycle.cue:8:6
+     ./sharecycle.cue:9:5
++shareCycle.t1.Y.x.x: field not allowed:
++    ./sharecycle.cue:4:11
++    ./sharecycle.cue:4:3
++shareCycle.t2.Y.x.x: field not allowed:
+     ./sharecycle.cue:10:11
+-shareCycle.t2.Y.x.x: structural cycle
+-shareCycle.t3.Y.x.x: structural cycle
++    ./sharecycle.cue:10:3
++shareCycle.t3.Y.x.x: field not allowed:
++    ./sharecycle.cue:16:10
++    ./sharecycle.cue:16:3
+ 
+ Result:
+ (_|_){
+@@ -27,102 +32,75 @@
            }
          }
          #Config1: (#struct){
@@ -546,7 +656,7 @@ diff old new
          }
          Config: (#struct){
            cfg: (#struct){
-@@ -137,24 +110,13 @@
+@@ -148,24 +126,13 @@
          }
        }
        #Config1: (#struct){
@@ -574,7 +684,7 @@ diff old new
        }
        #Config: (#struct){
          cfg: (#struct){
-@@ -162,12 +124,7 @@
+@@ -173,12 +140,7 @@
            }
          }
        }
@@ -588,7 +698,7 @@ diff old new
        out: (#struct){
          sch: (#struct){
            cfg: (#struct){
-@@ -236,9 +193,7 @@
+@@ -247,9 +209,7 @@
    }
    debug: (struct){
      sharingOn: (struct){
@@ -599,8 +709,87 @@ diff old new
        b: (struct){
          c: (int){ 1 }
        }
+@@ -284,15 +244,17 @@
+   shareCycle: (_|_){
+     // [eval]
+     t1: (_|_){
+-      // [structural cycle]
+-      #X: (#struct){
+-      }
+-      Y: (_|_){
+-        // [structural cycle]
+-        x: (_|_){
+-          // [structural cycle]
+-          x: (_|_){
+-            // [structural cycle] shareCycle.t1.Y.x.x: structural cycle
++      // [eval]
++      #X: (#struct){
++      }
++      Y: (_|_){
++        // [eval]
++        x: (_|_){
++          // [eval]
++          x: (_|_){
++            // [eval] shareCycle.t1.Y.x.x: field not allowed:
++            //     ./sharecycle.cue:4:11
++            //     ./sharecycle.cue:4:3
+           }
+         }
+       }
+@@ -306,23 +268,29 @@
+           // [eval] shareCycle.t2.Y.x: conflicting values int and {x:(#X & Y)} (mismatched types int and struct):
+           //     ./sharecycle.cue:8:6
+           //     ./sharecycle.cue:9:5
++          // shareCycle.t2.Y.x.x: field not allowed:
+           //     ./sharecycle.cue:10:11
+-          x: (_|_){
+-            // [structural cycle] shareCycle.t2.Y.x.x: structural cycle
++          //     ./sharecycle.cue:10:3
++          x: (_|_){
++            // [eval] shareCycle.t2.Y.x.x: field not allowed:
++            //     ./sharecycle.cue:10:11
++            //     ./sharecycle.cue:10:3
+           }
+         }
+       }
+     }
+     t3: (_|_){
+-      // [structural cycle]
+-      #X: (#struct){
+-      }
+-      Y: (_|_){
+-        // [structural cycle]
+-        x: (_|_){
+-          // [structural cycle]
+-          x: (_|_){
+-            // [structural cycle] shareCycle.t3.Y.x.x: structural cycle
++      // [eval]
++      #X: (#struct){
++      }
++      Y: (_|_){
++        // [eval]
++        x: (_|_){
++          // [eval]
++          x: (_|_){
++            // [eval] shareCycle.t3.Y.x.x: field not allowed:
++            //     ./sharecycle.cue:16:10
++            //     ./sharecycle.cue:16:3
+           }
+         }
+       }
 -- out/eval --
-(struct){
+Errors:
+shareCycle.t1.Y.x.x: structural cycle
+shareCycle.t2.Y.x: conflicting values int and {x:(#X & Y)} (mismatched types int and struct):
+    ./sharecycle.cue:8:6
+    ./sharecycle.cue:9:5
+    ./sharecycle.cue:10:11
+shareCycle.t2.Y.x.x: structural cycle
+shareCycle.t3.Y.x.x: structural cycle
+
+Result:
+(_|_){
+  // [eval]
   issue3679: (struct){
     b: (#list){
       0: (string){ "bar" }
@@ -872,6 +1061,53 @@ diff old new
       }
     }
   }
+  shareCycle: (_|_){
+    // [eval]
+    t1: (_|_){
+      // [structural cycle]
+      #X: (#struct){
+      }
+      Y: (_|_){
+        // [structural cycle]
+        x: (_|_){
+          // [structural cycle]
+          x: (_|_){
+            // [structural cycle] shareCycle.t1.Y.x.x: structural cycle
+          }
+        }
+      }
+    }
+    t2: (_|_){
+      // [eval]
+      #X: (int){ int }
+      Y: (_|_){
+        // [eval]
+        x: (_|_){
+          // [eval] shareCycle.t2.Y.x: conflicting values int and {x:(#X & Y)} (mismatched types int and struct):
+          //     ./sharecycle.cue:8:6
+          //     ./sharecycle.cue:9:5
+          //     ./sharecycle.cue:10:11
+          x: (_|_){
+            // [structural cycle] shareCycle.t2.Y.x.x: structural cycle
+          }
+        }
+      }
+    }
+    t3: (_|_){
+      // [structural cycle]
+      #X: (#struct){
+      }
+      Y: (_|_){
+        // [structural cycle]
+        x: (_|_){
+          // [structural cycle]
+          x: (_|_){
+            // [structural cycle] shareCycle.t3.Y.x.x: structural cycle
+          }
+        }
+      }
+    }
+  }
 }
 -- out/compile --
 --- acrossdisjunction.cue
@@ -1071,3 +1307,30 @@ diff old new
     }
   }
 }
+--- sharecycle.cue
+{
+  shareCycle: {
+    t1: {
+      #X: {}
+      Y: {
+        x: (〈1;#X〉 & 〈1;Y〉)
+      }
+    }
+  }
+  shareCycle: {
+    t2: {
+      #X: int
+      Y: {
+        x: (〈1;#X〉 & 〈1;Y〉)
+      }
+    }
+  }
+  shareCycle: {
+    t3: {
+      #X: {}
+      Y: {
+        x: (〈1;Y〉 & 〈1;#X〉)
+      }
+    }
+  }
+}

internal/core/adt/eval_test.go

@@ -76,8 +76,6 @@ var needFix = map[string]string{
 // counter errors.
 // TODO: These counters should all go to zero.
 var skipDebugDepErrors = map[string]int{
-	"cycle/disjunction":        1,
-	"cycle/structural":         1,
 	"disjunctions/elimination": 4,
 	"eval/issue545":            1,
 	"eval/notify":              3,

internal/core/adt/tasks.go

@@ -119,6 +119,9 @@ func processResolver(ctx *OpContext, t *task, mode runMode) {
 	// we are done computing and we can return the arc as is.
 	ci, skip := t.node.detectCycleV3(d, t.env, r, ci)
 	if skip {
+		// Either we have a structure cycle or we are unifying with another
+		// conjunct. In either case, we are no longer structure sharing here.
+		t.node.unshare()
 		return
 	}