internal/core/adt: use explicit hole id

Previously, holes were identified by their closeContext.
This worked, but not obviously so. Using explicit hole
numbers will make it clearer to see the code is correct
and also helps interpreting debugging data.

The information is also copied into closeContext for
to allow showing it in the graphical debugger.

Issue #3635

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: I50b481704af3bbd01c558e6d9b2d981061d25052
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1207168
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Matthew Sackman <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>

Author: mpvl

internal/core/adt/conjunct.go

@@ -202,9 +202,11 @@ func (n *nodeContext) scheduleConjunct(c Conjunct, id CloseInfo) {
 
 		// TODO(perf): reuse envDisjunct values so that we can also reuse the
 		// disjunct slice.
+		n.ctx.holeID++
 		d := envDisjunct{
 			env:     env,
 			cloneID: id,
+			holeID:  n.ctx.holeID,
 			src:     x,
 			expr:    x,
 		}
@@ -649,9 +651,11 @@ func (n *nodeContext) insertValueConjunct(env *Environment, v Value, id CloseInf
 		id := id
 		id.setOptionalV3(n)
 
+		n.ctx.holeID++
 		d := envDisjunct{
 			env:     env,
 			cloneID: id,
+			holeID:  n.ctx.holeID,
 			src:     x,
 			value:   x,
 		}

internal/core/adt/context.go

@@ -220,6 +220,10 @@ type OpContext struct {
 	// encountered. A value of 0 indicates we are not within such field.
 	optionalMark int
 
+	// holdID is a unique identifier for the current "hole", a choice of
+	// disjunct to be made when processing disjunctions.
+	holeID int
+
 	// inDisjunct indicates that non-monotonic checks should be skipped.
 	// This is used if we want to do some extra work to eliminate disjunctions
 	// early. The result of unification should be thrown away if this check is

internal/core/adt/debug.go

@@ -623,6 +623,9 @@ func (m *mermaidContext) pstr(cc *closeContext) string {
 	// Show the origin of the closeContext.
 	indentOnNewline(w, 3)
 	fmt.Fprintf(w, "+%d", cc.depth)
+	if cc.holeID != 0 {
+		fmt.Fprintf(w, " H%d", cc.holeID)
+	}
 
 	w.WriteString(close)
 

internal/core/adt/disjunct.go

@@ -87,6 +87,7 @@ import (
 type envDisjunct struct {
 	env     *Environment
 	cloneID CloseInfo
+	holeID  int
 
 	// fields for new evaluator
 

internal/core/adt/disjunct2.go

@@ -209,6 +209,7 @@ type disjunct struct {
 // is relatively rare, we keep it separate to avoid bloating the closeContext.
 type disjunctHole struct {
 	cc         *closeContext
+	holeID     int
 	underlying *closeContext
 }
 
@@ -228,11 +229,13 @@ func (n *nodeContext) scheduleDisjunction(d envDisjunct) {
 	// case mergeVertex will override the original value, or multiple disjuncts,
 	// in which case the original is set to the disjunct itself.
 	ccHole.incDisjunct(n.ctx, DISJUNCT)
+	ccHole.holeID = d.holeID
 
 	n.disjunctions = append(n.disjunctions, d)
 
 	n.disjunctCCs = append(n.disjunctCCs, disjunctHole{
 		cc:         ccHole, // this value is cloned in doDisjunct.
+		holeID:     d.holeID,
 		underlying: ccHole,
 	})
 }
@@ -291,7 +294,7 @@ func (n *nodeContext) processDisjunctions() *Bottom {
 		}
 
 		// Mark no final in nodeContext and observe later.
-		results = n.crossProduct(results, cross, d, mode)
+		results = n.crossProduct(results, cross, d, mode, d.holeID)
 
 		// TODO: do we unwind only at the end or also intermittently?
 		switch len(results) {
@@ -346,7 +349,7 @@ func (n *nodeContext) processDisjunctions() *Bottom {
 
 // crossProduct computes the cross product of the disjuncts of a disjunction
 // with an existing set of results.
-func (n *nodeContext) crossProduct(dst, cross []*nodeContext, dn *envDisjunct, mode runMode) []*nodeContext {
+func (n *nodeContext) crossProduct(dst, cross []*nodeContext, dn *envDisjunct, mode runMode, hole int) []*nodeContext {
 	defer n.unmarkDepth(n.markDepth())
 	defer n.unmarkOptional(n.markOptional())
 
@@ -357,7 +360,7 @@ func (n *nodeContext) crossProduct(dst, cross []*nodeContext, dn *envDisjunct, m
 
 		for j, d := range dn.disjuncts {
 			c := MakeConjunct(dn.env, d.expr, dn.cloneID)
-			r, err := p.doDisjunct(c, d.mode, mode)
+			r, err := p.doDisjunct(c, d.mode, mode, hole)
 
 			if err != nil {
 				// TODO: store more error context
@@ -404,7 +407,7 @@ func (n *nodeContext) collectErrors(dn *envDisjunct) (errs *Bottom) {
 	return b
 }
 
-func (n *nodeContext) doDisjunct(c Conjunct, m defaultMode, mode runMode) (*nodeContext, *Bottom) {
+func (n *nodeContext) doDisjunct(c Conjunct, m defaultMode, mode runMode, hole int) (*nodeContext, *Bottom) {
 	if c.CloseInfo.cc == nil {
 		panic("nil closeContext during init")
 	}
@@ -426,10 +429,13 @@ func (n *nodeContext) doDisjunct(c Conjunct, m defaultMode, mode runMode) (*node
 		// a closeContext corresponding to a disjunction always has a parent.
 		// We therefore do not need to check whether x.parent is nil.
 		o := oc.allocCC(d.cc)
-		if c.CloseInfo.cc == d.underlying {
+		if hole == d.holeID {
 			ccHole = o
+			if d.cc.conjunctCount == 0 {
+				panic("unexpected zero conjunctCount")
+			}
 		}
-		holes = append(holes, disjunctHole{o, d.underlying})
+		holes = append(holes, disjunctHole{o, d.holeID, d.underlying})
 	}
 
 	if ccHole == nil {

internal/core/adt/fields.go

@@ -170,6 +170,10 @@ type closeContext struct {
 	// in disjunction overlays. This is mostly for debugging.
 	generation int
 
+	// a non-zero value indicates that the closeContext is part of a disjunction
+	// and that it is associated with the given Hole Index.
+	holeID int
+
 	dependencies []*ccDep // For testing only. See debug.go
 
 	// externalDeps lists the closeContexts associated with a root node for

internal/core/adt/overlay.go

@@ -270,6 +270,7 @@ func (ctx *overlayContext) allocCC(cc *closeContext) *closeContext {
 	o := &closeContext{generation: ctx.generation}
 	cc.overlay = o
 	o.depth = cc.depth
+	o.holeID = cc.holeID
 
 	if cc.parent != nil {
 		o.parent = ctx.allocCC(cc.parent)