internal/core/adt: fix overzealous disambiguation
Equality is used to remove duplicate entries in a disjunction.
However, it did not consider optional fields. This could
lead to differing disjuncts being deleted.
The approach we take is to check that any of the
original StructLits with optional fields are associated
with both Vertices.
Note that it is somewhat okay for Equality to return false
negatives, as the spec does not guarantee this. At some
point we need to come up with something that is
more consistent.
This change leads to a reopening of Issue #353.
Change-Id: I7288061c7d3a51f7a754618c990a14e0275244d0
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8203
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cue/testdata/eval/closed_disjunction.txtar b/cue/testdata/eval/closed_disjunction.txtar
index 3bab475..b0c2132 100644
--- a/cue/testdata/eval/closed_disjunction.txtar
+++ b/cue/testdata/eval/closed_disjunction.txtar
@@ -32,8 +32,11 @@
Result:
(_|_){
// [eval]
- #A: (#struct){
- }
+ #A: (struct){ |(*(#struct){
+ }, (#struct){
+ }, (#struct){
+ }, (#struct){
+ }) }
a: (#struct){
b: (int){ 3 }
c: (int){ 3 }
diff --git a/cue/testdata/eval/issue353.txtar b/cue/testdata/eval/issue353.txtar
index 10216af..9b26d24 100644
--- a/cue/testdata/eval/issue353.txtar
+++ b/cue/testdata/eval/issue353.txtar
@@ -13,12 +13,16 @@
}
-- out/eval --
(struct){
- e: (#struct){
- a: (string){ "hello" }
- }
- #Example: (#struct){
- a: (string){ string }
- }
+ e: (struct){ |((#struct){
+ a: (string){ "hello" }
+ }, (#struct){
+ a: (string){ "hello" }
+ }) }
+ #Example: (struct){ |((#struct){
+ a: (string){ string }
+ }, (#struct){
+ a: (string){ string }
+ }) }
}
-- out/compile --
--- in.cue
diff --git a/internal/core/adt/equality.go b/internal/core/adt/equality.go
index c8a7d9c..ca7b41f 100644
--- a/internal/core/adt/equality.go
+++ b/internal/core/adt/equality.go
@@ -47,6 +47,9 @@
if x.IsClosed(ctx) != y.IsClosed(ctx) {
return false
}
+ if !equalOptional(ctx, x, y) {
+ return false
+ }
loop1:
for _, a := range x.Arcs {
@@ -81,6 +84,37 @@
return equalTerminal(ctx, v, w)
}
+// equalOptional tests if x and y have the same set of close information.
+// Right now this just checks if it has the same source structs that
+// define optional fields.
+// TODO: the following refinements are possible:
+// - unify optional fields and equate the optional fields
+// - do the same for pattern constraints, where the pattern constraints
+// are collated by pattern equality.
+// - a further refinement would collate patterns by ranges.
+//
+// For all these refinements it would be necessary to have well-working
+// structure sharing so as to not repeatedly recompute optional arcs.
+func equalOptional(ctx *OpContext, x, y *Vertex) bool {
+ return verifyStructs(x, y) && verifyStructs(y, x)
+}
+
+func verifyStructs(x, y *Vertex) bool {
+outer:
+ for _, s := range x.Structs {
+ if !s.StructLit.HasOptional() {
+ continue
+ }
+ for _, t := range y.Structs {
+ if s.StructLit == t.StructLit {
+ continue outer
+ }
+ }
+ return false
+ }
+ return true
+}
+
func equalTerminal(ctx *OpContext, v, w Value) bool {
if v == w {
return true
