cue: treat disjunctions as normal non-concrete values

This brings the implementation in line with the spec
relating to the treatment of default values.

Before getting a default resulted in a "multiple
values remaining" error if the value was a disjunction.
However, there is nothing special about such
disjuctions compared to any other non-concrete
value. Generating an error early resulted in some
strange and sometimes spurious error messages.

Issue #52

Change-Id: I221ff8affc6469b3caf633be0674dfd265ea8b0f
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/2680
Reviewed-by: Marcel van Lohuizen <[email protected]>

Author: mpvl

cmd/cue/cmd/testdata/partial/eval_conc.out

@@ -1,4 +1,4 @@
-sum: more than one element remaining (1 and 2):
+sum: incomplete value ((1 | 2)):
     ./testdata/partial/partial.cue:4:6
 b.idx: invalid non-ground value string (must be concrete int|string):
     ./testdata/partial/partial.cue:7:9

cue/ast_test.go

@@ -291,7 +291,7 @@ func TestEmit(t *testing.T) {
 			a: 8000 | 7080
 			a: 7080 | int
 		}`,
-		out: `<0>{a: _|_((8000 | 7080):more than one element remaining (8000 and 7080))}`,
+		out: `<0>{a: (8000 | 7080)}`,
 		rw:  evalFull,
 	}}
 	for _, tc := range testCases {

cue/eval.go

@@ -318,43 +318,40 @@ func (x *disjunction) evalPartial(ctx *context) (result evaluated) {
 }
 
 func (x *disjunction) manifest(ctx *context) (result evaluated) {
-	var err, marked, unmarked1, unmarked2 evaluated
-	for _, d := range x.values {
-		// Because of the lazy evaluation strategy, we may still have
-		// latent unification.
-		if err := validate(ctx, d.val); err != nil {
-			continue
-		}
+	values := make([]dValue, 0, len(x.values))
+	validValue := false
+	for _, dv := range x.values {
 		switch {
-		case d.marked:
-			if marked != nil {
-				// TODO: allow disjunctions to be returned as is.
-				return ctx.mkErr(x, codeIncomplete, "more than one default remaining (%v and %v)", ctx.str(marked), ctx.str(d.val))
-			}
-			marked = d.val.(evaluated)
-		case unmarked1 == nil:
-			unmarked1 = d.val.(evaluated)
+		case isBottom(dv.val):
+		case dv.marked:
+			values = append(values, dv)
 		default:
-			unmarked2 = d.val.(evaluated)
+			validValue = true
 		}
 	}
-	switch {
-	case marked != nil:
-		return marked
-
-	case unmarked2 != nil:
-		return ctx.mkErr(x, codeIncomplete, "more than one element remaining (%v and %v)",
-			ctx.str(unmarked1), ctx.str(unmarked2))
 
-	case unmarked1 != nil:
-		return unmarked1
+	switch {
+	case len(values) > 0:
+		// values contains all the valid defaults
+	case !validValue:
+		return x
+	default:
+		for _, dv := range x.values {
+			dv.marked = false
+			values = append(values, dv)
+		}
+	}
 
-	case err != nil:
-		return err
+	switch len(values) {
+	case 0:
+		return x
 
-	default:
-		return ctx.mkErr(x, "empty disjunction")
+	case 1:
+		return values[0].val.evalPartial(ctx)
 	}
+
+	x = &disjunction{x.baseValue, values, true}
+	return x.normalize(ctx, x).val
 }
 
 func (x *binaryExpr) evalPartial(ctx *context) (result evaluated) {

cue/evaluator.go

@@ -20,17 +20,15 @@ func (c *context) manifest(v value) evaluated {
 		return evaluated
 	}
 outer:
-	for {
-		switch x := evaluated.(type) {
-		case *disjunction:
-			evaluated = x.manifest(c)
+	switch x := evaluated.(type) {
+	case *disjunction:
+		evaluated = x.manifest(c)
 
-		case *list:
-			return x.manifest(c)
+	case *list:
+		return x.manifest(c)
 
-		default:
-			break outer
-		}
+	default:
+		break outer
 	}
 	return evaluated
 }

cue/export.go

@@ -524,11 +524,7 @@ func (p *exporter) expr(v value) ast.Expr {
 		list := &ast.ListLit{}
 		var expr ast.Expr = list
 		for _, e := range x.elem.arcs {
-			if doEval(p.mode) {
-				list.Elts = append(list.Elts, p.expr(e.v.evalPartial(p.ctx)))
-			} else {
-				list.Elts = append(list.Elts, p.expr(e.v))
-			}
+			list.Elts = append(list.Elts, p.expr(e.v))
 		}
 		max := maxNum(x.len)
 		num, ok := max.(*numLit)

cue/export_test.go

@@ -160,8 +160,8 @@ func TestExport(t *testing.T) {
 			}`,
 		out: unindent(`
 			{
-				a: 1 | 2 | *_|_
-				b: [1 | 2 | *_|_]
+				a: 1 | 2
+				b: [1 | 2]
 			}`),
 	}, {
 		raw:  true,

cue/resolve_test.go

@@ -518,7 +518,7 @@ func TestBasicRewrite(t *testing.T) {
 	rewriteHelper(t, testCases, evalPartial)
 }
 
-func TestChooseFirst(t *testing.T) {
+func TestChooseDefault(t *testing.T) {
 	testCases := []testCase{{
 		desc: "pick first",
 		in: `
@@ -531,13 +531,15 @@ func TestChooseFirst(t *testing.T) {
 		`,
 		out: "<0>{a: 5, b: <1>{c: <2>{a: 2}}}",
 	}, {
+		// In this test, default results to bottom, meaning that the non-default
+		// value remains.
 		desc: "simple disambiguation conflict",
 		in: `
 			a: *"a" | "b"
 			b: *"b" | "a"
 			c: a & b
 			`,
-		out: `<0>{a: "a", b: "b", c: _|_(("a" | "b" | *_|_):more than one element remaining ("a" and "b"))}`,
+		out: `<0>{a: "a", b: "b", c: ("a" | "b")}`,
 	}, {
 		desc: "associativity of defaults",
 		in: `
@@ -547,7 +549,7 @@ func TestChooseFirst(t *testing.T) {
 			x: a & b
 			y: b & c
 			`,
-		out: `<0>{a: "a", b: "a", c: _|_((*"a" | *"b" | "c"):more than one default remaining ("a" and "b")), x: "a", y: _|_((*"a" | *"b" | "c"):more than one default remaining ("a" and "b"))}`,
+		out: `<0>{a: "a", b: "a", c: (*"a" | *"b"), x: "a", y: (*"a" | *"b")}`,
 	}}
 	rewriteHelper(t, testCases, evalFull)
 }
@@ -1910,7 +1912,7 @@ func TestFullEval(t *testing.T) {
 			x: {a:1}|{a:2}
 			y: x & {a:3}
 		`,
-		out: `<0>{x: _|_((<1>{a: 1} | <2>{a: 2}):more than one element remaining ({a: 1} and {a: 2})), y: _|_((<3>.x & <4>{a: 3}):empty disjunction: {a: (1 & 3)})}`,
+		out: `<0>{x: (<1>{a: 1} | <2>{a: 2}), y: _|_((<3>.x & <4>{a: 3}):empty disjunction: {a: (1 & 3)})}`,
 	}, {
 		desc: "cannot resolve references that would be ambiguous",
 		in: `
@@ -1930,10 +1932,10 @@ func TestFullEval(t *testing.T) {
 			`a1: _|_(((*0 | 1) & (<1>.a3 - <1>.a2)):cycle detected), ` +
 			`a3: 1, ` +
 			`a2: _|_(((*0 | 1) & (<1>.a3 - <1>.a1)):cycle detected), ` +
-			`b1: _|_((0 | 1 | *_|_):more than one element remaining (0 and 1)), ` +
-			`b2: _|_((0 | 1 | *_|_):more than one element remaining (0 and 1)), ` +
-			`c1: _|_((<2>{a: 1, b: 2} | <3>{a: 2, b: 1} | *_|_):more than one element remaining ({a: 1, b: 2} and {a: 2, b: 1})), ` +
-			`c2: _|_((<4>{a: 2, b: 1} | <5>{a: 1, b: 2} | *_|_):more than one element remaining ({a: 2, b: 1} and {a: 1, b: 2}))}`,
+			`b1: (0 | 1), ` +
+			`b2: (0 | 1), ` +
+			`c1: (<2>{a: 1, b: 2} | <3>{a: 2, b: 1}), ` +
+			`c2: (<4>{a: 2, b: 1} | <5>{a: 1, b: 2})}`,
 	}}
 	rewriteHelper(t, testCases, evalFull)
 }

doc/tutorial/basics/defaults.md

@@ -29,5 +29,5 @@ protocol: *"udp" | "tcp"
 `$ cue eval defaults.cue`
 ```
 replicas: 1
-protocol: "tcp" | "udp" | *_|_
+protocol: "tcp" | "udp"
 ```