Skip to content

Commit 396202c

Browse files
committed
internal/core/adt: don't pre-expand disjunctions
This also simplifies combining defeault modes, which really is just max, given the proper ordering of modes. Change-Id: I468c4c343e53ae86a5a14b6a477c8a389a212855 Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8109 Reviewed-by: Marcel van Lohuizen <[email protected]> Reviewed-by: CUE cueckoo <[email protected]>
1 parent 69e0c96 commit 396202c

File tree

4 files changed

+53
-61
lines changed

4 files changed

+53
-61
lines changed

cue/testdata/basicrewrite/014_disjunctions.txtar

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ i1: "c"
8484
o1: (int){ |((int){ 1 }, (int){ 2 }, (int){ 3 }) }
8585
o2: (int){ 1 }
8686
o3: (int){ 2 }
87-
o4: (int){ |((int){ 2 }, (int){ 1 }, (int){ 3 }) }
87+
o4: (int){ |((int){ 1 }, (int){ 2 }, (int){ 3 }) }
8888
o5: (int){ |(*(int){ 2 }, (int){ 1 }, (int){ 3 }) }
8989
o6: (int){ |((int){ 1 }, (int){ 2 }, (int){ 3 }) }
9090
o7: (int){ |((int){ 2 }, (int){ 3 }) }

