internal/core/adt: evaluate pattern values when registering them

The or() builtin returns a Disjunction holding raw, unevaluated list
elements. When such a value was used as a pattern label, the pattern
was registered as-is, deferring evaluation of its branches until
matchPatternValue force-finalized them upon a later field insertion.
At that point a node the branches depend on may be mid-evaluation, so
a comprehension over it would see a partial field set, collapsing the
inner or() and locking the spurious result in permanently.

Evaluate vertices held by a pattern value when the pattern constraint
task runs, mirroring what already happens for resolvers and literal
disjunctions used as pattern labels: at this point dependencies can
still be run reentrantly, or block and requeue the task. This keeps
or() lazy in all other contexts, retaining the performance win of
returning a Disjunction directly.

Fixes #4399

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I5ba836a2a24343b6b6ce1a03c192cf0a5e75b7f4
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1242737
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/cycle/issue4397.txtar

@@ -19,14 +19,14 @@ result: {"cue": cue, "tags": tags} @test(eq, {
 	tags: {cue: {modules: ["login", "mod"]}}
 })
 -- out/eval/stats --
-Leaks:  1
-Freed:  26
-Reused: 4
-Allocs: 23
+Leaks:  2
+Freed:  28
+Reused: 9
+Allocs: 21
 Retain: 0
 
 Unifications: 18
-Conjuncts:    38
-Disjuncts:    2
+Conjuncts:    43
+Disjuncts:    4
 
-NumCloseIDs: 13
+NumCloseIDs: 17

cue/testdata/cycle/issue4399.txtar

@@ -1,8 +1,8 @@
-# A pattern label using or() over a cyclic reference collapses the inner or()
-# disjunction to the first field of cue when close() adds a field that is not
-# in tags.cue.modules: the pattern is registered as a Disjunction holding
-# unevaluated list elements, which are then force-finalized while cue is
-# mid-evaluation, computing the inner or() from a partial field set.
+# A pattern label using or() over a cyclic reference used to collapse the
+# inner or() disjunction to the first field of cue when close() added a field
+# that is not in tags.cue.modules: the pattern was registered as a Disjunction
+# holding unevaluated list elements, which were then force-finalized while cue
+# was mid-evaluation, computing the inner or() from a partial field set.
 -- in.cue --
 cue: {
 	[or(tags.cue.modules)]: y: {}
@@ -17,30 +17,19 @@ cue: close({
 	mod:   _
 })
 
-result: {"cue": cue, "tags": tags} @test(err, code=eval, contains="conflicting values \"other\" and \"login\"", pos=[-9:27, -9:49, -9:50, -8:22]) @test(eq:todo, {
+result: {"cue": cue, "tags": tags} @test(eq, {
 	cue: {other: {}, login: {y: {}}, mod: {y: {}}}
 	tags: {cue: {modules: ["login", "mod"]}}
 })
--- out/errors.txt --
-[eval] tags.cue.modules.0: conflicting values "other" and "login":
-    ./in.cue:5:27
-    ./in.cue:5:49
-    ./in.cue:5:50
-    ./in.cue:6:22
-[eval] tags.cue.modules.1: conflicting values "other" and "mod":
-    ./in.cue:5:27
-    ./in.cue:5:49
-    ./in.cue:5:50
-    ./in.cue:6:31
 -- out/eval/stats --
-Leaks:  0
-Freed:  25
-Reused: 2
-Allocs: 23
+Leaks:  2
+Freed:  35
+Reused: 12
+Allocs: 25
 Retain: 0
 
-Unifications: 17
-Conjuncts:    30
-Disjuncts:    0
+Unifications: 19
+Conjuncts:    52
+Disjuncts:    6
 
-NumCloseIDs: 9
+NumCloseIDs: 25

internal/core/adt/constraints.go

@@ -135,6 +135,8 @@ func matchPattern(ctx *OpContext, pattern Value, f Feature) bool {
 // This is an optimization an intended to be faster than regular CUE evaluation
 // for the majority of cases where pattern constraints are used.
 func matchPatternValue(ctx *OpContext, pattern Value, f Feature) (result bool) {
+	// Normally completePatternValue has already evaluated the pattern at
+	// registration time; this finalize remains as a backstop.
 	if v, ok := pattern.(*Vertex); ok {
 		v.unify(ctx, Flags{condition: scalarKnown, mode: finalize, checkTypos: false})
 	}

internal/core/adt/fields.go

@@ -334,9 +334,39 @@ func (n *nodeContext) addConstraint(arc *Vertex, mode ArcType, c Conjunct, check
 	arc.insertConjunct(n.ctx, c, c.CloseInfo, mode, check, false)
 }
 
+// completePatternValue evaluates any vertices held by a pattern value before
+// it is registered as a pattern constraint. Values returned by builtins, such
+// as a Disjunction returned by or(), may hold vertices that have not been
+// evaluated yet. If their evaluation were left to matchPatternValue, it could
+// be forced at a point where a node they depend on is mid-evaluation, locking
+// in results computed from a partial field set. Evaluating them here, while
+// dependencies can still be run or block the calling task, avoids that.
+// See https://cuelang.org/issue/4399.
+//
+// The traversal mirrors the Value kinds that matchPatternValue fast-tracks;
+// values it does not descend into, such as the arguments of a
+// BuiltinValidator, are still finalized late by matchPatternValue.
+func completePatternValue(ctx *OpContext, v Value) {
+	var vals []Value
+	switch x := v.(type) {
+	case *Vertex:
+		x.unify(ctx, Flags{condition: scalarValue, mode: yield})
+		return
+	case *Disjunction:
+		vals = x.Values
+	case *Conjunction:
+		vals = x.Values
+	}
+	for _, a := range vals {
+		completePatternValue(ctx, a)
+	}
+}
+
 func (n *nodeContext) insertPattern(pattern Value, c Conjunct) {
 	n.assertInitialized()
 
+	completePatternValue(n.ctx, pattern)
+
 	// Collect patterns in root vertex. This allows comparing disjuncts for
 	// equality as well as inserting new arcs down the line as they are
 	// inserted.

internal/core/compile/testdata/sync/cycle/issue4399.txtar

@@ -1,8 +1,8 @@
-# A pattern label using or() over a cyclic reference collapses the inner or()
-# disjunction to the first field of cue when close() adds a field that is not
-# in tags.cue.modules: the pattern is registered as a Disjunction holding
-# unevaluated list elements, which are then force-finalized while cue is
-# mid-evaluation, computing the inner or() from a partial field set.
+# A pattern label using or() over a cyclic reference used to collapse the
+# inner or() disjunction to the first field of cue when close() added a field
+# that is not in tags.cue.modules: the pattern was registered as a Disjunction
+# holding unevaluated list elements, which were then force-finalized while cue
+# was mid-evaluation, computing the inner or() from a partial field set.
 -- in.cue --
 cue: {
 	[or(tags.cue.modules)]: y: {}