Skip to content

Commit

Permalink
internal/core/adt: mark functions only used in old evaluator
Browse files Browse the repository at this point in the history
Functions that are no longer used by the new evaluator are
marked with the unreachableForDev assertion.
This avoids accidentally mixing old and new evaluator, but
also makes it easier to review which parts are called by the
new evaluator and which not.

Note that some functions that are now not marked may
still be only partially used in follow up CLs using the
OpContext.useDevVersion method.

But we wanted to have these changes in before the new
evaluator is added and in a separate CL, so that
1) these changes do not interfere with that CL
2) one can quickly see which parts are not called during the review.

The panic should be uncommented once the new evaluator lands.

Issue #2884

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: Ic96098928c7a35a6e02c5929d2ea788a8ceafc85
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1174006
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: CUEcueckoo <cueckoo@gmail.com>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
  • Loading branch information
mpvl committed Mar 7, 2024
1 parent 145b436 commit f55405b
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 1 deletion.
2 changes: 2 additions & 0 deletions internal/core/adt/closed2.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ func isComplexStruct(ctx *OpContext, v *Vertex) bool {
// TODO: cleanup code and error messages. Reduce duplication in some related
// code.
func verifyArc2(ctx *OpContext, f Feature, v *Vertex, isClosed bool) (found bool, err *Bottom) {
unreachableForDev(ctx)

// Don't check computed, temporary vertices.
if v.Label == InvalidLabel {
return true, nil
Expand Down
8 changes: 8 additions & 0 deletions internal/core/adt/composite.go
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,8 @@ func (v *Vertex) Elems() []*Vertex {
// GetArc returns a Vertex for the outgoing arc with label f. It creates and
// ads one if it doesn't yet exist.
func (v *Vertex) GetArc(c *OpContext, f Feature, t ArcType) (arc *Vertex, isNew bool) {
unreachableForDev(c)

arc = v.Lookup(f)
if arc != nil {
arc.updateArcType(t)
Expand Down Expand Up @@ -998,6 +1000,8 @@ func hasConjunct(cs []Conjunct, c Conjunct) bool {
}

func (n *nodeContext) addConjunction(c Conjunct, index int) {
unreachableForDev(n.ctx)

// NOTE: This does not split binary expressions for comprehensions.
// TODO: split for comprehensions and rewrap?
if x, ok := c.Elem().(*BinaryExpr); ok && x.Op == AndOp {
Expand Down Expand Up @@ -1027,13 +1031,17 @@ func (v *Vertex) addConjunctUnchecked(c Conjunct) {
// addConjunctDynamic adds a conjunct to a vertex and immediately evaluates
// it, whilst doing the same for any vertices on the notify list, recursively.
func (n *nodeContext) addConjunctDynamic(c Conjunct) {
unreachableForDev(n.ctx)

n.node.Conjuncts = append(n.node.Conjuncts, c)
n.addExprConjunct(c, partial)
n.notifyConjunct(c)

}

func (n *nodeContext) notifyConjunct(c Conjunct) {
unreachableForDev(n.ctx)

for _, rec := range n.notify {
arc := rec.v
if !arc.hasConjunct(c) {
Expand Down
4 changes: 4 additions & 0 deletions internal/core/adt/comprehension.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,8 @@ func (s *compState) yield(env *Environment) (ok bool) {
// embeddings before inserting the results to ensure that the order of
// evaluation does not matter.
func (n *nodeContext) injectComprehensions(state vertexStatus) (progress bool) {
unreachableForDev(n.ctx)

workRemaining := false

// We use variables, instead of range, as the list may grow dynamically.
Expand Down Expand Up @@ -371,6 +373,8 @@ func (n *nodeContext) injectComprehensions(state vertexStatus) (progress bool) {
// as iterating over the node in which they are defined. Such comprehensions
// are legal as long as they do not modify the arc set of the node.
func (n *nodeContext) injectSelfComprehensions(state vertexStatus) {
unreachableForDev(n.ctx)

// We use variables, instead of range, as the list may grow dynamically.
for i := 0; i < len(n.selfComprehensions); i++ {
n.processComprehension(&n.selfComprehensions[i], state)
Expand Down
9 changes: 8 additions & 1 deletion internal/core/adt/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ package adt
// This file contains types to help in the transition from the old to new
// evaluation model.

func unreachableForDev(c *OpContext) {
if c.isDevVersion() {
// TODO: uncomment when new evaluator is ready.
// panic("unreachable for development version")
}
}

type combinedFlags uint32

// oldOnly indicates that a Vertex should only be evaluated for the old
Expand Down Expand Up @@ -44,7 +51,7 @@ func final(state vertexStatus, cond condition) combinedFlags {
}

func deprecated(c *OpContext, state vertexStatus) combinedFlags {
// if c.useDevVersion() {
// if c.isDevVersion() {
// panic("calling function may not be used in new evaluator")
// }
return combinedFlags(state)
Expand Down
2 changes: 2 additions & 0 deletions internal/core/adt/disjunct.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ func (n *nodeContext) expandDisjuncts(
parentMode defaultMode, // default mode of this disjunct
recursive, last bool) {

unreachableForDev(n.ctx)

n.ctx.stats.Disjuncts++

// refNode is used to collect cyclicReferences for all disjuncts to be
Expand Down
21 changes: 21 additions & 0 deletions internal/core/adt/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,8 @@ func (c *OpContext) unify(v *Vertex, flags combinedFlags) {

// insertConjuncts inserts conjuncts previously not inserted.
func (n *nodeContext) insertConjuncts(state vertexStatus) bool {
unreachableForDev(n.ctx)

// Exit early if we have a concrete value and only need partial results.
if state == partial {
for n.conjunctsPartialPos < len(n.conjuncts) {
Expand Down Expand Up @@ -463,6 +465,7 @@ func (n *nodeContext) doNotify() {

func (n *nodeContext) postDisjunct(state vertexStatus) {
ctx := n.ctx
unreachableForDev(ctx)

for {
// Use maybeSetCache for cycle breaking
Expand Down Expand Up @@ -737,6 +740,8 @@ func (n *nodeContext) incompleteErrors(final bool) *Bottom {
// though, that any potential performance issues are eliminated for
// Protobuf-like oneOf fields.
func (n *nodeContext) checkClosed(state vertexStatus) bool {
unreachableForDev(n.ctx)

ignore := state != finalized || n.skipNonMonotonicChecks()

v := n.node
Expand All @@ -756,6 +761,8 @@ func (n *nodeContext) checkClosed(state vertexStatus) bool {
}

func (n *nodeContext) completeArcs(state vertexStatus) {
unreachableForDev(n.ctx)

if DebugSort > 0 {
DebugSortArcs(n.ctx, n.node)
}
Expand Down Expand Up @@ -1084,6 +1091,8 @@ type defaultInfo struct {
}

func (n *nodeContext) addNotify(v *Vertex, cc *closeContext) {
unreachableForDev(n.ctx)

if v != nil && !n.node.hasAllConjuncts {
n.notify = append(n.notify, receiver{v, cc})
}
Expand Down Expand Up @@ -1172,6 +1181,8 @@ func (c *OpContext) newNodeContext(node *Vertex) *nodeContext {
}

func (v *Vertex) getNodeContext(c *OpContext, ref int) *nodeContext {
unreachableForDev(c)

if v.state == nil {
if v.status == finalized {
return nil
Expand Down Expand Up @@ -1484,6 +1495,8 @@ func (n *nodeContext) addErr(err errors.Error) {
// into the nodeContext if successful or queue it for later evaluation if it is
// incomplete or is not value.
func (n *nodeContext) addExprConjunct(v Conjunct, state vertexStatus) {
unreachableForDev(n.ctx)

env := v.Env
id := v.CloseInfo

Expand Down Expand Up @@ -1535,6 +1548,8 @@ func (n *nodeContext) addExprConjunct(v Conjunct, state vertexStatus) {
// evalExpr is only called by addExprConjunct. If an error occurs, it records
// the error in n and returns nil.
func (n *nodeContext) evalExpr(v Conjunct, state vertexStatus) {
unreachableForDev(n.ctx)

// Require an Environment.
ctx := n.ctx

Expand Down Expand Up @@ -1626,6 +1641,8 @@ func (n *nodeContext) evalExpr(v Conjunct, state vertexStatus) {
}

func (n *nodeContext) addVertexConjuncts(c Conjunct, arc *Vertex, inline bool) {
unreachableForDev(n.ctx)

closeInfo := c.CloseInfo

// We need to ensure that each arc is only unified once (or at least) a
Expand Down Expand Up @@ -2083,6 +2100,8 @@ func (n *nodeContext) insertFieldUnchecked(f Feature, mode ArcType, x Conjunct)
// TODO(errors): detect when a field is added to a struct that is already used
// in a for clause.
func (n *nodeContext) expandOne(state vertexStatus) (done bool) {
unreachableForDev(n.ctx)

// Don't expand incomplete expressions if we detected a cycle.
if n.done() || (n.hasCycle && !n.hasNonCycle) {
return false
Expand Down Expand Up @@ -2120,6 +2139,8 @@ func (n *nodeContext) expandOne(state vertexStatus) (done bool) {

// injectDynamic evaluates and inserts dynamic declarations.
func (n *nodeContext) injectDynamic() (progress bool) {
unreachableForDev(n.ctx)

ctx := n.ctx
k := 0

Expand Down

0 comments on commit f55405b

Please sign in to comment.