internal/core/adt: first implementation of disjunctions

This is an incomplete implementation. The following mechanisms
are still TODO:
- cross product of disjunctions
- inheriting tasks from "forked" nodeContexts (this is especially
  relavant for list values).
- efficient dedupping of incomplete disjuncts
- efficient memory management
- "quick check" fast paths

The main algorithm is implemented in disjunct2.go and overlay.go.
See a description of the algorith in disjuncts2.go.

Tests are enabled in a follow-up CL.

Issue #2884
Issue #2851

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

cue/testdata/eval/insertion.txtar

@@ -258,16 +258,15 @@ Disjuncts:    73
         y: (int){ 5 }
       }
       v: (int){ 1 }
-      w: (int){ 2 }
       x: (int){ 2 }
       y: (int){ 5 }
+      w: (int){ 2 }
     }
   }
   recursive: (struct){
     t1: (struct){
       e: (struct){
         f: (int){ 1 }
-        g: (int){ 1 }
       }
       c: (struct){
         d: (struct){
@@ -292,7 +291,6 @@ Disjuncts:    73
         }
         g: (int){ 1 }
       }
-      g: (int){ 1 }
     }
   }
   embeddedFunctionalExpr: (string){
@@ -308,19 +306,39 @@ Disjuncts:    73
 diff old new
 --- old
 +++ new
-@@ -47,10 +47,10 @@
+@@ -47,9 +47,9 @@
          v: (int){ 1 }
          y: (int){ 5 }
        }
--      x: (int){ 2 }
--      y: (int){ 5 }
-       v: (int){ 1 }
++      v: (int){ 1 }
+       x: (int){ 2 }
+       y: (int){ 5 }
+-      v: (int){ 1 }
        w: (int){ 2 }
-+      x: (int){ 2 }
-+      y: (int){ 5 }
      }
    }
-   recursive: (struct){
+@@ -57,7 +57,6 @@
+     t1: (struct){
+       e: (struct){
+         f: (int){ 1 }
+-        g: (int){ 1 }
+       }
+       c: (struct){
+         d: (struct){
+@@ -82,7 +81,6 @@
+         }
+         g: (int){ 1 }
+       }
+-      g: (int){ 1 }
+     }
+   }
+   embeddedFunctionalExpr: (string){
+-- diff/todo/p0 --
+Dropped fields: this is a result of registering for notification after
+inserting conjuncts, instead of before it. This change was made to avoid
+forcing adding a level of conjunction insertion unnecessarily. To fix this
+we should uncouple adding notification from insertion.
+See scheduleVertexConjuncts in conjuncts.go.
 -- diff/todo/p3 --
 Reordering.
 -- out/compile --

cue/testdata/eval/v0.7.txtar

@@ -913,7 +913,6 @@ Result:
         }
         e: (struct){
           f: (int){ 1 }
-          g: (int){ 1 }
         }
       }
     }
@@ -925,8 +924,8 @@ Result:
               a: (int){ 1 }
               b: (int){ 2 }
             }
-            b: (int){ 2 }
             a: (int){ 1 }
