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 <mpvl@golang.org>
diff --git a/cmd/cue/cmd/testdata/partial/eval_conc.out b/cmd/cue/cmd/testdata/partial/eval_conc.out
index 006c7ca..eae33c1 100644
--- a/cmd/cue/cmd/testdata/partial/eval_conc.out
+++ b/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
diff --git a/cue/ast_test.go b/cue/ast_test.go
index 96ea535..806cce1 100644
--- a/cue/ast_test.go
+++ b/cue/ast_test.go
@@ -291,7 +291,7 @@
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 {
diff --git a/cue/eval.go b/cue/eval.go
index 44db675..74256db 100644
--- a/cue/eval.go
+++ b/cue/eval.go
@@ -318,43 +318,40 @@
}
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
-
- case err != nil:
- return err
-
+ case len(values) > 0:
+ // values contains all the valid defaults
+ case !validValue:
+ return x
default:
- return ctx.mkErr(x, "empty disjunction")
+ for _, dv := range x.values {
+ dv.marked = false
+ values = append(values, dv)
+ }
}
+
+ switch len(values) {
+ case 0:
+ return x
+
+ 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) {
diff --git a/cue/evaluator.go b/cue/evaluator.go
index f8c7bb4..48ff71c 100644
--- a/cue/evaluator.go
+++ b/cue/evaluator.go
@@ -20,17 +20,15 @@
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
}
diff --git a/cue/export.go b/cue/export.go
index 54f5923..e856a30 100644
--- a/cue/export.go
+++ b/cue/export.go
@@ -524,11 +524,7 @@
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)
diff --git a/cue/export_test.go b/cue/export_test.go
index 392fb60..c3ca25b 100644
--- a/cue/export_test.go
+++ b/cue/export_test.go
@@ -160,8 +160,8 @@
}`,
out: unindent(`
{
- a: 1 | 2 | *_|_
- b: [1 | 2 | *_|_]
+ a: 1 | 2
+ b: [1 | 2]
}`),
}, {
raw: true,
diff --git a/cue/resolve_test.go b/cue/resolve_test.go
index c44856d..920465d 100644
--- a/cue/resolve_test.go
+++ b/cue/resolve_test.go
@@ -518,7 +518,7 @@
rewriteHelper(t, testCases, evalPartial)
}
-func TestChooseFirst(t *testing.T) {
+func TestChooseDefault(t *testing.T) {
testCases := []testCase{{
desc: "pick first",
in: `
@@ -531,13 +531,15 @@
`,
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 @@
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 @@
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 @@
`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)
}
diff --git a/doc/tutorial/basics/defaults.md b/doc/tutorial/basics/defaults.md
index c348c25..99d9e48 100644
--- a/doc/tutorial/basics/defaults.md
+++ b/doc/tutorial/basics/defaults.md
@@ -29,5 +29,5 @@
`$ cue eval defaults.cue`
```
replicas: 1
-protocol: "tcp" | "udp" | *_|_
+protocol: "tcp" | "udp"
```
\ No newline at end of file
diff --git a/doc/tutorial/kubernetes/testdata/manual.out b/doc/tutorial/kubernetes/testdata/manual.out
index ba2e1a8..5112aa9 100644
--- a/doc/tutorial/kubernetes/testdata/manual.out
+++ b/doc/tutorial/kubernetes/testdata/manual.out
@@ -1990,7 +1990,7 @@
containerPort: 7788
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "secret-volume"
mountPath: "/etc/ssl"
@@ -2230,7 +2230,7 @@
containerPort: 7443
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "secret-volume"
mountPath: "/etc/ssl"
@@ -2436,7 +2436,7 @@
containerPort: 8080
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "secret-updater"
mountPath: "/etc/certs"
@@ -2627,7 +2627,7 @@
containerPort: 7788
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "secret-volume"
mountPath: "/etc/ssl"
@@ -2897,18 +2897,18 @@
containerPort: 8080
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "ssd-caller"
mountPath: "/logs"
}, {
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "secret-ssh-key"
mountPath: "/sslcerts"
readOnly: true
}, {
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "secret-caller"
mountPath: "/etc/certs"
@@ -3177,18 +3177,18 @@
containerPort: 8080
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "secret-ssh-key"
mountPath: "/sslcerts"
readOnly: true
}, {
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "dishwasher-disk"
mountPath: "/logs"
}, {
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "secret-dishwasher"
mountPath: "/etc/certs"
@@ -3454,12 +3454,12 @@
containerPort: 8080
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "expiditer-disk"
mountPath: "/logs"
}, {
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "secret-expiditer"
mountPath: "/etc/certs"
@@ -3710,13 +3710,13 @@
containerPort: 8080
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "secret-headchef"
mountPath: "/sslcerts"
readOnly: true
}, {
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "headchef-disk"
mountPath: "/logs"
@@ -3965,13 +3965,13 @@
containerPort: 8080
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "secret-kitchen"
mountPath: "/etc/certs"
readOnly: true
}, {
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "linecook-disk"
mountPath: "/logs"
@@ -4224,13 +4224,13 @@
containerPort: 8080
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "secret-ssh-key"
mountPath: "/etc/certs"
readOnly: true
}, {
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "pastrychef-disk"
mountPath: "/logs"
@@ -4763,12 +4763,12 @@
containerPort: 9093
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "alertmanager"
mountPath: "/alertmanager"
}, {
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "config-volume"
mountPath: "/etc/alertmanager"
@@ -5041,7 +5041,7 @@
containerPort: 8080
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "grafana-volume"
mountPath: "/var/lib/grafana"
@@ -5278,13 +5278,13 @@
hostPort: 9100
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "proc"
mountPath: "/host/proc"
readOnly: true
}, {
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "sys"
mountPath: "/host/sys"
@@ -5538,7 +5538,7 @@
containerPort: 9090
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "config-volume"
mountPath: "/etc/prometheus"
@@ -6294,7 +6294,7 @@
containerPort: 4180
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "config-volume"
mountPath: "/etc/authproxy"
@@ -6606,7 +6606,7 @@
containerPort: 7443
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "secret-volume"
mountPath: "/etc/ssl"
@@ -6810,7 +6810,7 @@
containerPort: 443
}]
volumeMounts: [{
- _|_ /* more than one element remaining (false and true) */
+ _|_ /* non-concrete value bool */
name: "secret-volume"
mountPath: "/etc/ssl"
diff --git a/internal/cuetest/sim.go b/internal/cuetest/sim.go
index ed105c7..c14a0c1 100644
--- a/internal/cuetest/sim.go
+++ b/internal/cuetest/sim.go
@@ -93,7 +93,7 @@
want := strings.TrimSpace(cfg.Golden)
if got != want {
- t.Errorf("files differ:\n%s", diff.Diff(want, got))
+ t.Errorf("files differ:\n%s", diff.Diff(got, want))
}
}
