From 785114246b26e673a375c40094ea5d01932978bd Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Mon, 20 Mar 2023 12:46:08 +0100 Subject: [PATCH] internal/core/adt: unexport Unify MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We are moving away from using states. Replace Unify with alternative methods. The CompleteArcs method has been introduced to deal with the case where only the arc set needs to be known, not a full evaluation. The unify calls have also been updated for code internal to adt if this code is expected to be sharable with the new evaluator. Signed-off-by: Marcel van Lohuizen Change-Id: I9f3143dda4c04e556473e4487c85e65e1dfddba1 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/551408 Reviewed-by: Aram Hăvărneanu Reviewed-by: Roger Peppe Unity-Result: CUEcueckoo TryBot-Result: CUEcueckoo --- cue/types.go | 4 ++-- internal/core/adt/binop.go | 4 ++-- internal/core/adt/composite.go | 11 ++++++++--- internal/core/adt/context.go | 10 +++++----- internal/core/adt/eval.go | 16 +++++++++------- internal/core/adt/expr.go | 18 +++++++++--------- internal/core/compile/builtin.go | 2 +- internal/core/eval/eval.go | 6 +----- internal/core/validate/validate_test.go | 2 +- pkg/internal/context.go | 2 +- pkg/list/sort.go | 2 +- 11 files changed, 40 insertions(+), 37 deletions(-) diff --git a/cue/types.go b/cue/types.go index 89f3f5e26d4..332373ae0e6 100644 --- a/cue/types.go +++ b/cue/types.go @@ -2368,8 +2368,8 @@ func (v Value) Expr() (Op, []Value) { b.AddConjunct(adt.MakeRootConjunct(env, disjunct.Val)) ctx := eval.NewContext(v.idx, nil) - ctx.Unify(&a, adt.Finalized) - ctx.Unify(&b, adt.Finalized) + a.Finalize(ctx) + b.Finalize(ctx) if allowed(ctx, v.v, &b) != nil { // Everything subsumed bottom continue outerExpr diff --git a/internal/core/adt/binop.go b/internal/core/adt/binop.go index 556e189c856..1bee779e333 100644 --- a/internal/core/adt/binop.go +++ b/internal/core/adt/binop.go @@ -207,7 +207,7 @@ func BinOp(c *OpContext, op Op, left, right Value) Value { n := &Vertex{} n.AddConjunct(MakeConjunct(c.Env(0), list, c.ci)) - c.Unify(n, Conjuncts) + n.CompleteArcs(c) return n } @@ -269,7 +269,7 @@ func BinOp(c *OpContext, op Op, left, right Value) Value { n := &Vertex{} n.AddConjunct(MakeConjunct(c.Env(0), list, c.ci)) - c.Unify(n, Conjuncts) + n.CompleteArcs(c) return n } diff --git a/internal/core/adt/composite.go b/internal/core/adt/composite.go index 40670b0e5a1..cb5bdc20c9e 100644 --- a/internal/core/adt/composite.go +++ b/internal/core/adt/composite.go @@ -256,7 +256,7 @@ func (v *Vertex) IsDefined(c *OpContext) bool { if v.isDefined() { return true } - c.Unify(v, Finalized) + v.Finalize(c) return v.isDefined() } @@ -566,7 +566,7 @@ func (v *Vertex) IsErr() bool { } func (v *Vertex) Err(c *OpContext, state VertexStatus) *Bottom { - c.Unify(v, state) + c.unify(v, state) if b, ok := v.BaseValue.(*Bottom); ok { return b } @@ -580,10 +580,15 @@ func (v *Vertex) Finalize(c *OpContext) { // case the caller did not handle existing errors in the context. err := c.errs c.errs = nil - c.Unify(v, Finalized) + c.unify(v, Finalized) c.errs = err } +// CompleteArcs ensures the set of arcs has been computed. +func (v *Vertex) CompleteArcs(c *OpContext) { + c.unify(v, Conjuncts) +} + func (v *Vertex) AddErr(ctx *OpContext, b *Bottom) { v.SetValue(ctx, Finalized, CombineErrors(nil, v.Value(), b)) } diff --git a/internal/core/adt/context.go b/internal/core/adt/context.go index ce98a8ef47b..99c86bc3230 100644 --- a/internal/core/adt/context.go +++ b/internal/core/adt/context.go @@ -320,7 +320,7 @@ func (c *OpContext) Env(upCount int32) *Environment { func (c *OpContext) relNode(upCount int32) *Vertex { e := c.e.up(upCount) - c.Unify(e.Vertex, Partial) + c.unify(e.Vertex, Partial) return e.Vertex } @@ -779,7 +779,7 @@ func (c *OpContext) unifyNode(v Expr, state VertexStatus) (result Value) { } if v.isUndefined() || state > v.status { - c.Unify(v, state) + c.unify(v, state) } return v @@ -859,9 +859,9 @@ func (c *OpContext) lookup(x *Vertex, pos token.Pos, l Feature, state VertexStat // hasAllConjuncts, but that are finalized too early, get conjuncts // processed beforehand. if state > a.status { - c.Unify(a, state) + c.unify(a, state) } else if a.state != nil { - c.Unify(a, Partial) + c.unify(a, Partial) } if a.IsConstraint() { @@ -1329,6 +1329,6 @@ func (c *OpContext) NewList(values ...Value) *Vertex { for _, x := range values { list.Elems = append(list.Elems, x) } - c.Unify(v, Finalized) + v.Finalize(c) return v } diff --git a/internal/core/adt/eval.go b/internal/core/adt/eval.go index ab42f9ed105..3c26576b5b9 100644 --- a/internal/core/adt/eval.go +++ b/internal/core/adt/eval.go @@ -76,7 +76,7 @@ var incompleteSentinel = &Bottom{ func (c *OpContext) evaluate(v *Vertex, r Resolver, state VertexStatus) Value { if v.isUndefined() { // Use node itself to allow for cycle detection. - c.Unify(v, state) + c.unify(v, state) if v.ArcType == ArcVoid { if v.status == Evaluating { @@ -145,10 +145,12 @@ func (c *OpContext) evaluate(v *Vertex, r Resolver, state VertexStatus) Value { return v } -// Unify fully unifies all values of a Vertex to completion and stores -// the result in the Vertex. If unify was called on v before it returns -// the cached results. -func (c *OpContext) Unify(v *Vertex, state VertexStatus) { +// unify unifies values of a Vertex to and stores the result in the Vertex. If +// unify was called on v before it returns the cached results. +// state can be used to indicate to which extent processing should continue. +// state == finalized means it is evaluated to completion. See vertexStatus +// for more details. +func (c *OpContext) unify(v *Vertex, state VertexStatus) { // defer c.PopVertex(c.PushVertex(v)) if Debug { c.nest++ @@ -752,7 +754,7 @@ func (n *nodeContext) completeArcs(state VertexStatus) { wasVoid := a.ArcType == ArcVoid - ctx.Unify(a, Finalized) + ctx.unify(a, Finalized) if a.ArcType == ArcVoid { continue @@ -1619,7 +1621,7 @@ func (n *nodeContext) addVertexConjuncts(c Conjunct, arc *Vertex, inline bool) { // is necessary to prevent lookups in unevaluated structs. // TODO(cycles): this can probably most easily be fixed with a // having a more recursive implementation. - n.ctx.Unify(arc, Partial) + n.ctx.unify(arc, Partial) } // Don't add conjuncts if a node is referring to itself. diff --git a/internal/core/adt/expr.go b/internal/core/adt/expr.go index ef2ec5757fd..279c8a1a90e 100644 --- a/internal/core/adt/expr.go +++ b/internal/core/adt/expr.go @@ -82,7 +82,7 @@ func (x *StructLit) evaluate(c *OpContext, state VertexStatus) Value { // used in a context where more conjuncts are added. It may also lead // to disjuncts being in a partially expanded state, leading to // misaligned nodeContexts. - c.Unify(v, Conjuncts) + v.CompleteArcs(c) return v } @@ -292,7 +292,7 @@ func (x *ListLit) evaluate(c *OpContext, state VertexStatus) Value { IsDynamic: true, Conjuncts: []Conjunct{{e, x, c.ci}}, } - c.Unify(v, Conjuncts) + v.CompleteArcs(c) return v } @@ -900,7 +900,7 @@ func (x *LetReference) resolve(ctx *OpContext, state VertexStatus) *Vertex { // now, though, as it is not entirely clear it is fine to remove this. // We can reevaluate this once we have redone some of the planned order of // evaluation work. - ctx.Unify(arc, Finalized) + arc.Finalize(ctx) b, ok := arc.BaseValue.(*Bottom) if !arc.MultiLet && !ok { return arc @@ -1252,9 +1252,9 @@ func (x *BinaryExpr) evaluate(c *OpContext, state VertexStatus) Value { // evaluate the struct regardless to ensure that cycle reporting // keeps working. if env.Vertex.IsDynamic || c.inValidator > 0 { - c.Unify(v, Finalized) + v.Finalize(c) } else { - c.Unify(v, Conjuncts) + v.CompleteArcs(c) } return v @@ -1340,7 +1340,7 @@ func (c *OpContext) validate(env *Environment, src ast.Node, x Expr, op Op, stat // - walk over all fields and verify that fields are not contradicting // previously marked fields. // - c.Unify(v, Finalized) + v.Finalize(c) if v.status == EvaluatingArcs { // We have a cycle, which may be an error. Cycle errors may occur @@ -1606,7 +1606,7 @@ func (x *Builtin) call(c *OpContext, p token.Pos, validate bool, args []Value) E IsDynamic: true, Conjuncts: []Conjunct{{env, x, c.ci}}, } - c.Unify(n, Finalized) + n.Finalize(c) if _, ok := n.BaseValue.(*Bottom); ok { c.addErrf(0, pos(a), "cannot use %s as %s in argument %d to %s", @@ -1741,7 +1741,7 @@ func (x *DisjunctionExpr) Source() ast.Node { func (x *DisjunctionExpr) evaluate(c *OpContext, state VertexStatus) Value { e := c.Env(0) v := &Vertex{Conjuncts: []Conjunct{{e, x, c.ci}}} - c.Unify(v, Finalized) // TODO: also partial okay? + v.Finalize(c) // TODO: also partial okay? // TODO: if the disjunction result originated from a literal value, we may // consider the result closed to create more permanent errors. return v @@ -1881,7 +1881,7 @@ func (x *ForClause) yield(s *compState) { continue } - c.Unify(a, Partial) + c.unify(a, Partial) if a.ArcType == ArcVoid { continue } diff --git a/internal/core/compile/builtin.go b/internal/core/compile/builtin.go index 41b13fcfc83..85cb5e95590 100644 --- a/internal/core/compile/builtin.go +++ b/internal/core/compile/builtin.go @@ -143,7 +143,7 @@ var orBuiltin = &adt.Builtin{ &adt.DisjunctionExpr{Values: d, HasDefaults: false}, closeInfo, )) - c.Unify(v, adt.Conjuncts) + v.CompleteArcs(c) return v }, } diff --git a/internal/core/eval/eval.go b/internal/core/eval/eval.go index 5e26fc63782..8266bdf04b7 100644 --- a/internal/core/eval/eval.go +++ b/internal/core/eval/eval.go @@ -28,7 +28,7 @@ func Evaluate(r adt.Runtime, v *adt.Vertex) { Runtime: r, Format: format, }) - c.Unify(v, adt.Finalized) + v.Finalize(c) } func New(r adt.Runtime) *Unifier { @@ -40,10 +40,6 @@ type Unifier struct { e *adt.OpContext } -func (e *Unifier) Unify(ctx *adt.OpContext, v *adt.Vertex, state adt.VertexStatus) { - e.e.Unify(v, state) -} - func (e *Unifier) Stats() *stats.Counts { return e.e.Stats() } diff --git a/internal/core/validate/validate_test.go b/internal/core/validate/validate_test.go index 6928e49e870..db82b103466 100644 --- a/internal/core/validate/validate_test.go +++ b/internal/core/validate/validate_test.go @@ -196,7 +196,7 @@ y: conflicting values 4 and 2: if err != nil { t.Fatal(err) } - ctx.Unify(v, adt.Finalized) + v.Finalize(ctx) if tc.lookup != "" { v = v.Lookup(adt.MakeIdentLabel(r, tc.lookup, "main")) } diff --git a/pkg/internal/context.go b/pkg/internal/context.go index 6589911905b..cf086670b15 100644 --- a/pkg/internal/context.go +++ b/pkg/internal/context.go @@ -62,7 +62,7 @@ func (c *CallCtxt) Struct(i int) Struct { x := c.args[i] switch v, ok := x.(*adt.Vertex); { case ok && !v.IsList(): - c.ctx.Unify(v, adt.Conjuncts) + v.CompleteArcs(c.ctx) return Struct{c.ctx, v} case v != nil: diff --git a/pkg/list/sort.go b/pkg/list/sort.go index e963800c3cf..b61db6823b8 100644 --- a/pkg/list/sort.go +++ b/pkg/list/sort.go @@ -112,7 +112,7 @@ func makeValueSorter(list []cue.Value, cmp cue.Value) (s valueSorter) { Parent: v.V.Parent, Conjuncts: v.V.Conjuncts, } - ctx.Unify(n, adt.Conjuncts) + n.CompleteArcs(ctx) s = valueSorter{ a: list,