internal/core/adt: keep nodeContext in Vertex

This allows reusing the nodeContext in different phases
and better control over prevendting duplicate work.

Change-Id: I54223595ea69320c96c7e3d3fbd68d96d138cfb1
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8106
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>

cue/testdata/basicrewrite/018_self-reference_cycles.txtar

@@ -27,11 +27,11 @@ c: [c[1], c[0]]
 (struct){
   a: (_|_){
     // [cycle] cycle error:
-    //     ./in.cue:2:4
+    //     ./in.cue:1:4
   }
   b: (_|_){
     // [cycle] cycle error:
-    //     ./in.cue:2:4
+    //     ./in.cue:1:4
   }
   c: (#list){
     0: (_){ _ }

cue/testdata/cycle/021_delayed_constraint_failure.txtar

@@ -26,7 +26,7 @@ b: _|_ // conflicting values 210 and 200
 }
 -- out/eval --
 Errors:
-b: conflicting values 200 and 210:
+b: conflicting values 210 and 200:
     ./in.cue:2:4
     ./in.cue:3:4
 x: conflicting values 101 and 100:
@@ -38,7 +38,7 @@ Result:
   // [eval]
   a: (int){ 100 }
   b: (_|_){
-    // [eval] b: conflicting values 200 and 210:
+    // [eval] b: conflicting values 210 and 200:
     //     ./in.cue:2:4
     //     ./in.cue:3:4
   }

cue/testdata/cycle/049_self-reference_cycles_conflicts_with_strings.txtar

@@ -42,10 +42,6 @@ Result:
       //     ./in.cue:2:5
       //     ./in.cue:5:7
     }
-    y: (_|_){
-      // [eval] a.x: conflicting values "hey!?" and "hey":
-      //     ./in.cue:2:5
-      //     ./in.cue:5:7
-    }
+    y: (string){ "hey!" }
   }
 }

cue/testdata/cycle/structural.txtar

@@ -409,6 +409,7 @@ b7.a.0: structural cycle
 c1.a.c.c: structural cycle
 d1.a.b.c.d.t: structural cycle
 d1.r: structural cycle
+d2.r.c.d.t: structural cycle
 e1.a.c: structural cycle
 e1.b.c: structural cycle
 e2.a.c: structural cycle
@@ -468,10 +469,6 @@ p3.#U.#T.a.b.link: structural cycle
 p5.#T.a.0.link: structural cycle
 p6.#U.#T.a.0.link: structural cycle
 z1.z.g.h: structural cycle
-b4.x.y.0.0: structural cycle:
-    ./in.cue:53:8
-d2.r.c.d.t: structural cycle:
-    ./in.cue:254:8
 0: structural cycle:
     ./in.cue:264:19
 
