Skip to content

Commit cdfb056

Browse files
committed
internal/core/adt: rewrite Environments for disjuncts
The Evaluator uses an Environment that is used to resolve references within CUE expressions. Each Environment points to a Vertex in which to look up a reference at that level. When a disjunction for a node is evaluated, the node is "cloned" for each disjunct. Environments that point to the "cloned" node must be updated to point to the respective disjunct. This is especially true for the various disjuncts that are inserted. This change adds this rewriting code. Note that for conjuncts that were already added, either the value will not change, or the expression will be re-evalauted when it is later unified with something else. We nonetheless add some code for this as this invariant may not hold if we apply more aggressive structure sharing. This code is marked with a TODO. Issue #3829 Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: If5804ad37bae17f8015022404b6af254b0fc1026 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1212022 TryBot-Result: CUEcueckoo <[email protected]> Unity-Result: CUE porcuepine <[email protected]> Reviewed-by: Matthew Sackman <[email protected]>
1 parent a16bbf7 commit cdfb056

File tree

3 files changed

+45
-72
lines changed

3 files changed

+45
-72
lines changed

cue/testdata/disjunctions/resolve.txtar

-65
Original file line numberDiff line numberDiff line change
@@ -51,71 +51,6 @@ Retain: 0
5151
Unifications: 32
5252
Conjuncts: 67
5353
Disjuncts: 44
54-
-- out/evalalpha --
55-
(struct){
56-
resolveAcrossDisjunct: (struct){
57-
t1: (struct){ |((struct){
58-
#B: (int){ int }
59-
d: (int){ int }
60-
}, (struct){
61-
#B: (int){ int }
62-
}, (struct){
63-
#B: (int){ 1 }
64-
d: (int){ int }
65-
}, (struct){
66-
#B: (int){ 1 }
67-
}) }
68-
nested: (struct){ |((struct){
69-
a: (struct){
70-
b: (struct){
71-
#B: (int){ int }
72-
}
73-
}
74-
d: (int){ int }
75-
}, (struct){
76-
a: (struct){
77-
b: (struct){
78-
#B: (int){ int }
79-
}
80-
}
81-
}, (struct){
82-
a: (struct){
83-
b: (struct){
84-
#B: (int){ 1 }
85-
}
86-
}
87-
d: (int){ int }
88-
}, (struct){
89-
a: (struct){
90-
b: (struct){
91-
#B: (int){ 1 }
92-
}
93-
}
94-
}) }
95-
}
96-
}
97-
-- diff/-out/evalalpha<==>+out/eval --
98-
diff old new
99-
--- old
100-
+++ new
101-
@@ -7,7 +7,7 @@
102-
#B: (int){ int }
103-
}, (struct){
104-
#B: (int){ 1 }
105-
- d: (int){ 1 }
106-
+ d: (int){ int }
107-
}, (struct){
108-
#B: (int){ 1 }
109-
}) }
110-
@@ -30,7 +30,7 @@
111-
#B: (int){ 1 }
112-
}
113-
}
114-
- d: (int){ 1 }
115-
+ d: (int){ int }
116-
}, (struct){
117-
a: (struct){
118-
b: (struct){
11954
-- out/eval --
12055
(struct){
12156
resolveAcrossDisjunct: (struct){

internal/core/adt/disjunct2.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ func (n *nodeContext) crossProduct(dst, cross []*nodeContext, dn *envDisjunct, m
389389
ID.node.nextDisjunct(j, len(dn.disjuncts), d.expr)
390390

391391
c := MakeConjunct(dn.env, d.expr, dn.cloneID)
392-
r, err := p.doDisjunct(c, d.mode, mode)
392+
r, err := p.doDisjunct(c, d.mode, mode, n.node)
393393

394394
if err != nil {
395395
// TODO: store more error context
@@ -436,7 +436,10 @@ func (n *nodeContext) collectErrors(dn *envDisjunct) (errs *Bottom) {
436436
return b
437437
}
438438

439-
func (n *nodeContext) doDisjunct(c Conjunct, m defaultMode, mode runMode) (*nodeContext, *Bottom) {
439+
// doDisjunct computes a single disjunct. n is the current disjunct that is
440+
// augmented, whereas orig is the original node where disjunction processing
441+
// started. orig is used to clean up Environments.
442+
func (n *nodeContext) doDisjunct(c Conjunct, m defaultMode, mode runMode, orig *Vertex) (*nodeContext, *Bottom) {
440443
n.ctx.inDisjunct++
441444
defer func() { n.ctx.inDisjunct-- }()
442445

@@ -458,6 +461,7 @@ func (n *nodeContext) doDisjunct(c Conjunct, m defaultMode, mode runMode) (*node
458461

459462
d := oc.cloneRoot(n)
460463
d.runMode = mode
464+
c.Env = derefDisjunctsEnv(c.Env, orig, d.node)
461465

462466
d.defaultMode = combineDefault(m, n.defaultMode)
463467

internal/core/adt/overlay.go

+39-5
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ type overlayContext struct {
5757
// inserting selected disjuncts into a new Vertex.
5858
func (ctx *overlayContext) cloneRoot(root *nodeContext) *nodeContext {
5959
// Clone all vertices that need to be cloned to support the overlay.
60-
v := ctx.cloneVertex(root.node)
60+
v := ctx.cloneVertex(root.node, nil, nil)
6161
v.IsDisjunct = true
6262

6363
for _, v := range ctx.vertices {
@@ -101,29 +101,46 @@ func (ctx *overlayContext) unlinkOverlay() {
101101
// benefit this gives. More importantly, we should first implement the filter
102102
// to eliminate disjunctions pre-copy based on discriminator fields and what
103103
// have you. This is not unlikely to eliminate
104-
func (ctx *overlayContext) cloneVertex(x *Vertex) *Vertex {
104+
func (ctx *overlayContext) cloneVertex(x, from, to *Vertex) *Vertex {
105105
if x.overlay != nil {
106106
return x.overlay
107107
}
108108

109109
v := &Vertex{}
110110
*v = *x
111111

112+
// from == nil signals that cloneVertex is directly called from cloneRoot.
113+
// All nested calls to cloneVertex will be called with the value of x and v
114+
// of this vertex. Ideally, cloneRoot would call cloneVertex with the
115+
// correct values already, but x is not known yet at that point.
116+
if from == nil {
117+
from, to = x, v
118+
}
119+
112120
x.overlay = v
113121

114122
ctx.vertices = append(ctx.vertices, x)
115123

124+
v.Conjuncts = slices.Clone(v.Conjuncts)
125+
116126
// The group of the root closeContext should point to the Conjuncts field
117127
// of the Vertex. As we already allocated the group, we use that allocation,
118128
// but "move" it to v.Conjuncts.
119-
v.Conjuncts = slices.Clone(v.Conjuncts)
129+
// TODO: Is this ever necessary? It is certainly necessary to rewrite
130+
// environments from inserted disjunction values, but expressions that
131+
// were already added will typically need to be recomputed and recreated
132+
// anyway. We add this in to be a bit defensive and reinvestigate once we
133+
// have more aggressive structure sharing implemented
134+
for i, c := range v.Conjuncts {
135+
v.Conjuncts[i].Env = derefDisjunctsEnv(c.Env, x, v)
136+
}
120137

121138
if a := x.Arcs; len(a) > 0 {
122139
// TODO(perf): reuse buffer.
123140
v.Arcs = make([]*Vertex, len(a))
124141
for i, arc := range a {
125142
// TODO(perf): reuse when finalized.
126-
arc := ctx.cloneVertex(arc)
143+
arc := ctx.cloneVertex(arc, from, to)
127144
v.Arcs[i] = arc
128145
arc.Parent = v
129146
}
@@ -139,7 +156,7 @@ func (ctx *overlayContext) cloneVertex(x *Vertex) *Vertex {
139156
for i, p := range pc.Pairs {
140157
npc.Pairs[i] = PatternConstraint{
141158
Pattern: p.Pattern,
142-
Constraint: ctx.cloneVertex(p.Constraint),
159+
Constraint: ctx.cloneVertex(p.Constraint, from, to),
143160
}
144161
}
145162
}
@@ -154,6 +171,23 @@ func (ctx *overlayContext) cloneVertex(x *Vertex) *Vertex {
154171
return v
155172
}
156173

174+
// derefDisjunctsEnv creates a new env for each Environment in the Up chain
175+
// with each Environment where Vertex is "from" to one where Vertex is "to".
176+
func derefDisjunctsEnv(env *Environment, from, to *Vertex) *Environment {
177+
if env == nil {
178+
return nil
179+
}
180+
up := derefDisjunctsEnv(env.Up, from, to)
181+
if up != env.Up || env.Vertex == from {
182+
env = &Environment{
183+
Up: up,
184+
Vertex: to,
185+
DynamicLabel: env.DynamicLabel,
186+
}
187+
}
188+
return env
189+
}
190+
157191
func (ctx *overlayContext) cloneNodeContext(n *nodeContext) *nodeContext {
158192
n.node.getState(ctx.ctx) // ensure state is initialized.
159193

0 commit comments

Comments
 (0)