Skip to content

Commit

Permalink
internal/core/adt: get rid of use of evaluatingArcs status
Browse files Browse the repository at this point in the history
This is an old mechanism that is not incredibly precise.
By fixing a bug in the handling of pattern constraints
we can now get rid of it. Basically, pattern constraints
count as having an increased depth. So far, we have
forgotten to increase these counters.

This is a No-op at the moment, but will simplify some
changes in the future.

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: I749f795d9daa83acc45c8372a4823fcf04ad9cd0
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202813
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
  • Loading branch information
mpvl committed Oct 21, 2024
1 parent 6418113 commit 769017e
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 11 deletions.
10 changes: 1 addition & 9 deletions internal/core/adt/cycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -723,16 +723,8 @@ func (n *nodeContext) markCyclicPathV3(arc *Vertex, env *Environment, x Resolver
// hasAncestorV3 checks whether a node is currently being processed. The code
// still assumes that is includes any node that is currently being processed.
func (n *nodeContext) hasAncestorV3(arc *Vertex) bool {
// TODO: consider removing this. For now we still need it, but we could
// possibly remove it after we strictly separate processing lookups versus
// full evaluation.
if arc.status == evaluatingArcs {
return true
}

// TODO(evalv3): replace the use of the old status mechanism.
// the depth counters is an alternative mechanism to the status used in
// the v2 evaluator. It is slightly more precise:
// Use depth counters to keep track of cycles:
// - it allows detecting reference cycles as well (state evaluating is
// no longer used in v3)
// - it can capture cycles across inline structs, which do not have
Expand Down
6 changes: 4 additions & 2 deletions internal/core/adt/unify.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,9 @@ func (v *Vertex) unify(c *OpContext, needs condition, mode runMode) bool {
// For this reason we disable it now. It may be the case that we need
// to enable it for computing disjunctions.
//
n.incDepth()
defer n.decDepth()

if pc := n.node.PatternConstraints; pc != nil {
for _, c := range pc.Pairs {
c.Constraint.unify(n.ctx, allKnown, attemptOnly)
Expand Down Expand Up @@ -519,6 +522,7 @@ func (n *nodeContext) completeAllArcs(needs condition, mode runMode) bool {
}

n.incDepth()
defer n.decDepth()

// XXX(0.7): only set success if needs complete arcs.
success := true
Expand Down Expand Up @@ -578,8 +582,6 @@ func (n *nodeContext) completeAllArcs(needs condition, mode runMode) bool {
}
}

n.decDepth()

k := 0
for _, a := range n.node.Arcs {
if a.ArcType != ArcNotPresent {
Expand Down

0 comments on commit 769017e

Please sign in to comment.