@@ -561,9 +558,10 @@ Result:
   b4: (_|_){
     // [structural cycle]
     b: (_|_){
-      // [structural cycle] b4.x.y.0.0: structural cycle:
-      //     ./in.cue:53:8
-      0: (int){ 1 }
+      // [structural cycle]
+      0: (_|_){
+        // [structural cycle] b4.x.y.0: structural cycle
+      }
     }
     x: (_|_){
       // [structural cycle]
@@ -586,9 +584,7 @@ Result:
     x: (struct){
       y: (struct){
         a: (#list){
-          0: ((int|list)){ |((#list){
-              0: (int){ int }
-            }, (int){ int }) }
+          0: (int){ int }
         }
       }
     }
@@ -1053,8 +1049,14 @@ Result:
   d2: (_|_){
     // [structural cycle]
     x: (_|_){
-      // [structural cycle] d2.r.c.d.t: structural cycle:
-      //     ./in.cue:254:8
+      // [structural cycle]
+      d: (_|_){
+        // [structural cycle]
+        h: (int){ int }
+        t: (_|_){
+          // [structural cycle]
+        }
+      }
     }
     r: (_|_){
       // [structural cycle]
@@ -1064,8 +1066,7 @@ Result:
           // [structural cycle]
           h: (int){ int }
           t: (_|_){
-            // [structural cycle] d2.r.c.d.t: structural cycle:
-            //     ./in.cue:254:8
+            // [structural cycle] d2.r.c.d.t: structural cycle
             c: (_|_){// {
               //   d: {
               //     h: int
@@ -1088,6 +1089,16 @@ Result:
             h: (int){ int }
             t: (_|_){
               // [structural cycle]
+              c: (_|_){
+                // [structural cycle]
+                d: (_|_){
+                  // [structural cycle]
+                  h: (int){ int }
+                  t: (_|_){
+                    // [structural cycle]
+                  }
+                }
+              }
             }
           }
         }

cue/testdata/eval/incomplete.txtar

@@ -48,21 +48,19 @@ Result:
   E: (struct){
     a: (_|_){
       // [cycle] cycle error:
-      //     ./in.cue:12:6
-      // cycle error:
-      //     ./in.cue:13:6
+      //     ./in.cue:11:6
     }
     b: (_|_){
       // [cycle] cycle error:
-      //     ./in.cue:12:6
+      //     ./in.cue:11:6
       // cycle error:
-      //     ./in.cue:13:6
+      //     ./in.cue:12:6
     }
     c: (_|_){
       // [cycle] cycle error:
-      //     ./in.cue:12:6
+      //     ./in.cue:11:6
       // cycle error:
-      //     ./in.cue:13:6
+      //     ./in.cue:12:6
     }
   }
   permanentlyIncompleteOperands: (_|_){

cue/testdata/fulleval/026_dont_convert_incomplete_errors_to_non-incomplete.txtar

@@ -49,11 +49,11 @@ s3:  strings.ContainsAny(str, "dd")
   n1: (struct){
     min: (_|_){
       // [cycle] cycle error:
-      //     ./in.cue:3:23
+      //     ./in.cue:3:12
     }
     max: (_|_){
       // [cycle] cycle error:
-      //     ./in.cue:3:23
+      //     ./in.cue:3:12
     }
   }
   n2: (_|_){

internal/core/adt/composite.go

@@ -137,7 +137,7 @@ func (e *Environment) evalCached(c *OpContext, x Expr) Value {
 		}
 		env, src := c.e, c.src
 		c.e, c.src = e, x.Source()
-		v = c.eval(x)
+		v = c.evalState(x, Partial)
 		c.e, c.src = env, src
 		e.cache[x] = v
 	}
@@ -161,7 +161,13 @@ type Vertex struct {
 	// Label is the feature leading to this vertex.
 	Label Feature
 
-	// TODO: move the following status fields to a separate struct.
+	// State:
+	//   eval: nil, BaseValue: nil -- unevaluated
+	//   eval: *,   BaseValue: nil -- evaluating
+	//   eval: *,   BaseValue: *   -- finalized
+	//
+	state *nodeContext
+	// TODO: move the following status fields to nodeContext.
 
 	// status indicates the evaluation progress of this vertex.
 	status VertexStatus
@@ -245,6 +251,10 @@ const (
 	// nodeContext to allow reusing the computations done so far.
 	Partial
 
+	// AllArcs is request only. It must be past Partial, but
+	// before recursively resolving arcs.
+	AllArcs
+
 	// EvaluatingArcs indicates that the arcs of the Vertex are currently being
 	// evaluated. If this is encountered it indicates a structural cycle.
 	// Value does not have to be nil
@@ -274,7 +284,6 @@ func (v *Vertex) UpdateStatus(s VertexStatus) {
 // Value returns the Value of v without definitions if it is a scalar
 // or itself otherwise.
 func (v *Vertex) Value() Value {
-	// TODO: rename to Value.
 	switch x := v.BaseValue.(type) {
 	case nil:
 		return nil
@@ -425,6 +434,15 @@ func Unwrap(v Value) Value {
 	if !ok {
 		return v
 	}
+	// b, _ := x.BaseValue.(*Bottom)
+	if n := x.state; n != nil && x.BaseValue == cycle {
+		if n.errs != nil && !n.errs.IsIncomplete() {
+			return n.errs
+		}
+		if n.scalar != nil {
+			return n.scalar
+		}
+	}
 	return x.Value()
 }
 
@@ -575,13 +593,17 @@ func (v *Vertex) AddConjunct(c Conjunct) *Bottom {
 		// This is likely a bug in the evaluator and should not happen.
 		return &Bottom{Err: errors.Newf(token.NoPos, "cannot add conjunct")}
 	}
+	v.addConjunct(c)
+	return nil
+}
+
+func (v *Vertex) addConjunct(c Conjunct) {
 	for _, x := range v.Conjuncts {
 		if x == c {
-			return nil
+			return
 		}
 	}
 	v.Conjuncts = append(v.Conjuncts, c)
-	return nil
 }
 
 func (v *Vertex) AddStruct(s *StructLit, env *Environment, ci CloseInfo) *StructInfo {

internal/core/adt/context.go

@@ -504,7 +504,7 @@ func (c *OpContext) getDefault(v Value) (result Value, ok bool) {
 func (c *OpContext) Evaluate(env *Environment, x Expr) (result Value, complete bool) {
 	s := c.PushState(env, x.Source())
 
-	val := c.eval(x)
+	val := c.evalState(x, Partial)
 
 	complete = true
 
@@ -541,11 +541,6 @@ func (c *OpContext) value(x Expr) (result Value) {
 	return v
 }
 
-func (c *OpContext) eval(x Expr) (result Value) {
-	v := c.evalState(x, Partial)
-	return v
-}
-
 func (c *OpContext) evalState(v Expr, state VertexStatus) (result Value) {
 	savedSrc := c.src
 	c.src = v.Source()
@@ -583,14 +578,11 @@ func (c *OpContext) evalState(v Expr, state VertexStatus) (result Value) {
 		if c.HasErr() {
 			return nil
 		}
-		if isIncomplete(arc) {
-			if arc != nil {
-				return arc.Value() // *Bottom
-			}
+		if arc == nil {
 			return nil
 		}
 
-		v := c.Unifier.Evaluate(c, arc)
+		v := c.Unifier.evaluate(c, arc, state)
 		return v
 
 	default:
@@ -728,7 +720,9 @@ func pos(x Node) token.Pos {
 func (c *OpContext) node(orig Node, x Expr, scalar bool) *Vertex {
 	// TODO: always get the vertex. This allows a whole bunch of trickery
 	// down the line.
-	v := c.evalState(x, EvaluatingArcs)
+	// This must be partial, because
+	// TODO: this should always be "AllArcs"
+	v := c.evalState(x, AllArcs)
 
 	v, ok := c.getDefault(v)
 	if !ok {