internal/core/adt: keep lookups of settled arcs from forcing unrelated tasks

Vertex.lookup drives the target scheduler whenever not all of its tasks
have started, demanding allTasksCompleted. That also runs pushed-down
comprehensions that cannot affect the arc being looked up: a
comprehension whose body consists of literal fields only contributes
conjuncts to the arcs it pre-created at scheduling time, none of which
is the requested one.

Running such a comprehension is not just unnecessary work. Its guard may
resolve a disjunction, which evaluates its disjuncts in finalize mode;
that mode force-freezes blocked tasks, permanently recording incomplete
errors on vertices whose dependencies are still being computed further
up the evaluation stack.

In the reproducer, a reference to a settled member arc _port forced the
comprehension guard on its parent struct. Resolving the sibling spec
disjunction finalized a "\(port)" interpolation elsewhere in the tree
while the port vertex was mid-lookup on the stack, freezing it as an
"invalid interpolation: non-concrete value" error even though port
resolved to a concrete value immediately afterwards.

When the requested arc is already a member and provably cannot gain
conjuncts from a pushed-down comprehension, drop allTasksCompleted from
the needs mask when driving the scheduler: the task selection in process
then leaves such comprehensions unforced while other pending tasks still
run.

Fixes #4448

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I7513822cb7a536efe84ef975db5d79a25fca8093
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1242735
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@gmail.com>

Author: mvdan

cue/testdata/comprehensions/issue4448.txtar

@@ -45,8 +45,7 @@ app: {
 		_port: 8080 @test(eq, 8080)
 		spec: {
 			src: "x" @test(eq, "x")
-			tags: [#cfg._all] @test(err, at=0.0, code=incomplete,
-				contains="invalid interpolation", contains="non-concrete value", pos=[-19:13]) @test(eq:todo, [["8080"]])
+			tags: [#cfg._all] @test(eq, [["8080"]])
 		}
 	}
 }
