internal/core/adt: fix disjunct dedupping regression
Equal was called on unfinalized values, which could lead
to premuture elimination of values.
Fixes #641
Change-Id: I56343ecee2c8bb6332903b5deed7906fd00925b3
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8224
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cue/testdata/eval/disjunctions.txtar b/cue/testdata/eval/disjunctions.txtar
index 31eba44..5027d2b 100644
--- a/cue/testdata/eval/disjunctions.txtar
+++ b/cue/testdata/eval/disjunctions.txtar
@@ -108,6 +108,31 @@
j: i.bar
}
+issue641: {
+ #A: {
+ type: "a"
+ x: ""
+ }
+
+ #B: {
+ type: "b"
+ x: string
+ }
+
+ #C: {
+ b: #A | #B
+ }
+
+ e: [string]: #C & {
+ b: #A | #B
+ }
+
+ e: foobar: #C & {
+ b: #B & {
+ x: "foobar"
+ }
+ }
+}
-- out/eval --
Errors:
f: 2 errors in empty disjunction:
@@ -251,6 +276,33 @@
// ./in.cue:107:6
}
}
+ issue641: (struct){
+ #A: (#struct){
+ type: (string){ "a" }
+ x: (string){ "" }
+ }
+ #B: (#struct){
+ type: (string){ "b" }
+ x: (string){ string }
+ }
+ #C: (#struct){
+ b: (struct){ |((#struct){
+ type: (string){ "a" }
+ x: (string){ "" }
+ }, (#struct){
+ type: (string){ "b" }
+ x: (string){ string }
+ }) }
+ }
+ e: (struct){
+ foobar: (#struct){
+ b: (#struct){
+ type: (string){ "b" }
+ x: (string){ "foobar" }
+ }
+ }
+ }
+ }
}
-- out/compile --
--- in.cue
@@ -411,4 +463,29 @@
})
j: 〈0;i〉.bar
}
+ issue641: {
+ #A: {
+ type: "a"
+ x: ""
+ }
+ #B: {
+ type: "b"
+ x: string
+ }
+ #C: {
+ b: (〈1;#A〉|〈1;#B〉)
+ }
+ e: {
+ [string]: (〈1;#C〉 & {
+ b: (〈2;#A〉|〈2;#B〉)
+ })
+ }
+ e: {
+ foobar: (〈1;#C〉 & {
+ b: (〈2;#B〉 & {
+ x: "foobar"
+ })
+ })
+ }
+ }
}
diff --git a/internal/core/adt/disjunct.go b/internal/core/adt/disjunct.go
index a24de24..d7b8f2f 100644
--- a/internal/core/adt/disjunct.go
+++ b/internal/core/adt/disjunct.go
@@ -165,22 +165,16 @@
}
return
}
- if n.node.BaseValue == nil {
- n.node.BaseValue = n.getValidators()
- }
-
- // TODO: clean up this mess:
- result := *n.node // XXX: n.result = snapshotVertex(n.node)?
-
- if recursive && state < Finalized {
- *n = m
- }
- n.result = result
- n.node = &n.result
if recursive {
+ *n = m
+ n.result = *n.node // XXX: n.result = snapshotVertex(n.node)?
+ n.node = &n.result
n.disjuncts = append(n.disjuncts, n)
}
+ if n.node.BaseValue == nil {
+ n.node.BaseValue = n.getValidators()
+ }
case len(n.disjunctions) > 0:
// Process full disjuncts to ensure that erroneous disjuncts are
@@ -201,6 +195,8 @@
// not be done unless there is complete information.
state = Partial
}
+ // TODO(perf): ideally always finalize. See comment below
+ // state = Finalized
for _, dn := range a {
switch {
@@ -272,6 +268,12 @@
outer:
for _, d := range n.disjuncts {
for _, v := range p.disjuncts {
+ // TODO(perf): not checking equality here may lead to polynomial
+ // blowup. Doesn't seem to affect large configurations, though,
+ // where this could matter at the moment.
+ if state != Finalized {
+ break
+ }
if Equal(n.ctx, &v.result, &d.result) {
if d.defaultMode == isDefault {
v.defaultMode = isDefault
