internal/core/adt: break notification dependency on structural cycle

This case is specifically is not handled if structure
sharing is off, or when a let is involved.

Consider, for instance, this case:

    a: next: X
    let X = a

Here, `a` is substituted for `X`, after which a
structural cycle is detected. At this point,
any notification resulting from X should be
cancelled. Basically, cycle detection followed
a different path, where this was not done.

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

Author: mpvl

internal/core/adt/conjunct.go

@@ -554,6 +554,10 @@ func (n *nodeContext) addNotify2(v *Vertex, c CloseInfo) {
 
 	cc := c.cc
 
+	// TODO: it should not be necessary to register for notifications for
+	// let expressions, so we could also filter for !n.node.Label.IsLet().
+	// However, somehow this appears to result in slightly better error
+	// messages.
 	if root.addNotifyDependency(n.ctx, cc) {
 		// TODO: this is mostly identical to the slice in the root closeContext.
 		// Use only one once V2 is removed.

internal/core/adt/eval_test.go

@@ -76,14 +76,11 @@ var needFix = map[string]string{
 // counter errors.
 // TODO: These counters should all go to zero.
 var skipDebugDepErrors = map[string]int{
-	"compile/scope":            1,
-	"comprehensions/pushdown":  2,
 	"cycle/builtins":           2,
 	"cycle/comprehension":      1,
 	"cycle/disjunction":        4,
-	"cycle/evaluate":           1,
 	"cycle/issue990":           1,
-	"cycle/structural":         9,
+	"cycle/structural":         7,
 	"disjunctions/errors":      3,
 	"disjunctions/elimination": 19,
 	"disjunctions/nested":      1,

internal/core/adt/unify.go

@@ -285,6 +285,7 @@ func (v *Vertex) unify(c *OpContext, needs condition, mode runMode) bool {
 	case needs&subFieldsProcessed != 0:
 		switch {
 		case assertStructuralCycleV3(n):
+			n.breakIncomingDeps()
 		// TODO: consider bailing on error if n.errs != nil.
 		case n.completeAllArcs(needs, mode):
 		}
@@ -462,13 +463,15 @@ func (n *nodeContext) completeNodeTasks(mode runMode) {
 		}()
 	}
 
-	if p := v.Parent; p != nil && p.state != nil {
-		if !v.IsDynamic && n.completed&allAncestorsProcessed == 0 {
-			p.state.completeNodeTasks(mode)
+	if !v.Label.IsLet() {
+		if p := v.Parent; p != nil && p.state != nil {
+			if !v.IsDynamic && n.completed&allAncestorsProcessed == 0 {
+				p.state.completeNodeTasks(mode)
+			}
 		}
 	}
 
-	if v.IsDynamic || v.Parent.allChildConjunctsKnown() {
+	if v.IsDynamic || v.Label.IsLet() || v.Parent.allChildConjunctsKnown() {
 		n.signal(allAncestorsProcessed)
 	}