internal/core/adt: use setBaseValue to set Vertex.BaseValue

Having a single point of entry through which BaseValue
is set helps debugging.

evalv2 also needed to update the status while setting
this value. This is no longer necessary for evalv3. To
keep things simple, we only ensure that for evalv3 code
uses this entry point.

Note that the method is defined on nodeContext. This
additionally enforces that a modification can only be
made while a Vertex is actively being evaluated.

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: I3f10a1e47309a2808f873806c7625dc7c7012247

internal/core/adt/composite.go

@@ -851,6 +851,18 @@ func (v *Vertex) setValue(ctx *OpContext, state vertexStatus, value BaseValue) *
 	return nil
 }
 
+func (n *nodeContext) setBaseValue(value BaseValue) {
+	n.node.BaseValue = value
+}
+
+// swapBaseValue swaps the BaseValue of a node with the given value and returns
+// the previous value.
+func (n *nodeContext) swapBaseValue(value BaseValue) (saved BaseValue) {
+	saved = n.node.BaseValue
+	n.setBaseValue(value)
+	return saved
+}
+
 // ToVertex wraps v in a new Vertex, if necessary.
 func ToVertex(v Value) *Vertex {
 	switch x := v.(type) {

internal/core/adt/context.go

@@ -753,7 +753,7 @@ func (c *OpContext) evalState(v Expr, state combinedFlags) (result Value) {
 						}
 						err := c.Newf("cycle with field %v", x)
 						b := &Bottom{Code: CycleError, Err: err}
-						v.setValue(c, v.status, b)
+						s.setBaseValue(b)
 						return b
 						// TODO: use this instead, as is usual for incomplete errors,
 						// and also move this block one scope up to also apply to

internal/core/adt/cycle.go

@@ -611,14 +611,14 @@ func assertStructuralCycle(n *nodeContext) bool {
 }
 
 func (n *nodeContext) reportCycleError() {
-	n.node.BaseValue = CombineErrors(nil,
+	n.setBaseValue(CombineErrors(nil,
 		n.node.Value(),
 		&Bottom{
 			Code:  StructuralCycleError,
 			Err:   n.ctx.Newf("structural cycle"),
 			Value: n.node.Value(),
 			// TODO: probably, this should have the referenced arc.
-		})
+		}))
 	n.node.Arcs = nil
 }
 

internal/core/adt/disjunct.go

@@ -210,7 +210,7 @@ func (n *nodeContext) expandDisjuncts(
 			n.disjuncts = append(n.disjuncts, n)
 		}
 		if n.node.BaseValue == nil {
-			n.node.BaseValue = n.getValidators(state)
+			n.setBaseValue(n.getValidators(state))
 		}
 
 		n.usedDefault = append(n.usedDefault, defaultInfo{

internal/core/adt/disjunct2.go

@@ -317,7 +317,7 @@ func (n *nodeContext) processDisjunctions() *Bottom {
 
 	case 1:
 		d := cross[0].node
-		n.node.BaseValue = d
+		n.setBaseValue(d)
 		n.defaultMode = cross[0].defaultMode
 
 	default:
@@ -419,9 +419,7 @@ func (n *nodeContext) doDisjunct(c Conjunct, m defaultMode, mode runMode) (*node
 
 	v := d.node
 
-	saved := n.node.BaseValue
-	n.node.BaseValue = v
-	defer func() { n.node.BaseValue = saved }()
+	defer n.setBaseValue(n.swapBaseValue(v))
 
 	// Clear relevant scheduler states.
 	// TODO: do something more principled: just ensure that a node that has
@@ -487,7 +485,7 @@ func (n *nodeContext) finalizeDisjunctions() {
 	}
 
 	v := n.node
-	v.BaseValue = d
+	n.setBaseValue(d)
 
 	// The conjuncts will have too much information. Better have no
 	// information than incorrect information.

internal/core/adt/errors.go

@@ -131,21 +131,22 @@ func isIncomplete(v *Vertex) bool {
 //
 // If x is not already an error, the value is recorded in the error for
 // reference.
-func (v *Vertex) AddChildError(recursive *Bottom) {
+func (n *nodeContext) AddChildError(recursive *Bottom) {
+	v := n.node
 	v.ChildErrors = CombineErrors(nil, v.ChildErrors, recursive)
 	if recursive.IsIncomplete() {
 		return
 	}
 	x := v.BaseValue
 	err, _ := x.(*Bottom)
 	if err == nil {
-		v.BaseValue = &Bottom{
+		n.setBaseValue(&Bottom{
 			Code:         recursive.Code,
 			Value:        v,
 			HasRecursive: true,
 			ChildError:   true,
 			Err:          recursive.Err,
-		}
+		})
 		return
 	}
 
@@ -154,7 +155,7 @@ func (v *Vertex) AddChildError(recursive *Bottom) {
 		err.Code = recursive.Code
 	}
 
-	v.BaseValue = err
+	n.setBaseValue(err)
 }
 
 // CombineErrors combines two errors that originate at the same Vertex.

internal/core/adt/eval.go

@@ -823,7 +823,7 @@ func (n *nodeContext) completeArcs(state vertexStatus) {
 				}
 
 				if err := a.Bottom(); err != nil {
-					n.node.AddChildError(err)
+					n.AddChildError(err)
 				}
 			}
 

internal/core/adt/tasks.go

@@ -332,7 +332,7 @@ func (n *nodeContext) updateListType(list Expr, id CloseInfo, isClosed bool, ell
 		m = &ListMarker{
 			IsOpen: true,
 		}
-		n.node.setValue(n.ctx, conjuncts, m)
+		n.setBaseValue(m)
 	}
 	m.IsOpen = m.IsOpen && !isClosed
 

internal/core/adt/unify.go

@@ -260,7 +260,7 @@ func (v *Vertex) unify(c *OpContext, needs condition, mode runMode) bool {
 		if b := n.node.Bottom(); b != nil {
 			err = CombineErrors(nil, b, err)
 		}
-		n.node.BaseValue = err
+		n.setBaseValue(err)
 	}
 
 	if mode == attemptOnly {
@@ -434,7 +434,7 @@ func (n *nodeContext) updateScalar() {
 	// Set BaseValue to scalar, but only if it was not set before. Most notably,
 	// errors should not be discarded.
 	if n.scalar != nil && (!n.node.IsErr() || isCyclePlaceholder(n.node.BaseValue)) {
-		n.node.BaseValue = n.scalar
+		n.setBaseValue(n.scalar)
 		n.signal(scalarKnown)
 	}
 }
@@ -521,7 +521,7 @@ func (n *nodeContext) completeAllArcs(needs condition, mode runMode) bool {
 		if !a.Label.IsLet() && a.ArcType <= ArcRequired {
 			a := a.DerefValue()
 			if err := a.Bottom(); err != nil {
-				n.node.AddChildError(err)
+				n.AddChildError(err)
 			}
 			success = true // other arcs are irrelevant
 		}