Skip to content

Commit

Permalink
internal/core/adt: use partial lookup
Browse files Browse the repository at this point in the history
For selectors and indices, don't pass Finalized state
for selector operand. Currently this doesn't matter,
but in the comprehension changes such nodes are more
actively evaluated, which may lead to spurious cycles.

Not doing this active evaluation, in turn, may lead
to void arcs being detected as full arcs.

Change-Id: Iba539a379bd9e68cd7a3b70e5d249476dcfc0698
Signed-off-by: Marcel van Lohuizen <[email protected]>
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/533293
Unity-Result: CUEcueckoo <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Paul Jolly <[email protected]>
  • Loading branch information
mpvl committed Feb 17, 2022
1 parent b995f5b commit 8dee602
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
7 changes: 6 additions & 1 deletion internal/core/adt/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ func (c *OpContext) unifyNode(v Expr, state VertexStatus) (result Value) {
return nil
}

if v.isUndefined() {
if v.isUndefined() || state > v.status {
// Keep a minimum state of AllArcs.
state := state
if state < AllArcs {
Expand Down Expand Up @@ -844,6 +844,11 @@ outer:
}
x.state.usedArcs = append(x.state.usedArcs, a)
}

if a != nil && state > a.status {
c.Unify(a, state)
}

if a == nil {
if x.state != nil {
for _, e := range x.state.exprs {
Expand Down
12 changes: 10 additions & 2 deletions internal/core/adt/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,11 @@ func (x *SelectorExpr) Source() ast.Node {
}

func (x *SelectorExpr) resolve(c *OpContext, state VertexStatus) *Vertex {
n := c.node(x, x.X, x.Sel.IsRegular(), state)
// TODO: the node should really be evaluated as AllArcs, but the order
// of evaluation is slightly off, causing too much to be evaluated.
// This may especially result in incorrect results when using embedded
// scalars.
n := c.node(x, x.X, x.Sel.IsRegular(), Partial)
if n == emptyNode {
return n
}
Expand Down Expand Up @@ -929,7 +933,11 @@ func (x *IndexExpr) Source() ast.Node {

func (x *IndexExpr) resolve(ctx *OpContext, state VertexStatus) *Vertex {
// TODO: support byte index.
n := ctx.node(x, x.X, true, state)
// TODO: the node should really be evaluated as AllArcs, but the order
// of evaluation is slightly off, causing too much to be evaluated.
// This may especially result in incorrect results when using embedded
// scalars.
n := ctx.node(x, x.X, true, Partial)
i := ctx.value(x.Index)
if n == emptyNode {
return n
Expand Down

0 comments on commit 8dee602

Please sign in to comment.