internal/core/adt: avoid finalizing too early

unify pro-actively evaluates pending arcs. This
pro-active finalization could lead to tasks being
forcibly unblocked too early, leaving values to
function as "undefined".

We now only "require" pending arcs (yield) and
do not finalize. Note that if this results in blocks, this
will have the same effect as finalization.

This does uncover a situation where a value may
"yield" oustside of an underlying tasks. In this case,
we can simply "fall back" to finalization.

Added "related" issue, to show that this change does
not break that test.

Fixes #3919

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: Ifb6f6374edd9a5a93a43c83bfa002b3b4ee703fc
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1214950
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>

Author: mpvl

cue/testdata/eval/comprehensions.txtar

@@ -271,8 +271,9 @@ Result:
             // [eval] issue3691.structuralCycle.a.b.c: conflicting values string and {[string]:X} (mismatched types string and struct):
             //     ./issue3691.cue:14:15
             //     ./issue3691.cue:15:5
-            b: (_|_){
-              // [cycle] cycle error
+            b*: (_|_){// [e]if true {
+              //   c: 〈3;a〉
+              // }[e] & 〈1;X〉
             }
           }
         }
@@ -307,18 +308,20 @@ diff old new
              //     ./issue3691.cue:8:10
            }
          }
-@@ -145,13 +140,8 @@
+@@ -145,13 +140,9 @@
              // [eval] issue3691.structuralCycle.a.b.c: conflicting values string and {[string]:X} (mismatched types string and struct):
              //     ./issue3691.cue:14:15
              //     ./issue3691.cue:15:5
 -            //     ./issue3691.cue:17:3
 -            //     ./issue3691.cue:18:10
-             b: (_|_){
+-            b: (_|_){
 -              // [structural cycle]
 -              c: (_|_){
 -                // [structural cycle] issue3691.structuralCycle.a.b.c.b.c: structural cycle
 -              }
-+              // [cycle] cycle error
++            b*: (_|_){// [e]if true {
++              //   c: 〈3;a〉
++              // }[e] & 〈1;X〉
              }
            }
          }

cue/testdata/eval/issue3919.txtar

@@ -19,63 +19,114 @@ reduced: {
 	}
 	port: int | *30080
 }
+-- secondary.cue --
+related: {
+	[string]: {
+		if true {
+			shared: all_total: foo.fooData.total
+		}
+	}
+	foo: fooData: {
+		total: tags: _hidden
+		_hidden: extra: {}
+	}
+	#Tags: [string]: {}
+	bar: {
+		shared: {
+			[string]: tags: #Tags
+			bar_total: foo.fooData.total
+		}
+	}
+}
 -- out/eval/stats --
-Leaks:  0
-Freed:  13
-Reused: 5
+Leaks:  2
+Freed:  32
+Reused: 26
 Allocs: 8
-Retain: 0
+Retain: 1
 
-Unifications: 9
-Conjuncts:    13
-Disjuncts:    13
+Unifications: 30
+Conjuncts:    51
+Disjuncts:    35
 -- out/evalalpha --
 (struct){
   full: (struct){
     out: (struct){
       env: (struct){
-        PORT: (_|_){
-          // [incomplete] full.out.env.PORT: error in call to math.Abs: non-concrete value _:
-          //     ./in.cue:7:11
-        }
+        PORT: (int){ 30080 }
       }
       port: (int){ |(*(int){ 30080 }, (int){ int }) }
     }
   }
   reduced: (struct){
-    a: (_|_){
-      // [incomplete] reduced.a: error in call to math.Abs: non-concrete value _:
-      //     ./in.cue:17:6
-    }
+    a: (int){ 30080 }
     port: (int){ |(*(int){ 30080 }, (int){ int }) }
   }
+  related: (struct){
+    foo: (struct){
+      fooData: (struct){
+        total: (struct){
+          tags: ~(related.foo.fooData._hidden)
+        }
+        _hidden: (struct){
+          extra: (struct){
+          }
+        }
+      }
+      shared: (struct){
+        all_total: ~(related.foo.fooData.total)
+      }
+    }
+    #Tags: (#struct){
+    }
+    bar: (struct){
+      shared: (struct){
+        bar_total: (struct){
+          tags: (#struct){
+            extra: (#struct){
+            }
+          }
+        }
+        all_total: (struct){
+          tags: (#struct){
+            extra: (#struct){
+            }
+          }
+        }
+      }
+    }
+  }
 }
 -- diff/-out/evalalpha<==>+out/eval --
 diff old new
 --- old
 +++ new
