internal/core/adt: fix default logic

We also need to consider both default modes of
the previous pass to work in all conditions.

Also renamed the variable names to reflect more
closely what is being tracked: we do not track
whether there is a default, but rather whether all
defaults are eliminated when taking a cross
product of a single disjunction.

Fixes #3958

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: I8b0b4b59c436a19778a4d1315588e950ae5d1487
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1216865
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1217391
Reviewed-by: Roger Peppe <[email protected]>

Author: mpvl

cue/testdata/disjunctions/elimination.txtar

@@ -1747,12 +1747,12 @@ issue3958: t2: {
   issue3958: (struct){
     t1: (struct){
       #schema: (int){ |((int){ int }, (int){ 3 }, (int){ 1 }) }
-      test1: (int){ |(*(int){ 3 }, (int){ int }, (int){ 1 }, (int){ 2 }) }
-      test2: (int){ |(*(int){ 3 }, (int){ int }, (int){ 1 }, (int){ 4 }) }
+      test1: (int){ |((int){ int }, (int){ 3 }, (int){ 1 }, (int){ 2 }) }
+      test2: (int){ |((int){ int }, (int){ 3 }, (int){ 1 }, (int){ 4 }) }
     }
     t2: (struct){
       a: (int){ |((int){ int }, (int){ 8 }, (int){ 3 }, (int){ 1 }, (int){ 4 }) }
-      b: (int){ |(*(int){ 3 }, (int){ int }, (int){ 1 }, (int){ 4 }) }
+      b: (int){ |((int){ int }, (int){ 3 }, (int){ 1 }, (int){ 4 }) }
     }
   }
   issue770: (struct){
@@ -2631,22 +2631,17 @@ diff old new
        }
      }
    }
-@@ -1204,12 +1085,12 @@
+@@ -1204,8 +1085,8 @@
    issue3958: (struct){
      t1: (struct){
        #schema: (int){ |((int){ int }, (int){ 3 }, (int){ 1 }) }
 -      test1: (int){ |((int){ int }, (int){ 2 }, (int){ 3 }, (int){ 1 }) }
 -      test2: (int){ |((int){ int }, (int){ 4 }, (int){ 3 }, (int){ 1 }) }
-+      test1: (int){ |(*(int){ 3 }, (int){ int }, (int){ 1 }, (int){ 2 }) }
-+      test2: (int){ |(*(int){ 3 }, (int){ int }, (int){ 1 }, (int){ 4 }) }
++      test1: (int){ |((int){ int }, (int){ 3 }, (int){ 1 }, (int){ 2 }) }
++      test2: (int){ |((int){ int }, (int){ 3 }, (int){ 1 }, (int){ 4 }) }
      }
      t2: (struct){
        a: (int){ |((int){ int }, (int){ 8 }, (int){ 3 }, (int){ 1 }, (int){ 4 }) }
--      b: (int){ |((int){ int }, (int){ 3 }, (int){ 1 }, (int){ 4 }) }
-+      b: (int){ |(*(int){ 3 }, (int){ int }, (int){ 1 }, (int){ 4 }) }
-     }
-   }
-   issue770: (struct){
 @@ -1227,10 +1108,10 @@
          v: (string){ |(*(string){ "a" }, (string){ "b" }, (string){ "c" }) }
        }

internal/core/adt/disjunct2.go

@@ -383,8 +383,8 @@ func (n *nodeContext) crossProduct(dst, cross []*nodeContext, dn *envDisjunct, m
 	// buffer may grow and has a max size of len(cross) * len(dn.disjuncts).
 	tmp := make([]*nodeContext, 0, len(cross))
 
-	leftHasDefault := false
-	rightHasDefault := false
+	leftDropsDefault := true
+	rightDropsDefault := true
 
 	for i, p := range cross {
 		ID := n.nextCrossProduct(i, len(cross), p)
@@ -406,20 +406,21 @@ func (n *nodeContext) crossProduct(dst, cross []*nodeContext, dn *envDisjunct, m
 			}
 
 			tmp = append(tmp, r)
-			if p.defaultMode == isDefault {
-				leftHasDefault = true
+			if p.defaultMode == isDefault || p.origDefaultMode == isDefault {
+				leftDropsDefault = false
 			}
 			if d.mode == isDefault {
-				rightHasDefault = true
+				rightDropsDefault = false
 			}
 		}
 	}
 
+	hasNonMaybe := false
 	for _, r := range tmp {
 		// Unroll nested disjunctions.
 		switch len(r.disjuncts) {
 		case 0:
-			r.defaultMode = combineDefault2(r.defaultMode, r.origDefaultMode, leftHasDefault, rightHasDefault)
+			r.defaultMode = combineDefault2(r.defaultMode, r.origDefaultMode, leftDropsDefault, rightDropsDefault)
 			// r did not have a nested disjunction.
 			dst = appendDisjunct(n.ctx, dst, r)
 
@@ -430,23 +431,34 @@ func (n *nodeContext) crossProduct(dst, cross []*nodeContext, dn *envDisjunct, m
 			for _, x := range r.disjuncts {
 				m := combineDefault(r.origDefaultMode, x.defaultMode)
 
-				// TODO(defaults): using rightHasDefault instead of true here is
-				// not according to the spec, but may result in better user
+				// TODO(defaults): using rightHasDefault instead of false here
+				// is not according to the spec, but may result in better user
 				// ergononmics. See Issue #1304.
-				x.defaultMode = combineDefault2(r.defaultMode, m, leftHasDefault, true)
+				x.defaultMode = combineDefault2(r.defaultMode, m, leftDropsDefault, false)
+				if x.defaultMode != maybeDefault {
+					hasNonMaybe = true
+				}
 				dst = appendDisjunct(n.ctx, dst, x)
 			}
 		}
 	}
 
+	if hasNonMaybe {
+		for _, r := range dst {
+			if r.defaultMode == maybeDefault {
+				r.defaultMode = notDefault
+			}
+		}
+	}
+
 	return dst
 }
 
-func combineDefault2(a, b defaultMode, hasDefaultA, hasDefaultB bool) defaultMode {
-	if !hasDefaultA {
+func combineDefault2(a, b defaultMode, dropsDefaultA, dropsDefaultB bool) defaultMode {
+	if dropsDefaultA {
 		a = maybeDefault
 	}
-	if !hasDefaultB {
+	if dropsDefaultB {
 		b = maybeDefault
 	}
 	return combineDefault(a, b)