internal/core/adt: fix default handling

This fixes a P1 bug in defaults for the new evaluator that
made default handling non-commutative. This fix, however,
also fixes some bugs that existed in the old evaluator. This
may cause some incompatibilities in the transition to the
new evaluator.

See the comments at the top of the file, as well as the changes
in specdeviation.txtar, for more details.

Issue #2884

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: Ic1742a6844fc4b9502a739a8a4ea8ac1ccbf46aa
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1191587
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>

cue/testdata/cycle/issue429.txtar

@@ -89,7 +89,7 @@ Result:
   s1: (#struct){
     min: (int){ 5 }
     res: (int){ |(*(int){ 0 }, (int){ &(>=0, int) }) }
-    max: (number){ |(*(int){ 5 }, (number){ >5 }) }
+    max: (number){ |((number){ >5 }, (int){ 5 }) }
   }
   s2: (#struct){
     max: (int){ 5 }
@@ -183,9 +183,10 @@ diff old new
    s1: (#struct){
 -    res: (int){ |(*(int){ 0 }, (int){ &(>=0, int) }) }
 -    min: (int){ 5 }
+-    max: (number){ |(*(int){ 5 }, (number){ >5 }) }
 +    min: (int){ 5 }
 +    res: (int){ |(*(int){ 0 }, (int){ &(>=0, int) }) }
-     max: (number){ |(*(int){ 5 }, (number){ >5 }) }
++    max: (number){ |((number){ >5 }, (int){ 5 }) }
    }
    s2: (#struct){
 -    res: (int){ |(*(int){ 0 }, (int){ &(>=0, int) }) }
@@ -278,6 +279,11 @@ diff old new
 #nonEmptyRange: missing disjunction error, or at least missing validation.
 At least this seems more correct than the old evaluator. It is an
 incomplete error at best.
+-- diff/explanation --
+s1.max: the changes in default behavior as are shown here are according to spec,
+as is described in disjunctions/specdeviation.txtar. These changes may pose too
+much of a problem for the transition to the new evaluator, though.
+TODO: consider reintroducing bugs.
 -- diff/todo/p3 --
 Reordering.
 Missing empty disjunction message.

cue/testdata/disjunctions/specdeviation.txtar

@@ -76,12 +76,12 @@ Disjuncts:    172
   Q: (int){ |(*(int){ 1 }, (int){ int }) }
   q: (int){ |(*(int){ 1 }, (int){ int }) }
   P: (int){ 2 }
-  p: (int){ |(*(int){ 2 }, (int){ int }) }
+  p: (int){ |((int){ 2 }, (int){ int }) }
   r: (int){ |((int){ 1 }, (int){ 2 }) }
   s: (int){ |(*(int){ 1 }, (int){ 2 }) }
   s1: (#struct){
     min: (int){ 5 }
-    max: (number){ |(*(int){ 5 }, (number){ >5 }) }
+    max: (number){ |((number){ >5 }, (int){ 5 }) }
     res: (int){ |(*(int){ 0 }, (int){ &(>=0, int) }) }
   }
   #Size: (#struct){
@@ -90,7 +90,7 @@ Disjuncts:    172
     min: (number){ |(*(int){ 1 }, (number){ >0 }) }
   }
   staged: (struct){
-    c: (string){ |(*(string){ "a" }, *(string){ "b" }) }
+    c: (string){ |(*(string){ "a" }, (string){ "b" }) }
     d: (string){ |(*(string){ "a" }, (string){ "b" }) }
   }
   issue763a: (struct){
@@ -114,29 +114,28 @@ Disjuncts:    172
 diff old new
 --- old
 +++ new
-@@ -6,9 +6,9 @@
+@@ -2,13 +2,13 @@
+   Q: (int){ |(*(int){ 1 }, (int){ int }) }
+   q: (int){ |(*(int){ 1 }, (int){ int }) }
+   P: (int){ 2 }
+-  p: (int){ |(*(int){ 2 }, (int){ int }) }
++  p: (int){ |((int){ 2 }, (int){ int }) }
    r: (int){ |((int){ 1 }, (int){ 2 }) }
    s: (int){ |(*(int){ 1 }, (int){ 2 }) }
    s1: (#struct){
 -    max: (number){ |(*(int){ 5 }, (number){ >5 }) }
 -    res: (int){ |(*(int){ 0 }, (int){ &(>=0, int) }) }
      min: (int){ 5 }
-+    max: (number){ |(*(int){ 5 }, (number){ >5 }) }
++    max: (number){ |((number){ >5 }, (int){ 5 }) }
 +    res: (int){ |(*(int){ 0 }, (int){ &(>=0, int) }) }
    }
    #Size: (#struct){
      max: (number){ |(*(int){ 1 }, (number){ >0 }, (number){ >1 }) }
-@@ -16,7 +16,7 @@
-     min: (number){ |(*(int){ 1 }, (number){ >0 }) }
-   }
-   staged: (struct){
--    c: (string){ |(*(string){ "a" }, (string){ "b" }) }
-+    c: (string){ |(*(string){ "a" }, *(string){ "b" }) }
-     d: (string){ |(*(string){ "a" }, (string){ "b" }) }
-   }
-   issue763a: (struct){
--- diff/todo/p1 --
-staged.c: default not discarded. At the very least it is not symmetrical with d.
+-- diff/explanation --
+The changes in default behavior as are shown here are according to spec, as is
+described at the top of the file. These changes may pose too much of a problem
+for the transition to the new evaluator, though.
+TODO: consider reintroducing bugs.
 -- diff/todo/p3 --
 Reordering.
 -- out/eval --

internal/core/adt/disjunct2.go

@@ -318,6 +318,7 @@ func (n *nodeContext) processDisjunctions() *Bottom {
 	case 1:
 		d := cross[0].node
 		n.node.BaseValue = d
+		n.defaultMode = cross[0].defaultMode
 
 	default:
 		// append, rather than assign, to allow reusing the memory of
@@ -411,6 +412,8 @@ func (n *nodeContext) doDisjunct(c Conjunct, m defaultMode, mode runMode) (*node
 
 	d := oc.cloneRoot(n)
 
+	d.defaultMode = combineDefault(m, n.defaultMode)
+
 	v := d.node
 
 	saved := n.node.BaseValue
@@ -425,7 +428,6 @@ func (n *nodeContext) doDisjunct(c Conjunct, m defaultMode, mode runMode) (*node
 
 	d.overlays = n
 	d.disjunctCCs = append(d.disjunctCCs, holes...)
-	d.defaultMode = combineDefault(m, n.defaultMode)
 	d.disjunct = c
 	c.CloseInfo.cc = ccHole
 	d.scheduleConjunct(c, c.CloseInfo)