@@ -55,15 +54,11 @@ app: {
     ./in.cue:19:13
 [incomplete] #Mixin.#cfg.entries.w._lines.0: invalid interpolation: non-concrete value _ (type _):
     ./in.cue:19:13
-[incomplete] app.#cfg.entries.w._lines.0: invalid interpolation: non-concrete value _ (type _):
-    ./in.cue:19:13
-[incomplete] app.#cfg.entries.w._lines.0: invalid interpolation: non-concrete value _ (type _):
-    ./in.cue:19:13
 -- out/eval/stats --
 Leaks:  4
 Freed:  45
-Reused: 23
-Allocs: 26
+Reused: 28
+Allocs: 21
 Retain: 0
 
 Unifications: 41

internal/core/adt/comprehension.go

@@ -334,3 +334,12 @@ func pushDownDeps(n *nodeContext, t *task, x Node) condition {
 
 	return completes | allTasksCompleted
 }
+
+// isPushedDownComp reports whether t is a comprehension task whose body
+// was fully pushed down: [pushDownDeps] returns bare allTasksCompleted
+// exactly when the body consists of literal fields only. Such a task
+// contributes conjuncts solely to the arcs it pre-created at scheduling
+// time, registering itself in their parentTasks.
+func (t *task) isPushedDownComp() bool {
+	return t.run == handleComprehension && t.completes == allTasksCompleted
+}

internal/core/adt/disjunct2.go

@@ -249,8 +249,7 @@ func hasDeferredCompAncestor(v *Vertex) bool {
 			return false
 		}
 		for _, t := range p.state.tasks {
-			if t.run == handleComprehension &&
-				t.completes == allTasksCompleted {
+			if t.isPushedDownComp() {
 				return true
 			}
 		}

internal/core/adt/sched.go

@@ -534,16 +534,13 @@ processNextTask:
 			// A comprehension still evaluating its clauses does not defer
 			// resolvers: its guards may depend on the resolver's conjuncts.
 			continue
-		case hasPendingDisjunction && t.run == handleComprehension &&
-			t.completes == allTasksCompleted:
+		case hasPendingDisjunction && t.isPushedDownComp():
 			// Defer pushed-down comprehensions until pending
-			// disjunctions have expanded the parent into disjuncts.
-			// Only comps whose body was fully pushed down (after
-			// pushDownDeps, completes == allTasksCompleted) are
-			// delayed: such comps need to run inside each disjunct
-			// so the pushed-down arcs see that disjunct's view.
-			// Comps with broader completes (e.g. for-comps over a
-			// list) are not delayed.
+			// disjunctions have expanded the parent into disjuncts:
+			// such comps need to run inside each disjunct so the
+			// pushed-down arcs see that disjunct's view. Comps with
+			// broader completes (e.g. for-comps over a list) are not
+			// delayed.
 			continue
 		default:
 			runTask(t, mode)

internal/core/adt/unify.go

@@ -812,6 +812,38 @@ func root(v *Vertex) *Vertex {
 	return v
 }
 
+// arcSettledForLookup reports whether the arc for f is already a member
+// whose conjunct set cannot be affected by pushed-down comprehension
+// tasks, so that a lookup-driven process call may leave those tasks
+// unforced by dropping allTasksCompleted from its needs mask.
+//
+// This preserves the fine-grained dependency tracking of comprehension
+// pushdown: a pushed-down comprehension task (see [task.isPushedDownComp])
+// contributes conjuncts solely to the arcs it pre-created at scheduling
+// time, so one that may still contribute to f is found in the parentTasks
+// of this node or of the arc itself.
+//
+// TODO(cycle): this guard removes one trigger of a deeper problem. When a
+// forced task resolves a disjunction, doDisjunct leaks finalize mode into
+// vertices outside its overlay, and the freeze cycle-breaker then commits
+// permanent incomplete errors for tasks blocked on a vertex that is merely
+// in-flight on the evaluation stack, not semantically cyclic. A more
+// principled fix would distinguish stack re-entrancy from value cycles in
+// the freeze path, or keep finalize scoped to the disjunct overlay.
+func (n *nodeContext) arcSettledForLookup(f Feature) bool {
+	// An ancestor comprehension may still add conjuncts to this node.
+	if n.hasActiveParentTask() {
+		return false
+	}
+	arc := n.node.LookupRaw(f)
+	if arc == nil || arc.ArcType != ArcMember {
+		return false
+	}
+	// Pushed-down comprehensions that may contribute to f, whether from
+	// this node or an ancestor, are registered on the arc itself.
+	return arc.state == nil || !arc.state.hasActiveParentTask()
+}
+
 func (v *Vertex) lookup(c *OpContext, pos token.Pos, f Feature, flags Flags) *Vertex {
 	needs := flags.condition
 	runMode := flags.mode
@@ -851,9 +883,17 @@ func (v *Vertex) lookup(c *OpContext, pos token.Pos, f Feature, flags Flags) *Ve
 
 		// Drive the lookup target forward when its scheduler has not yet
 		// started everything; the !allTasksStarted guard keeps us out of
-		// nodes that are already mid-execution.
+		// nodes that are already mid-execution. When the requested arc is
+		// already settled (see [nodeContext.arcSettledForLookup]), drop
+		// allTasksCompleted from the needs mask: the selectTasks filter in
+		// process then skips pushed-down comprehensions, which cannot
+		// affect the arc, instead of forcing them.
 		if !allTasksStarted(state) {
-			state.process(valueKnown|fieldConjunctsKnown|allTasksCompleted, attemptOnly)
+			needs := valueKnown | fieldConjunctsKnown | allTasksCompleted
+			if state.arcSettledForLookup(f) {
+				needs = valueKnown | fieldConjunctsKnown
+			}
+			state.process(needs, attemptOnly)
 			state.updateScalar()
 		}
 	}