cue/testdata/cycle/025_cannot_resolve_references_that_would_be_ambiguous.txtar

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ c2: (*{
6363
}
6464
a3: (int){ 1 }
6565
b1: (int){ |((int){ 0 }, (int){ 1 }) }
66-
b2: (int){ |((int){ 1 }, (int){ 0 }) }
66+
b2: (int){ |((int){ 0 }, (int){ 1 }) }
6767
c1: (struct){ |((struct){
6868
a: (int){ 1 }
6969
b: (int){ 2 }

cue/testdata/eval/disjunctions.txtar

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,11 @@ t11: {
9595
a: #A
9696
}
9797

98+
cross: {
99+
a: *"word" | string
100+
a: string | *"word"
101+
}
102+
98103

99104
d100: {
100105
// Should we allow a selector to imply a struct or list? Would be convenient.
@@ -234,13 +239,16 @@ Result:
234239
}, (#struct){
235240
}) }
236241
}
242+
cross: (struct){
243+
a: (string){ |(*(string){ "word" }, (string){ string }) }
244+
}
237245
d100: (struct){
238246
i: ((null|struct)){ |((null){ null }, (struct){
239247
bar: (int){ 2 }
240248
}) }
241249
j: (_|_){
242250
// [incomplete] d100.j: unresolved disjunction null | {bar:2} (type (null|struct)):
243-
// ./in.cue:102:6
251+
// ./in.cue:107:6
244252
}
245253
}
246254
}
@@ -393,6 +401,10 @@ Result:
393401
a: 〈0;b〉
394402
a: 〈0;#A〉
395403
}
404+
cross: {
405+
a: (*"word"|string)
406+
a: (string|*"word")
407+
}
396408
d100: {
397409
i: (null|{
398410
bar: 2

internal/core/adt/disjunct.go

Lines changed: 38 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
package adt
1616

1717
import (
18-
"sort"
19-
2018
"cuelang.org/go/cue/errors"
2119
"cuelang.org/go/cue/token"
2220
)
@@ -86,48 +84,30 @@ import (
8684

8785
type envDisjunct struct {
8886
env *Environment
89-
values []disjunct
90-
numDefaults int
91-
hasDefaults bool
9287
cloneID CloseInfo
93-
}
94-
95-
type disjunct struct {
96-
v *Vertex
97-
expr Expr
98-
isDefault bool
88+
expr *DisjunctionExpr
89+
value *Disjunction
90+
hasDefaults bool
9991
}
10092

10193
func (n *nodeContext) addDisjunction(env *Environment, x *DisjunctionExpr, cloneID CloseInfo) {
102-
a := make([]disjunct, 0, len(x.Values))
10394

95+
// TODO: precompute
10496
numDefaults := 0
10597
for _, v := range x.Values {
10698
isDef := v.Default // || n.hasDefaults(env, v.Val)
10799
if isDef {
108100
numDefaults++
109101
}
110-
a = append(a, disjunct{nil, v.Val, isDef})
111102
}
112103

113-
sort.SliceStable(a, func(i, j int) bool {
114-
return !a[j].isDefault && a[i].isDefault != a[j].isDefault
115-
})
116-
117104
n.disjunctions = append(n.disjunctions,
118-
envDisjunct{env, a, numDefaults, numDefaults > 0, cloneID})
119-
105+
envDisjunct{env, cloneID, x, nil, numDefaults > 0})
120106
}
121107

122108
func (n *nodeContext) addDisjunctionValue(env *Environment, x *Disjunction, cloneID CloseInfo) {
123-
a := make([]disjunct, 0, len(x.Values))
124-
125-
for i, v := range x.Values {
126-
a = append(a, disjunct{v, nil, i < x.NumDefaults})
127-
}
128-
129109
n.disjunctions = append(n.disjunctions,
130-
envDisjunct{env, a, x.NumDefaults, x.HasDefaults, cloneID})
110+
envDisjunct{env, cloneID, nil, x, x.HasDefaults})
131111

132112
}
133113

@@ -216,22 +196,33 @@ func (n *nodeContext) expandDisjuncts(
216196
}
217197

218198
for _, dn := range a {
219-
for _, v := range d.values {
220-
cn := dn.clone()
221-
*cn.node = snapshotVertex(dn.snapshot)
222-
223-
if v.v != nil {
224-
cn.addValueConjunct(d.env, v.v, d.cloneID)
225-
} else {
226-
c := MakeConjunct(d.env, v.expr, d.cloneID)
199+
switch {
200+
case d.expr != nil:
201+
for _, v := range d.expr.Values {
202+
cn := dn.clone()
203+
*cn.node = snapshotVertex(dn.snapshot)
204+
205+
c := MakeConjunct(d.env, v.Val, d.cloneID)
227206
cn.addExprConjunct(c)
207+
208+
newMode := mode(d.hasDefaults, v.Default)
209+
cn.defaultMode = combineDefault(dn.defaultMode, newMode)
210+
211+
cn.expandDisjuncts(state, n, newMode, true)
228212
}
229213

230-
newMode := mode(d, v)
214+
case d.value != nil:
215+
for i, v := range d.value.Values {
216+
cn := dn.clone()
217+
*cn.node = snapshotVertex(dn.snapshot)
218+
219+
cn.addValueConjunct(d.env, v, d.cloneID)
231220

232-
cn.expandDisjuncts(state, n, newMode, true)
221+
newMode := mode(d.hasDefaults, i < d.value.NumDefaults)
222+
cn.defaultMode = combineDefault(dn.defaultMode, newMode)
233223

234-
cn.defaultMode = combineDefault(dn.defaultMode, newMode)
224+
cn.expandDisjuncts(state, n, newMode, true)
225+
}
235226
}
236227
}
237228

@@ -243,6 +234,7 @@ func (n *nodeContext) expandDisjuncts(
243234

244235
if len(n.disjuncts) == 0 {
245236
n.makeError()
237+
break
246238
}
247239
}
248240

@@ -271,6 +263,9 @@ func (n *nodeContext) expandDisjuncts(
271263
for _, v := range p.disjuncts {
272264
if Equal(n.ctx, &v.result, &d.result) {
273265
n.ctx.Unifier.freeNodeContext(n)
266+
if d.defaultMode == isDefault {
267+
v.defaultMode = isDefault
268+
}
274269
continue outer
275270
}
276271
}
@@ -307,12 +302,12 @@ func (n *nodeContext) makeError() {
307302
n.node.SetValue(n.ctx, Finalized, b)
308303
}
309304

310-
func mode(d envDisjunct, v disjunct) defaultMode {
305+
func mode(hasDefault, marked bool) defaultMode {
311306
var mode defaultMode
312307
switch {
313-
case !d.hasDefaults:
308+
case !hasDefault:
314309
mode = maybeDefault
315-
case v.isDefault:
310+
case marked:
316311
mode = isDefault
317312
default:
318313
mode = notDefault
@@ -379,8 +374,8 @@ type defaultMode int
379374

380375
const (
381376
maybeDefault defaultMode = iota
382-
notDefault
383377
isDefault
378+
notDefault
384379
)
385380

386381
// combineDefaults combines default modes for unifying conjuncts.
@@ -391,24 +386,9 @@ const (
391386
// U2: (v1, d1) & (v2, d2) => (v1&v2, d1&d2)
392387
func combineDefault(a, b defaultMode) defaultMode {
393388
if a > b {
394-
a, b = b, a
395-
}
396-
switch {
397-
case a == maybeDefault && b == maybeDefault:
398-
return maybeDefault
399-
case a == maybeDefault && b == notDefault:
400-
return notDefault
401-
case a == maybeDefault && b == isDefault:
402-
return isDefault
403-
case a == notDefault && b == notDefault:
404-
return notDefault
405-
case a == notDefault && b == isDefault:
406-
return notDefault
407-
case a == isDefault && b == isDefault:
408-
return isDefault
409-
default:
410-
panic("unreachable")
389+
return a
411390
}
391+
return b
412392
}
413393

414394
// disjunctError returns a compound error for a failed disjunction.

0 commit comments

Comments
 (0)