internal/core/adt: evaluate nodes that are otherwise not evaluated

With structure sharing, nodes were only sometimes
evaluated, assuming that the shared node will be
eventually evaluated. This is not always the case,
however, for instance when a shared structure refers
to a part of a package that is otherwise not evaluated.

We now ensure that a shared node is _always_ finalized.
Sometimes, like when a node is on an optional path,
we cannot currently evaluate it on the part of the stack
that would cause a direct structural cycle to be detected.
We can eventually find something better, but for now we
just record the node to be evaluated at the top of the
stack.

Fixes #3511

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

cmd/cue/cmd/testdata/script/export_issue3511.txtar

@@ -0,0 +1,34 @@
+
+env CUE_EXPERIMENT=evalv3=1
+exec cue export
+cmp stdout out/stdout
+
+-- cue.mod/module.cue --
+module: "module.test/foo"
+language: version: "v0.9.0"
+
+-- main.cue --
+package p
+
+import "module.test/foo/imported@v0"
+
+items: [imported.List]
+-- imported/imported.cue --
+package imported
+
+Namespace: "default"
+
+List: [...{namespace: Namespace}]
+
+List: [{name: "kube-api-server"}]
+-- out/stdout --
+{
+    "items": [
+        [
+            {
+                "name": "kube-api-server",
+                "namespace": "default"
+            }
+        ]
+    ]
+}

internal/core/adt/context.go

@@ -229,6 +229,12 @@ type OpContext struct {
 	// structural cycle errors.
 	vertex *Vertex
 
+	// list of vertices that need to be finalized.
+	// TODO: remove this again once we have a proper way of detecting references
+	// across optional boundaries in hasAncestorV3. We can probably do this
+	// with an optional depth counter.
+	toFinalize []*Vertex
+
 	// These fields are used associate scratch fields for computing closedness
 	// of a Vertex. These fields could have been included in StructInfo (like
 	// Tomabechi's unification algorithm), but we opted for an indirection to

internal/core/adt/share.go

@@ -58,10 +58,20 @@ func (n *nodeContext) finalizeSharing() {
 		n.sharedID.cc.decDependent(n.ctx, SHARED, n.node.cc())
 		n.sharedID.cc = nil
 	}
-	if n.isShared {
-		v := n.node.BaseValue.(*Vertex)
+	if !n.isShared {
+		return
+	}
+	switch v := n.node.BaseValue.(type) {
+	case *Vertex:
+		if n.sharedID.CycleType == NoCycle {
+			v.Finalize(n.ctx)
+		} else if !v.isFinal() {
+			// TODO: ideally we just handle cycles in optional chains directly,
+			// rather than relying on this mechanism. This requires us to add
+			// a mechanism to detect that.
+			n.ctx.toFinalize = append(n.ctx.toFinalize, v)
+		}
 		if v.Parent == n.node.Parent {
-			v.unify(n.ctx, needTasksDone, finalize)
 			if !v.Rooted() && v.MayAttach() {
 				n.isShared = false
 				n.node.Arcs = v.Arcs
@@ -71,6 +81,10 @@ func (n *nodeContext) finalizeSharing() {
 				n.node.HasEllipsis = v.HasEllipsis
 			}
 		}
+	case *Bottom:
+		// An error trumps sharing. We can leave it as is.
+	default:
+		panic("unreachable")
 	}
 }
 

internal/core/adt/unify.go

@@ -130,6 +130,19 @@ func (v *Vertex) unify(c *OpContext, needs condition, mode runMode) bool {
 		}()
 	}
 
+	if c.evalDepth == 0 {
+		defer func() {
+			// This loop processes nodes that need to be evaluated, but should be
+			// evaluated outside of the stack to avoid structural cycle detection.
+			// See comment at toFinalize.
+			a := c.toFinalize
+			c.toFinalize = c.toFinalize[:0]
+			for _, x := range a {
+				x.Finalize(c)
+			}
+		}()
+	}
+
 	if mode == ignore {
 		return false
 	}