cue: fix IsClosed for new evaluator

The new evaluator does not implement IsSpan. Instead
it kept track of closedness through the new closeContext
construct. The relevant information on whether or not
a struct is closed is now copied to fields in the Vertex.
Rather than just having Closed, it keeps trackof this
with an additional field HasEllipsis. This allows
accumulating information as it comes in, instead of
possibly revert setting Closed, which is less error prone.
This is especially so because the old evaluator uses
Closed in a slightly different way.

Issue #3060

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

cue/types.go

@@ -1159,6 +1159,8 @@ func (v hiddenValue) IsClosed() bool {
 	case ListKind:
 		return v.v.IsClosedList()
 	case StructKind:
+		// TODO: remove this more expensive computation once the old evaluator
+		// is removed.
 		return !v.Allows(AnyString)
 	}
 	return false
@@ -1169,6 +1171,9 @@ func (v hiddenValue) IsClosed() bool {
 // Allows does not take into account validators like list.MaxItems(4). This may
 // change in the future.
 func (v Value) Allows(sel Selector) bool {
+	if v.v.HasEllipsis {
+		return true
+	}
 	c := v.ctx()
 	f := sel.sel.feature(c)
 	return v.v.Accept(c, f)

cue/types_test.go

@@ -340,8 +340,6 @@ func TestValueType(t *testing.T) {
 	}}
 	for _, tc := range testCases {
 		runMatrix(t, tc.value, func(t *testing.T, cfg *evalConfig) {
-			TODO_V3(t, cfg)
-
 			inst := cfg.getValue(t, tc.value)
 			v := inst.Lookup("v")
 			if got := v.Kind(); got != tc.kind {

internal/core/adt/composite.go

@@ -186,6 +186,10 @@ type Vertex struct {
 	// conjuncts, or ancestor conjuncts, is a definition.
 	Closed bool
 
+	// HasEllipsis indicates that this Vertex is open by means of an ellipsis.
+	// TODO: combine this field with Closed once we removed the old evaluator.
+	HasEllipsis bool
+
 	// MultiLet indicates whether multiple let fields were added from
 	// different sources. If true, a LetReference must be resolved using
 	// the per-Environment value cache.
@@ -925,10 +929,22 @@ func (v *Vertex) accepts(ok, required bool) bool {
 }
 
 func (v *Vertex) IsClosedStruct() bool {
+	// TODO: uncomment this. This fixes a bunch of closedness bugs
+	// in the old and new evaluator. For compability sake, though, we
+	// keep it as is for now.
+	// if v.Closed {
+	// 	return true
+	// }
+	// if v.HasEllipsis {
+	// 	return false
+	// }
 	switch x := v.BaseValue.(type) {
 	default:
 		return false
 
+	case *Vertex:
+		return v.Closed && !v.HasEllipsis
+
 	case *StructMarker:
 		if x.NeedClose {
 			return true

internal/core/adt/unify.go

@@ -162,10 +162,8 @@ func (v *Vertex) unify(c *OpContext, needs condition, mode runMode) bool {
 
 	w := v.DerefDisjunct()
 	if w != v {
-		if w.Closed {
-			// Should resolve with dereference.
-			v.Closed = true
-		}
+		// Should resolve with dereference.
+		v.Closed = w.Closed
 		v.status = w.status
 		v.ChildErrors = CombineErrors(nil, v.ChildErrors, w.ChildErrors)
 		v.Arcs = nil
@@ -282,19 +280,37 @@ func (v *Vertex) unify(c *OpContext, needs condition, mode runMode) bool {
 		v.ChildErrors = nil
 		v.Arcs = nil
 
+		result := w.unify(c, needs, mode)
+
 		// Set control fields that are referenced without dereferencing.
 		if w.Closed {
-			// Should resolve with dereference.
 			v.Closed = true
 		}
-
-		result := w.unify(c, needs, mode)
+		if w.HasEllipsis {
+			v.HasEllipsis = true
+		}
 		v.status = w.status
+
 		return result
 	}
 
+	// TODO: adding this is wrong, but it should not cause the snippet below
+	// to hang. Investigate.
+	// v.Closed = v.cc.isClosed
+	//
+	// This hangs:
+	// issue1940: {
+	// 	#T: ["a", #T] | ["c", #T] | ["d", [...#T]]
+	// 	#A: t: #T
+	// 	#B: x: #A
+	// 	#C: #B
+	// 	#C: x: #A
+	// }
+
 	// validationCompleted
 	if n.completed&(subFieldsProcessed) != 0 {
+		n.node.HasEllipsis = n.node.cc.hasEllipsis
+
 		n.node.updateStatus(finalized)
 
 		defer n.unmarkOptional(n.markOptional())