internal/core/adt: add some defensive code

This change is needed for an upcoming change, where
it would otherwise cause a crash. It addresses a
few issues:

In allChildConjunctsKnown: it was sometimes called
with a status that is finalized. This was possible in some
rare cases. It should be safe to simply return true
in these cases, so that is what we do now.

Separately, we changed one of the call sites in unify
to no call this function if such a condition is met.
Note that Rooted additionally tests whether the
nonRooted flag is set.

We separate out these changes so that bisection will
isolate any issues caused by this.

Issue #2854

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

Author: mpvl

internal/core/adt/states.go

@@ -294,6 +294,15 @@ func (v *Vertex) allChildConjunctsKnown() bool {
 		return true
 	}
 
+	if v.status == finalized {
+		// This can happen, for instance, if this is called on a parent of a
+		// rooted node that is marked as a parent for a dynamic node.
+		// In practice this should be handled by the caller, but we add this
+		// as an extra safeguard.
+		// TODO: remove this check at some point.
+		return true
+	}
+
 	return v.state.meets(fieldConjunctsKnown | allAncestorsProcessed)
 }
 

internal/core/adt/unify.go

@@ -152,7 +152,7 @@ func (v *Vertex) unify(c *OpContext, needs condition, mode runMode) bool {
 	// Note that if mode is final, we will guarantee that the conditions for
 	// this if clause are met down the line. So we assume this is already the
 	// case and set the signal accordingly if so.
-	if v.Label.IsLet() || v.IsDynamic || v.Parent.allChildConjunctsKnown() || mode == finalize {
+	if !v.Rooted() || v.Parent.allChildConjunctsKnown() || mode == finalize {
 		n.signal(allAncestorsProcessed)
 	}