+            b: (int){ 2 }
           }
           x: (struct){
             b: (struct){
@@ -1025,8 +1024,8 @@ Result:
               a: (int){ 1 }
               b: (int){ 2 }
             }
-            b: (int){ 2 }
             a: (int){ 1 }
+            b: (int){ 2 }
           }
           x: (struct){
             b: (struct){
@@ -1220,10 +1219,14 @@ diff old new
      t0: (struct){
        ok: (struct){
          c: (struct){
-@@ -264,134 +248,130 @@
+@@ -260,138 +244,133 @@
          }
-       }
-     }
+         e: (struct){
+           f: (int){ 1 }
+-          g: (int){ 1 }
+-        }
+-      }
+-    }
 -    t4: (_|_){
 -      // [eval]
 -      ok: (_|_){
@@ -1352,6 +1355,9 @@ diff old new
 -          y: (struct){
 -            c: (struct){
 -              a: (int){ 1 }
++        }
++      }
++    }
 +    t4: (struct){
 +      ok: (struct){
 +        p0: (struct){
@@ -1360,8 +1366,8 @@ diff old new
 +              a: (int){ 1 }
 +              b: (int){ 2 }
 +            }
-+            b: (int){ 2 }
 +            a: (int){ 1 }
++            b: (int){ 2 }
 +          }
 +          x: (struct){
 +            b: (struct){
@@ -1460,8 +1466,8 @@ diff old new
 +              a: (int){ 1 }
 +              b: (int){ 2 }
 +            }
-+            b: (int){ 2 }
 +            a: (int){ 1 }
++            b: (int){ 2 }
 +          }
 +          x: (struct){
 +            b: (struct){
@@ -1479,7 +1485,7 @@ diff old new
              }
            }
          }
-@@ -401,23 +381,23 @@
+@@ -401,23 +380,23 @@
                d: (struct){
                  b: (int){ 2 }
                }
@@ -1522,6 +1528,12 @@ diff old new
          check: (struct){
 -- diff/explanation --
 Bug fixes
+-- diff/todo/p0 --
+Dropped fields: this is a result of registering for notification after
+inserting conjuncts, instead of before it. This change was made to avoid
+forcing adding a level of conjunction insertion unnecessarily. To fix this
+we should uncouple adding notification from insertion.
+See scheduleVertexConjuncts in conjuncts.go.
 -- diff/todo/p2 --
 Reordering / positions.
 -- out/compile --

internal/core/adt/binop.go

@@ -208,6 +208,13 @@ func BinOp(c *OpContext, op Op, left, right Value) Value {
 			n := c.newInlineVertex(nil, nil, MakeConjunct(c.Env(0), list, c.ci))
 			n.CompleteArcs(c)
 
+			// NOTE: if we set isData to true, whoever processes the result will
+			// avoid having to process the expressions again. This improves
+			// performance. It also change the a potential cycle error message
+			// to a more concrete messages as if this result was unified as is.
+			// TODO: uncomment this and see if we like the result.
+			// n.isData = true
+
 			return n
 		}
 

internal/core/adt/composite.go

@@ -249,7 +249,7 @@ type Vertex struct {
 func (v *Vertex) rootCloseContext() *closeContext {
 	if v.cc == nil {
 		v.cc = &closeContext{
-			group:           &ConjunctGroup{},
+			group:           (*ConjunctGroup)(&v.Conjuncts),
 			parent:          nil,
 			src:             v,
 			parentConjuncts: v,

internal/core/adt/conjunct.go

@@ -66,7 +66,7 @@ func (n *nodeContext) scheduleConjunct(c Conjunct, id CloseInfo) {
 			id.cc = n.node.rootCloseContext()
 		}
 		if id.cc == cc {
-			panic("inconsistent state")
+			panic("inconsistent state: same closeContext")
 		}
 		var t closeNodeType
 		if c.CloseInfo.FromDef {
@@ -84,7 +84,7 @@ func (n *nodeContext) scheduleConjunct(c Conjunct, id CloseInfo) {
 		}
 
 		if id.cc.src != n.node {
-			panic("inconsistent state")
+			panic("inconsistent state: nodes differ")
 		}
 	default:
 
@@ -163,8 +163,22 @@ func (n *nodeContext) scheduleConjunct(c Conjunct, id CloseInfo) {
 		n.scheduleTask(handleListLit, env, x, id)
 
 	case *DisjunctionExpr:
-		panic("unimplemented")
-		// n.addDisjunction(env, x, id)
+		// TODO(perf): reuse envDisjunct values so that we can also reuse the
+		// disjunct slice.
+		d := envDisjunct{
+			env:     env,
+			cloneID: id,
+			src:     x,
+			expr:    x,
+		}
+		for _, dv := range x.Values {
+			d.disjuncts = append(d.disjuncts, disjunct{
+				expr:      dv.Val,
+				isDefault: dv.Default,
+				mode:      mode(x.HasDefaults, dv.Default),
+			})
+		}
+		n.scheduleDisjunction(d)
 
 	case *Comprehension:
 		// always a partial comprehension.
@@ -351,21 +365,39 @@ func (n *nodeContext) scheduleVertexConjuncts(c Conjunct, arc *Vertex, closeInfo
 		defer dc.decDependent(n.ctx, DEFER, nil)
 	}
 
-	if state := arc.getState(n.ctx); state != nil {
-		state.addNotify2(n.node, closeInfo)
-	}
-
-	for i := 0; i < len(arc.Conjuncts); i++ {
-		c := arc.Conjuncts[i]
-
-		// Note that we are resetting the tree here. We hereby assume that
-		// closedness conflicts resulting from unifying the referenced arc were
-		// already caught there and that we can ignore further errors here.
-		// c.CloseInfo = closeInfo
+	// TODO: need to split initialization phases to allow only initializing
+	// notifications without evaluating the node. This will be done later.
+	//
+	// if !n.node.nonRooted || n.node.IsDynamic {
+	// 	if state := arc.getState(n.ctx); state != nil {
+	// 		// n.completeNodeTasks()
+	// 	}
+	// }
+
+	if d, ok := arc.BaseValue.(*Disjunction); ok && false {
+		n.scheduleConjunct(MakeConjunct(c.Env, d, closeInfo), closeInfo)
+	} else {
+		for i := 0; i < len(arc.Conjuncts); i++ {
+			c := arc.Conjuncts[i]
+
+			// Note that we are resetting the tree here. We hereby assume that
+			// closedness conflicts resulting from unifying the referenced arc were
+			// already caught there and that we can ignore further errors here.
+			// c.CloseInfo = closeInfo
+
+			// We can use the original, but we know it will not be used
+
+			n.scheduleConjunct(c, closeInfo)
+		}
 
-		// We can use the original, but we know it will not be used
+	}
 
-		n.scheduleConjunct(c, closeInfo)
+	if !n.node.nonRooted || n.node.IsDynamic {
+		// TODO: Make a distinction between initializing state and just allowing
+		// notifications to be registered.
+		if state := arc.getState(n.ctx); state != nil {
+			state.addNotify2(n.node, closeInfo)
+		}
 	}
 }
 
@@ -482,7 +514,22 @@ func (n *nodeContext) insertValueConjunct(env *Environment, v Value, id CloseInf
 
 	switch x := v.(type) {
 	case *Disjunction:
-		n.addDisjunctionValue(env, x, id)
+		// TODO(perf): reuse envDisjunct values so that we can also reuse the
+		// disjunct slice.
+		d := envDisjunct{
+			env:     env,
+			cloneID: id,
+			src:     x,
+			value:   x,
+		}
+		for i, dv := range x.Values {
+			d.disjuncts = append(d.disjuncts, disjunct{
+				expr:      dv,
+				isDefault: i < x.NumDefaults,
+				mode:      mode(x.HasDefaults, i < x.NumDefaults),
+			})
+		}
+		n.scheduleDisjunction(d)
 
 	case *Conjunction:
 		for _, x := range x.Values {

internal/core/adt/debug.go

@@ -142,6 +142,9 @@ const (
 	// TASK dependencies are used to track the completion of a task.
 	TASK
 
+	// DISJUNCT is used to mark an incomplete disjunct.
+	DISJUNCT
+
 	// EVAL tracks that the conjunct associated with a closeContext has been
 	// inserted using scheduleConjunct. A closeContext may not be deleted
 	// as long as the conjunct has not been evaluated yet.
@@ -175,6 +178,8 @@ func (k depKind) String() string {
 		return "NOTIFY"
 	case TASK:
 		return "TASK"
+	case DISJUNCT:
+		return "DISJUNCT"
 	case EVAL:
 		return "EVAL"
 	case ROOT:
@@ -470,7 +475,7 @@ func (m *mermaidContext) task(d *ccDep) string {
 		fmt.Fprintf(vc.tasks, "subgraph %s_tasks[tasks]\n", m.vertexID(v))
 	}
 
-	if v != d.task.node.node {
+	if d.task != nil && v != d.task.node.node {
 		panic("inconsistent task")
 	}
 	taskID := fmt.Sprintf("%s_%d", m.vertexID(v), d.taskID)
@@ -539,14 +544,17 @@ func (m *mermaidContext) cc(cc *closeContext) {
 				return
 			}
 			fallthrough
-		case ARC, NOTIFY:
+		case ARC, NOTIFY, DISJUNCT:
 			w = global
 			indentLevel = 1
 			name = m.pstr(d.dependency)
 
 		case TASK:
 			w = node
-			taskID := m.task(d)
+			taskID := "disjunct"
+			if d.task != nil {
+				taskID = m.task(d)
+			}
 			name = fmt.Sprintf("%s((%d))", taskID, d.taskID)
 		case ROOT, INIT:
 			w = node
@@ -614,7 +622,7 @@ func (m *mermaidContext) pstr(cc *closeContext) string {
 	w.WriteString(open)
 	w.WriteString("cc")
 	if cc.conjunctCount > 0 {
-		fmt.Fprintf(w, " c:%d", cc.conjunctCount)
+		fmt.Fprintf(w, " c:%d: d:%d", cc.conjunctCount, cc.disjunctCount)
 	}
 	indentOnNewline(w, 3)
 	w.WriteString(ptr)
@@ -634,7 +642,14 @@ func (m *mermaidContext) pstr(cc *closeContext) string {
 
 	w.WriteString(close)
 
-	if cc.conjunctCount > 0 {
+	switch {
+	case cc.conjunctCount == 0:
+	case cc.conjunctCount <= cc.disjunctCount:
+		// TODO: Extra checks for disjunctions?
+		// E.g.: cc.src is not a disjunction
+	default:
+		// If cc.conjunctCount > cc.disjunctCount.
+		// TODO: count the number of non-decremented DISJUNCT dependencies.
 		fmt.Fprintf(w, ":::err")
 		if cc.src == m.v {
 			m.hasError = true