Skip to content

Commit

Permalink
internal/core/adt: move and rename Indirect
Browse files Browse the repository at this point in the history
There are now different kinds of dereferencing. We rename
Indirect to DerefValue and move it close to its siblings.
Goals:
- keep all types of dereference together
- keep naming consistent
- use DerefValue, instead of Deref or DerefAll, to make the
  purpose of this particular dereference clear: getting the
  underlying value.

Added notes on which dereferences might need to change
in the future.

Issue #2854

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: I3ccde0ac21e248f65d9a462515840c77348f658a
  • Loading branch information
mpvl committed May 1, 2024
1 parent f9c2de9 commit 997439a
Show file tree
Hide file tree
Showing 11 changed files with 66 additions and 38 deletions.
4 changes: 2 additions & 2 deletions cue/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,11 @@ func (i *Iterator) Next() bool {
p := linkParent(i.val.parent_, i.val.v, arc)
i.f = arc.Label
i.arcType = arc.ArcType
// TODO: this should not indirect disjunctions, as this will cause losing
// TODO(deref): this should not indirect disjunctions, as this will cause losing
// information, which is now available in the new evaluator. Find a safe
// way to preserve disjunction information while keeping backward
// compatibility in the API.
arc = arc.Indirect()
arc = arc.DerefValue()
i.cur = makeValue(i.val.idx, arc, p)
i.p++
return true
Expand Down
45 changes: 23 additions & 22 deletions internal/core/adt/composite.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ type Vertex struct {
}

func deref(v *Vertex) *Vertex {
v = v.Indirect()
v = v.DerefValue()
n := v.state
if n != nil {
v = n.underlying
Expand Down Expand Up @@ -679,7 +679,10 @@ func (v *Vertex) isUndefined() bool {

// isFinal reports whether this node may no longer be modified.
func (v *Vertex) isFinal() bool {
v = v.Indirect()
// TODO(deref): the accounting of what is final should be recorded
// in the original node. Remove this dereference once the old
// evaluator has been removed.
v = v.DerefValue()
return v.status == finalized
}

Expand Down Expand Up @@ -852,7 +855,11 @@ func Unwrap(v Value) Value {
if !ok {
return v
}
x = x.Indirect()
// TODO(deref): BaseValue is currently overloaded to track cycles as well
// as the actual or dereferenced value. Once the old evaluator can be
// removed, we should use the new cycle tracking mechanism for cycle
// detection and keep BaseValue clean.
x = x.DerefValue()
if n := x.state; n != nil && isCyclePlaceholder(x.BaseValue) {
if n.errs != nil && !n.errs.IsIncomplete() {
return n.errs
Expand All @@ -864,19 +871,6 @@ func Unwrap(v Value) Value {
return x.Value()
}

// Indirect unrolls indirections of Vertex values. These may be introduced,
// for instance, by temporary bindings such as comprehension values.
// It returns v itself if v does not point to another Vertex.
func (v *Vertex) Indirect() *Vertex {
for {
arc, ok := v.BaseValue.(*Vertex)
if !ok {
return v
}
v = arc
}
}

// OptionalType is a bit field of the type of optional constraints in use by an
// Acceptor.
type OptionalType int8
Expand Down Expand Up @@ -973,7 +967,14 @@ func (v *Vertex) Accept(ctx *OpContext, f Feature) bool {
return true
}

v = v.Indirect()
// TODO(deref): right now a dereferenced value holds all the necessary
// closedness information. In the future we may want to allow sharing nodes
// with different closedness information. In that case, we should reconsider
// the use of this dereference. Consider, for instance:
//
// #a: b // this node is currently not shared, but could be.
// b: {c: 1}
v = v.DerefValue()
if x, ok := v.BaseValue.(*Disjunction); ok {
for _, v := range x.Values {
if x, ok := v.(*Vertex); ok && x.Accept(ctx, f) {
Expand Down Expand Up @@ -1068,12 +1069,12 @@ func (v *Vertex) IsList() bool {
func (v *Vertex) Lookup(f Feature) *Vertex {
for _, a := range v.Arcs {
if a.Label == f {
// TODO(share): this indirection should ultimately be eliminated:
// the original node may have useful information (like original
// conjuncts) that are eliminated after indirection.
// We should leave it up to the user of Lookup at what point an
// TODO(P1)/TODO(deref): this indirection should ultimately be
// eliminated: the original node may have useful information (like
// original conjuncts) that are eliminated after indirection. We
// should leave it up to the user of Lookup at what point an
// indirection is necessary.
a = a.Indirect()
a = a.DerefValue()
return a
}
}
Expand Down
6 changes: 4 additions & 2 deletions internal/core/adt/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ func (c *OpContext) resolveState(x Conjunct, r Resolver, state combinedFlags) (*
}

if !c.isDevVersion() {
arc = arc.Indirect()
arc = arc.DerefValue()
}

return arc, err
Expand All @@ -512,7 +512,9 @@ func (c *OpContext) Lookup(env *Environment, r Resolver) (*Vertex, *Bottom) {
err := c.PopState(s)

if arc != nil {
arc = arc.Indirect()
// TODO(P1)/TODO(deref): lookup should probably not use DerefValue, but
// rather only dereference disjunctions.
arc = arc.DerefValue()
}

return arc, err
Expand Down
2 changes: 1 addition & 1 deletion internal/core/adt/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (d *Disjunction) Default() Value {
//
// It also closes a list, representing its default value.
func (v *Vertex) Default() *Vertex {
v = v.Indirect()
v = v.DerefValue()
switch d := v.BaseValue.(type) {
default:
return v
Expand Down
6 changes: 3 additions & 3 deletions internal/core/adt/disjunct2.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ func appendDisjunct(ctx *OpContext, a []*nodeContext, x *nodeContext) []*nodeCon
return a
}

nv := x.node.Indirect()
nv := x.node.DerefValue()
nx := nv.BaseValue
if nx == nil || isCyclePlaceholder(nx) {
nx = x.getValidators(finalized)
Expand All @@ -528,7 +528,7 @@ func appendDisjunct(ctx *OpContext, a []*nodeContext, x *nodeContext) []*nodeCon
// (overlayed) closeContexts are identical.
outer:
for _, xn := range a {
xv := xn.node.Indirect()
xv := xn.node.DerefValue()
if xv.status != finalized || nv.status != finalized {
// Partial node

Expand Down Expand Up @@ -564,7 +564,7 @@ outer:
}
} else {
// Complete nodes.
if !Equal(ctx, xn.node.Indirect(), x.node.Indirect(), CheckStructural) {
if !Equal(ctx, xn.node.DerefValue(), x.node.DerefValue(), CheckStructural) {
continue outer
}
}
Expand Down
4 changes: 2 additions & 2 deletions internal/core/adt/equality.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ func equalVertex(ctx *OpContext, x *Vertex, v Value, flags Flag) bool {
return false
}

x = x.Indirect()
y = y.Indirect()
x = x.DerefValue()
y = y.DerefValue()

if x == y {
return true
Expand Down
2 changes: 1 addition & 1 deletion internal/core/adt/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ func (n *nodeContext) validateValue(state vertexStatus) {
} else if len(n.node.Structs) > 0 {
markStruct = n.kind&StructKind != 0 && !n.hasTop
}
v := n.node.Indirect().Value()
v := n.node.DerefValue().Value()
if n.node.BaseValue == nil && markStruct {
n.node.BaseValue = &StructMarker{}
v = n.node
Expand Down
2 changes: 1 addition & 1 deletion internal/core/adt/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -1431,7 +1431,7 @@ func (c *OpContext) validate(env *Environment, src ast.Node, x Expr, op Op, flag
}

func isFinalError(n *Vertex) bool {
n = n.Indirect()
n = n.DerefValue()
if b, ok := Unwrap(n).(*Bottom); ok && b.Code < IncompleteError {
return true
}
Expand Down
25 changes: 25 additions & 0 deletions internal/core/adt/share.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,31 @@ func (n *nodeContext) shareIfPossible(c Conjunct, arc *Vertex, id CloseInfo) boo
return true
}

// Vertex values that are held in BaseValue will be wrapped in the following
// order:
//
// disjuncts -> (shared | computed | data)
//
// DerefDisjunct
// - get the current value under computation
//
// DerefValue
// - get the value the node ultimately represents.
//

// DerefValue unrolls indirections of Vertex values. These may be introduced,
// for instance, by temporary bindings such as comprehension values.
// It returns v itself if v does not point to another Vertex.
func (v *Vertex) DerefValue() *Vertex {
for {
arc, ok := v.BaseValue.(*Vertex)
if !ok {
return v
}
v = arc
}
}

// DerefDisjunct indirects a node that points to a disjunction.
func (v *Vertex) DerefDisjunct() *Vertex {
for {
Expand Down
6 changes: 3 additions & 3 deletions internal/core/adt/unify.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ func (v *Vertex) unify(c *OpContext, needs condition, mode runMode) bool {

n.finalizeDisjunctions()

w = v.Indirect() // Dereference anything, including shared nodes.
w = v.DerefValue() // Dereference anything, including shared nodes.
if w != v {
// Clear value fields that are now referred to in the dereferenced
// value (w).
Expand Down Expand Up @@ -595,7 +595,7 @@ func (v *Vertex) lookup(c *OpContext, pos token.Pos, f Feature, flags combinedFl
needs := flags.conditions()
runMode := flags.runMode()

v = v.Indirect()
v = v.DerefValue()

c.Logf(c.vertex, "LOOKUP %v", f)

Expand Down Expand Up @@ -731,7 +731,7 @@ func (v *Vertex) accept(ctx *OpContext, f Feature) bool {
// return true, true
// }

v = v.Indirect()
v = v.DerefValue()

pc := v.PatternConstraints
if pc == nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/core/debug/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func (w *printer) printShared(v *adt.Vertex) (x *adt.Vertex, ok bool) {
useReference := v.IsShared
isCyclic := v.IsCyclic
s, ok := v.BaseValue.(*adt.Vertex)
v = v.Indirect()
v = v.DerefValue()
isCyclic = isCyclic || v.IsCyclic
if useReference && isCyclic && ok && len(v.Arcs) > 0 {
w.shared(s)
Expand Down

0 comments on commit 997439a

Please sign in to comment.