From 769017e0ab3e2eccc0e17bf537444efe67c70bdc Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Mon, 21 Oct 2024 11:52:55 +0200 Subject: [PATCH] internal/core/adt: get rid of use of evaluatingArcs status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Change-Id: I749f795d9daa83acc45c8372a4823fcf04ad9cd0 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202813 Reviewed-by: Daniel Martí TryBot-Result: CUEcueckoo Unity-Result: CUE porcuepine --- internal/core/adt/cycle.go | 10 +--------- internal/core/adt/unify.go | 6 ++++-- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/internal/core/adt/cycle.go b/internal/core/adt/cycle.go index e9725490e73..7ea7da965e2 100644 --- a/internal/core/adt/cycle.go +++ b/internal/core/adt/cycle.go @@ -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 diff --git a/internal/core/adt/unify.go b/internal/core/adt/unify.go index fa90d1f32db..69f51339ae0 100644 --- a/internal/core/adt/unify.go +++ b/internal/core/adt/unify.go @@ -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) @@ -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 @@ -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 {