Skip to content

Commit

Permalink
internal/core/eval: allow closedness override
Browse files Browse the repository at this point in the history
The old implementation was a bit too permissive handling
closedness in the API. The current implementation tracks
closedness status within the Vertex.

Some packages take apart the conjuncts of a value and then
reassemblem then using Unify. In this case, the closedness
information is invalidated. The added API allows passing the
closedness information of the original node to exactly specify
what is is allowed.

Change-Id: I63f177dcb8ce70b428a02649641b96163e257166
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/6648
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
  • Loading branch information
mpvl committed Jul 23, 2020
1 parent b68adfe commit d15def3
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 10 deletions.
26 changes: 26 additions & 0 deletions internal/core/eval/closed.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,32 @@ func (a *acceptor) OptionalTypes() (mask adt.OptionalType) {
return mask
}

// A disjunction acceptor represents a disjunction of all possible fields. Note
// that this is never used in evaluation as evaluation stops at incomplete nodes
// and a disjunction is incomplete. When the node is referenced, the original
// conjuncts are used instead.
//
// The value may be used in the API, though, where it may be an argument to
// UnifyAccept.
//
// TODO(perf): it would be sufficient to only implement the Accept method of an
// Acceptor. This could be implemented as an allocation-free wrapper type around
// a Disjunction. This will require a bit more API cleaning, though.
func newDisjunctionAcceptor(x *adt.Disjunction) adt.Acceptor {
tree := &CloseDef{}
sets := []fieldSet{}
for _, d := range x.Values {
if a, _ := d.Closed.(*acceptor); a != nil {
sets = append(sets, a.fields...)
tree.List = append(tree.List, a.tree)
}
}
if len(tree.List) == 0 && len(sets) == 0 {
return nil
}
return &acceptor{tree: tree, fields: sets}
}

// CloseDef defines how individual FieldSets (corresponding to conjuncts)
// combine to determine whether a field is contained in a closed set.
//
Expand Down
44 changes: 34 additions & 10 deletions internal/core/eval/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func (e *Evaluator) Evaluate(c *adt.OpContext, v *adt.Vertex) adt.Value {
if v.Value == nil {
save := *v
// Use node itself to allow for cycle detection.
s := e.evalVertex(c, v, adt.Partial)
s := e.evalVertex(c, v, adt.Partial, nil)

if d := s.disjunct; d != nil && len(d.Values) > 1 && d.NumDefaults != 1 {
v.Value = d
Expand Down Expand Up @@ -239,6 +239,19 @@ func (e *Evaluator) Evaluate(c *adt.OpContext, v *adt.Vertex) adt.Value {
// Phase two: record incomplete
// Phase three: record cycle.
func (e *Evaluator) Unify(c *adt.OpContext, v *adt.Vertex, state adt.VertexStatus) {
e.UnifyAccept(c, v, state, nil) //v.Closed)
}

// UnifyAccept is like Unify, but takes an extra argument to override the
// accepted set of fields.
//
// Instead of deriving the allowed set of fields, it verifies this set by
// consulting the given Acceptor. This can be useful when splitting an existing
// values into individual conjuncts and then unifying some of its components
// back into a new value. Under normal circumstances, this may not always
// succeed as the missing context may result in stricter closedness rules.
func (e *Evaluator) UnifyAccept(c *adt.OpContext, v *adt.Vertex, state adt.VertexStatus, accept adt.Acceptor) {

// defer c.PopVertex(c.PushVertex(v))

if state <= v.Status()+1 {
Expand All @@ -252,7 +265,7 @@ func (e *Evaluator) Unify(c *adt.OpContext, v *adt.Vertex, state adt.VertexStatu
return
}

n := e.evalVertex(c, v, state)
n := e.evalVertex(c, v, state, accept)

switch d := n.disjunct; {
case d != nil && len(d.Values) == 1:
Expand Down Expand Up @@ -282,6 +295,7 @@ func (e *Evaluator) Unify(c *adt.OpContext, v *adt.Vertex, state adt.VertexStatu
}
v.Arcs = nil
// v.Structs = nil // TODO: should we keep or discard the Structs?
v.Closed = newDisjunctionAcceptor(n.disjunct)

default:
if r := n.result(); r.Value != nil {
Expand All @@ -292,7 +306,7 @@ func (e *Evaluator) Unify(c *adt.OpContext, v *adt.Vertex, state adt.VertexStatu
// Else set it to something.

if v.Value == nil {
panic("errer")
panic("error")
}

// Check whether result is done.
Expand All @@ -301,13 +315,13 @@ func (e *Evaluator) Unify(c *adt.OpContext, v *adt.Vertex, state adt.VertexStatu
// evalVertex computes the vertex results. The state indicates the minimum
// status to which this vertex should be evaluated. It should be either
// adt.Finalized or adt.Partial.
func (e *Evaluator) evalVertex(c *adt.OpContext, v *adt.Vertex, state adt.VertexStatus) *nodeShared {
// fmt.Println(debug.NodeString(c.StringIndexer, v, nil))
func (e *Evaluator) evalVertex(c *adt.OpContext, v *adt.Vertex, state adt.VertexStatus, accept adt.Acceptor) *nodeShared {
shared := &nodeShared{
ctx: c,
eval: e,
node: v,
stack: nil, // silence linter
ctx: c,
eval: e,
node: v,
stack: nil, // silence linter
accept: accept,
}
saved := *v

Expand Down Expand Up @@ -577,7 +591,14 @@ func (n *nodeContext) postDisjunct() {
a.Closed = m
}
if updated != nil && m.isClosed {
if err := m.verifyArcAllowed(n.ctx, a.Label); err != nil {
if accept := n.nodeShared.accept; accept != nil {
if !accept.Accept(n.ctx, a.Label) {
label := a.Label.SelectorString(ctx)
n.node.Value = &adt.Bottom{
Err: errors.Newf(token.NoPos, "field `%s` not allowed by Acceptor", label),
}
}
} else if err := m.verifyArcAllowed(n.ctx, a.Label); err != nil {
n.node.Value = err
}
// TODO: use continue to not process already failed fields,
Expand Down Expand Up @@ -633,6 +654,9 @@ type nodeShared struct {
resultNode *nodeContext
result_ adt.Vertex
stack []int

// Closedness override.
accept adt.Acceptor
}

func (n *nodeShared) result() *adt.Vertex {
Expand Down

0 comments on commit d15def3

Please sign in to comment.