-@@ -2,13 +2,19 @@
-   full: (struct){
-     out: (struct){
-       env: (struct){
--        PORT: (int){ 30080 }
-+        PORT: (_|_){
-+          // [incomplete] full.out.env.PORT: error in call to math.Abs: non-concrete value _:
-+          //     ./in.cue:7:11
-+        }
+@@ -15,10 +15,7 @@
+     foo: (struct){
+       fooData: (struct){
+         total: (struct){
+-          tags: (struct){
+-            extra: (struct){
+-            }
+-          }
++          tags: ~(related.foo.fooData._hidden)
+         }
+         _hidden: (struct){
+           extra: (struct){
+@@ -26,12 +23,7 @@
+         }
+       }
+       shared: (struct){
+-        all_total: (struct){
+-          tags: (struct){
+-            extra: (struct){
+-            }
+-          }
+-        }
++        all_total: ~(related.foo.fooData.total)
        }
-       port: (int){ |(*(int){ 30080 }, (int){ int }) }
      }
-   }
-   reduced: (struct){
--    a: (int){ 30080 }
-+    a: (_|_){
-+      // [incomplete] reduced.a: error in call to math.Abs: non-concrete value _:
-+      //     ./in.cue:17:6
-+    }
-     port: (int){ |(*(int){ 30080 }, (int){ int }) }
-   }
- }
+     #Tags: (#struct){
 -- out/eval --
 (struct){
   full: (struct){
@@ -90,6 +141,48 @@ diff old new
     a: (int){ 30080 }
     port: (int){ |(*(int){ 30080 }, (int){ int }) }
   }
+  related: (struct){
+    foo: (struct){
+      fooData: (struct){
+        total: (struct){
+          tags: (struct){
+            extra: (struct){
+            }
+          }
+        }
+        _hidden: (struct){
+          extra: (struct){
+          }
+        }
+      }
+      shared: (struct){
+        all_total: (struct){
+          tags: (struct){
+            extra: (struct){
+            }
+          }
+        }
+      }
+    }
+    #Tags: (#struct){
+    }
+    bar: (struct){
+      shared: (struct){
+        bar_total: (struct){
+          tags: (#struct){
+            extra: (#struct){
+            }
+          }
+        }
+        all_total: (struct){
+          tags: (#struct){
+            extra: (#struct){
+            }
+          }
+        }
+      }
+    }
+  }
 }
 -- out/compile --
 --- in.cue
@@ -113,3 +206,36 @@ diff old new
     port: (int|*30080)
   }
 }
+--- secondary.cue
+{
+  related: {
+    [string]: {
+      if true {
+        shared: {
+          all_total: 〈3;foo〉.fooData.total
+        }
+      }
+    }
+    foo: {
+      fooData: {
+        total: {
+          tags: 〈1;_hidden〉
+        }
+        _hidden: {
+          extra: {}
+        }
+      }
+    }
+    #Tags: {
+      [string]: {}
+    }
+    bar: {
+      shared: {
+        [string]: {
+          tags: 〈3;#Tags〉
+        }
+        bar_total: 〈2;foo〉.fooData.total
+      }
+    }
+  }
+}

internal/core/adt/sched.go

@@ -413,9 +413,12 @@ processNextTask:
 		if s.meets(needs) {
 			return true
 		}
-		c.current().waitFor(s, needs)
-		s.yield()
-		panic("unreachable")
+		// This can happen in some cases. We "promote" to finalization if this
+		// was not triggered by a task.
+		if t := c.current(); t != nil {
+			t.waitFor(s, needs)
+			s.yield()
+		}
 
 	case finalize:
 		// remainder of function

internal/core/adt/unify.go

@@ -214,13 +214,8 @@ func (v *Vertex) unify(c *OpContext, needs condition, mode runMode, checkTypos b
 	if n.node.ArcType == ArcPending {
 		// forcefully do an early recursive evaluation to decide the state
 		// of the arc. See https://cuelang.org/issue/3621.
-		n.process(nodeOnlyNeeds, attemptOnly)
-		if n.node.ArcType == ArcPending {
-			for _, a := range n.node.Arcs {
-				a.unify(c, needs, attemptOnly, checkTypos)
-			}
-		}
-		n.completePending(mode)
+		n.process(pendingKnown, yield)
+		n.completePending(yield)
 	}
 
 	n.process(nodeOnlyNeeds, mode)