internal/core/adt: fix bug that left errors of incompleted expressions unreported
This also requires simplification of BoundExpr to simulate
complete cycle handling.
Fixes #475
Fixes #680
Fixes #684
Change-Id: I906d8cc6e3689abb74c7c1ebc2ba96ecd98aaf4c
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8323
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cue/testdata/builtins/incomplete.txtar b/cue/testdata/builtins/incomplete.txtar
index 76954a4..f8d346f 100644
--- a/cue/testdata/builtins/incomplete.txtar
+++ b/cue/testdata/builtins/incomplete.txtar
@@ -79,9 +79,13 @@
// [incomplete] list2.#Sub: undefined field b:
// ./in.cue:30:13
}
- Out2: (list){
+ Out2: (_|_){
+ // [incomplete] list2.#Sub: undefined field b:
+ // ./in.cue:30:13
}
- Out3: (list){
+ Out3: (_|_){
+ // [incomplete] list2.#Sub: undefined field b:
+ // ./in.cue:30:13
}
_Top: (_|_){
// [incomplete] list2.#Sub: undefined field b:
diff --git a/cue/testdata/cycle/051_resolved_self-reference_cycles_with_disjunction.txtar b/cue/testdata/cycle/051_resolved_self-reference_cycles_with_disjunction.txtar
index af4e0c3..2d2d32d 100644
--- a/cue/testdata/cycle/051_resolved_self-reference_cycles_with_disjunction.txtar
+++ b/cue/testdata/cycle/051_resolved_self-reference_cycles_with_disjunction.txtar
@@ -186,7 +186,10 @@
}
xc1: (int){ |((int){ 8 }, (int){ 9 }) }
xc2: (int){ 8 }
- xc3: (int){ 6 }
+ xc3: (_|_){
+ // [incomplete] xc3: unresolved disjunction 8 | 9 (type int):
+ // ./in.cue:29:10
+ }
xc4: (int){ 9 }
xc5: (int){ 10 }
xd1: (int){ 8 }
@@ -235,5 +238,8 @@
}
z1: (int){ |((int){ 11 }, (int){ 13 }) }
z2: (int){ 10 }
- z3: (int){ 8 }
+ z3: (_|_){
+ // [incomplete] z3: unresolved disjunction 11 | 13 (type int):
+ // ./in.cue:58:5
+ }
}
diff --git a/cue/testdata/eval/bounds.txtar b/cue/testdata/eval/bounds.txtar
index 8ca60f3..45fa97b 100644
--- a/cue/testdata/eval/bounds.txtar
+++ b/cue/testdata/eval/bounds.txtar
@@ -10,8 +10,67 @@
b5: >=21 & >20
b6: int & >5 & <= 6
+simplifyExpr: {
+ less1: <(<3)
+ less2: <(<=3)
+ less3: <=(<3)
+ less4: <=(<=3)
+ less5: <(!=3)
+ less6: <=(!=3)
+
+ gtr1: >(>3)
+ gtr2: >(>=3)
+ gtr3: >=(>3)
+ gtr4: >=(>=3)
+ gtr5: >(!=3)
+ gtr6: >=(!=3)
+
+ lg1: <(>3)
+ lg2: <(>=3)
+ lg3: <=(>3)
+ lg4: <=(>=3)
+
+ gl1: >(<3)
+ gl2: >(<=3)
+ gl3: >=(<3)
+ gl4: >=(<=3)
+
+ ne1: !=(!=3)
+ ne2: !=(<3)
+ ne3: !=(<=3)
+ ne4: !=(>3)
+ ne5: !=(>=3)
+
+ s: string
+ n: number
+ i: int
+ f: float
+ b: bytes
+ basic1: <(i)
+ basic2: >(n)
+ basic3: >=(s)
+ basic4: <=(f)
+ basic5: <=(b)
+
+ // Do NOT interpret this the same as `!= type`.
+ bne1: !=(s)
+ bne2: !=(n)
+ bne3: !=(n)
+ bne4: !=(i)
+ bne5: !=(b)
+
+ e1: <(=~"foo")
+ e2: >(null)
+}
+
-- out/eval --
-(struct){
+Errors:
+simplifyExpr.e2: cannot use null for bound >:
+ ./in.cue:62:11
+
+Result:
+(_|_){
+ // [eval]
x0: (int){ 5 }
x1: (int){ 30 }
b0: (number){ &(>0, <5) }
@@ -21,6 +80,72 @@
b4: (number){ >20 }
b5: (number){ >=21 }
b6: (int){ 6 }
+ simplifyExpr: (_|_){
+ // [eval]
+ less1: (number){ <3 }
+ less2: (number){ <3 }
+ less3: (number){ <3 }
+ less4: (number){ <=3 }
+ less5: (number){ number }
+ less6: (number){ number }
+ gtr1: (number){ >3 }
+ gtr2: (number){ >3 }
+ gtr3: (number){ >3 }
+ gtr4: (number){ >=3 }
+ gtr5: (number){ number }
+ gtr6: (number){ number }
+ lg1: (number){ number }
+ lg2: (number){ number }
+ lg3: (number){ number }
+ lg4: (number){ number }
+ gl1: (number){ number }
+ gl2: (number){ number }
+ gl3: (number){ number }
+ gl4: (number){ number }
+ ne1: (int){ 3 }
+ ne2: (number){ >=3 }
+ ne3: (number){ >3 }
+ ne4: (number){ <=3 }
+ ne5: (number){ <3 }
+ s: (string){ string }
+ n: (number){ number }
+ i: (int){ int }
+ f: (float){ float }
+ b: (bytes){ bytes }
+ basic1: (int){ int }
+ basic2: (number){ number }
+ basic3: (string){ string }
+ basic4: (float){ float }
+ basic5: (bytes){ bytes }
+ bne1: (_|_){
+ // [incomplete] simplifyExpr.bne1: non-concrete value s for bound !=:
+ // ./in.cue:55:11
+ }
+ bne2: (_|_){
+ // [incomplete] simplifyExpr.bne2: non-concrete value n for bound !=:
+ // ./in.cue:56:11
+ }
+ bne3: (_|_){
+ // [incomplete] simplifyExpr.bne3: non-concrete value n for bound !=:
+ // ./in.cue:57:11
+ }
+ bne4: (_|_){
+ // [incomplete] simplifyExpr.bne4: non-concrete value i for bound !=:
+ // ./in.cue:58:11
+ }
+ bne5: (_|_){
+ // [incomplete] simplifyExpr.bne5: non-concrete value b for bound !=:
+ // ./in.cue:59:11
+ }
+ e1: (_|_){
+ // [incomplete] simplifyExpr.e1: non-concrete value =~"foo" for bound <:
+ // ./in.cue:61:9
+ }
+ e2: (_|_){
+ // [eval] simplifyExpr.e2: cannot use null for bound >:
+ // ./in.cue:62:11
+ }
+ }
}
-- out/compile --
--- in.cue
@@ -34,4 +159,48 @@
b4: (>=20 & >20)
b5: (>=21 & >20)
b6: ((int & >5) & <=6)
+ simplifyExpr: {
+ less1: <<3
+ less2: <<=3
+ less3: <=<3
+ less4: <=<=3
+ less5: <!=3
+ less6: <=!=3
+ gtr1: >>3
+ gtr2: >>=3
+ gtr3: >=>3
+ gtr4: >=>=3
+ gtr5: >!=3
+ gtr6: >=!=3
+ lg1: <>3
+ lg2: <>=3
+ lg3: <=>3
+ lg4: <=>=3
+ gl1: ><3
+ gl2: ><=3
+ gl3: >=<3
+ gl4: >=<=3
+ ne1: !=!=3
+ ne2: !=<3
+ ne3: !=<=3
+ ne4: !=>3
+ ne5: !=>=3
+ s: string
+ n: number
+ i: int
+ f: float
+ b: bytes
+ basic1: <〈0;i〉
+ basic2: >〈0;n〉
+ basic3: >=〈0;s〉
+ basic4: <=〈0;f〉
+ basic5: <=〈0;b〉
+ bne1: !=〈0;s〉
+ bne2: !=〈0;n〉
+ bne3: !=〈0;n〉
+ bne4: !=〈0;i〉
+ bne5: !=〈0;b〉
+ e1: <=~"foo"
+ e2: >null
+ }
}
diff --git a/cue/testdata/eval/errunifiy.txtar b/cue/testdata/eval/errunifiy.txtar
index bcb09aa..f818cbb 100644
--- a/cue/testdata/eval/errunifiy.txtar
+++ b/cue/testdata/eval/errunifiy.txtar
@@ -15,7 +15,10 @@
Result:
(_|_){
// [user]
- a: (string){ "t" }
+ a: (_|_){
+ // [incomplete] empty list in call to or:
+ // ./in.cue:1:4
+ }
b: (_|_){
// [user] explicit error (_|_ literal) in source:
// ./in.cue:4:4
diff --git a/cue/testdata/eval/incomplete.txtar b/cue/testdata/eval/incomplete.txtar
index 2fb31ea..daa5c6e 100644
--- a/cue/testdata/eval/incomplete.txtar
+++ b/cue/testdata/eval/incomplete.txtar
@@ -1,11 +1,12 @@
-These should all be an incomplete errors.
+These should all be incomplete errors.
-- in.cue --
s: string
e1: s + s
-e2: >"bar" & s // TODO
-e3: >s & "foo" // TODO
+e2: >"bar" & s // okay
+e3: >s & "foo" // okay
+e3b: >s
e4: >e1 & s
e5: <e5 & s
@@ -22,10 +23,13 @@
a: "golang/go:1.13.5"
}
+issue680: (>10 * 2) & 0
+
+
-- out/eval --
Errors:
permanentlyIncompleteOperands.a: invalid operand string ('+' requires concrete value):
- ./in.cue:18:8
+ ./in.cue:19:8
Result:
(_|_){
@@ -37,39 +41,44 @@
}
e2: (string){ >"bar" }
e3: (string){ "foo" }
+ e3b: (string){ string }
e4: (_|_){
// [incomplete] e1: non-concrete value string in operand to +:
// ./in.cue:3:5
}
e5: (_|_){
// [cycle] cycle error:
- // ./in.cue:8:6
+ // ./in.cue:9:6
}
E: (struct){
a: (_|_){
// [cycle] cycle error:
- // ./in.cue:11:6
+ // ./in.cue:12:6
}
b: (_|_){
// [cycle] cycle error:
- // ./in.cue:11:6
- // cycle error:
// ./in.cue:12:6
+ // cycle error:
+ // ./in.cue:13:6
}
c: (_|_){
// [cycle] cycle error:
- // ./in.cue:11:6
- // cycle error:
// ./in.cue:12:6
+ // cycle error:
+ // ./in.cue:13:6
}
}
permanentlyIncompleteOperands: (_|_){
// [eval]
a: (_|_){
// [eval] permanentlyIncompleteOperands.a: invalid operand string ('+' requires concrete value):
- // ./in.cue:18:8
+ // ./in.cue:19:8
}
}
+ issue680: (_|_){
+ // [incomplete] issue680: non-concrete value >10 in operand to *:
+ // ./in.cue:23:12
+ }
}
-- out/compile --
--- in.cue
@@ -78,6 +87,7 @@
e1: (〈0;s〉 + 〈0;s〉)
e2: (>"bar" & 〈0;s〉)
e3: (>〈0;s〉 & "foo")
+ e3b: >〈0;s〉
e4: (>〈0;e1〉 & 〈0;s〉)
e5: (<〈0;e5〉 & 〈0;s〉)
E: {
@@ -89,4 +99,5 @@
a: ((string + ":") + string)
a: "golang/go:1.13.5"
}
+ issue680: ((>10 * 2) & 0)
}
diff --git a/cue/testdata/export/009.txtar b/cue/testdata/export/009.txtar
index a6f04b7..7ab8e78 100644
--- a/cue/testdata/export/009.txtar
+++ b/cue/testdata/export/009.txtar
@@ -81,15 +81,21 @@
3: (int){ int }
4: (int){ int }
}
- b: (list){
+ b: (_|_){
+ // [incomplete] b: non-concrete value <=5 in operand to *:
+ // ./in.cue:4:5
0: (int){ 1 }
1: (int){ 2 }
}
- c: (list){
+ c: (_|_){
+ // [incomplete] c: non-concrete value >=3 & <=5 in operand to *:
+ // ./in.cue:6:5
0: (int){ 1 }
1: (int){ 2 }
}
- d: (list){
+ d: (_|_){
+ // [incomplete] d: non-concrete value >=2 in operand to *:
+ // ./in.cue:8:5
0: (int){ 1 }
1: (int){ 2 }
}
diff --git a/cue/testdata/export/010.txtar b/cue/testdata/export/010.txtar
index be462a4..bbaaec3 100644
--- a/cue/testdata/export/010.txtar
+++ b/cue/testdata/export/010.txtar
@@ -82,15 +82,21 @@
3: (int){ int }
4: (int){ int }
}
- b: (list){
+ b: (_|_){
+ // [incomplete] b: non-concrete value <=5 in operand to *:
+ // ./in.cue:4:5
0: (int){ 1 }
1: (int){ 2 }
}
- c: (list){
+ c: (_|_){
+ // [incomplete] c: non-concrete value >=3 & <=5 in operand to *:
+ // ./in.cue:6:5
0: (int){ 1 }
1: (int){ 2 }
}
- d: (list){
+ d: (_|_){
+ // [incomplete] d: non-concrete value >=2 in operand to *:
+ // ./in.cue:8:5
0: (int){ 1 }
1: (int){ 2 }
}
diff --git a/internal/core/adt/eval.go b/internal/core/adt/eval.go
index e42a930..d360086 100644
--- a/internal/core/adt/eval.go
+++ b/internal/core/adt/eval.go
@@ -308,6 +308,7 @@
n.expandDisjuncts(disState, n, maybeDefault, false)
for _, d := range n.disjuncts {
+ d.finalizeIncomplete()
d.free()
}
@@ -351,6 +352,10 @@
// to not confuse memory management.
v.state = n
+ // We don't do this in postDisjuncts, as it should only be done after
+ // completing all disjunctions.
+ n.finalizeIncomplete()
+
if state != Finalized {
return
}
@@ -371,6 +376,17 @@
}
}
+// finalizeIncomplete collects all uncompleted expressions and adds them as
+// errors. As disjuncts are always evaluated with Finalized, care should be
+// taken to only call this after all disjunctions in a path have been completed.
+func (n *nodeContext) finalizeIncomplete() {
+ if !n.done() {
+ if err := n.incompleteErrors(); err != nil {
+ n.node.BaseValue = err
+ }
+ }
+}
+
func (n *nodeContext) doNotify() {
if n.errs == nil || len(n.notify) == 0 {
return
@@ -389,11 +405,6 @@
n.notify = n.notify[:0]
}
-func isStruct(v *Vertex) bool {
- _, ok := v.BaseValue.(*StructMarker)
- return ok
-}
-
func (n *nodeContext) postDisjunct(state VertexStatus) {
ctx := n.ctx
@@ -426,6 +437,16 @@
n.node.BaseValue = nil
}
}
+ // TODO: this ideally should be done here. However, doing so causes
+ // a somewhat more aggressive cutoff in disjunction cycles, which cause
+ // some incompatibilities. Fix in another CL.
+ //
+ // else if !n.done() {
+ // n.expandOne()
+ // if err := n.incompleteErrors(); err != nil {
+ // n.node.BaseValue = err
+ // }
+ // }
// We are no longer evaluating.
// n.node.UpdateStatus(Partial)
diff --git a/internal/core/adt/expr.go b/internal/core/adt/expr.go
index 00a871e..4551069 100644
--- a/internal/core/adt/expr.go
+++ b/internal/core/adt/expr.go
@@ -452,15 +452,110 @@
}
func (x *BoundExpr) evaluate(ctx *OpContext) Value {
+ v := ctx.value(x.Expr)
+ if isError(v) {
+ return v
+ }
+
+ switch k := v.Kind(); k {
+ case IntKind, FloatKind, NumKind, StringKind, BytesKind:
+ case NullKind:
+ if x.Op != NotEqualOp {
+ err := ctx.NewPosf(pos(x.Expr),
+ "cannot use null for bound %s", x.Op)
+ return &Bottom{Err: err}
+ }
+ default:
+ mask := IntKind | FloatKind | NumKind | StringKind | BytesKind
+ if x.Op == NotEqualOp {
+ mask |= NullKind
+ }
+ if k&mask != 0 {
+ ctx.addErrf(IncompleteError, ctx.pos(),
+ "non-concrete value %s for bound %s", ctx.Str(x.Expr), x.Op)
+ return nil
+ }
+ err := ctx.NewPosf(pos(x.Expr),
+ "invalid value %s (type %s) for bound %s",
+ ctx.Str(v), k, x.Op)
+ return &Bottom{Err: err}
+ }
+
if v, ok := x.Expr.(Value); ok {
if v == nil || v.Concreteness() > Concrete {
return ctx.NewErrf("bound has fixed non-concrete value")
}
return &BoundValue{x.Src, x.Op, v}
}
- v := ctx.value(x.Expr)
- if isError(v) {
- return v
+
+ // This simplifies boundary expressions. It is an alternative to an
+ // evaluation strategy that makes nodes increasingly more specific.
+ //
+ // For instance, a completely different implementation would be to allow
+ // the precense of a concrete value to ignore incomplete errors.
+ //
+ // TODO: consider an alternative approach.
+ switch y := v.(type) {
+ case *BoundValue:
+ switch {
+ case y.Op == NotEqualOp:
+ switch x.Op {
+ case LessEqualOp, LessThanOp, GreaterEqualOp, GreaterThanOp:
+ // <(!=3) => number
+ // Smaller than an arbitrarily large number is any number.
+ return &BasicType{K: y.Kind()}
+ case NotEqualOp:
+ // !=(!=3) ==> 3
+ // Not a value that is anything but a given value is that
+ // given value.
+ return y.Value
+ }
+
+ case x.Op == NotEqualOp:
+ // Invert if applicable.
+ switch y.Op {
+ case LessEqualOp:
+ return &BoundValue{x.Src, GreaterThanOp, y.Value}
+ case LessThanOp:
+ return &BoundValue{x.Src, GreaterEqualOp, y.Value}
+ case GreaterEqualOp:
+ return &BoundValue{x.Src, LessThanOp, y.Value}
+ case GreaterThanOp:
+ return &BoundValue{x.Src, LessEqualOp, y.Value}
+ }
+
+ case (x.Op == LessThanOp || x.Op == LessEqualOp) &&
+ (y.Op == GreaterThanOp || y.Op == GreaterEqualOp),
+ (x.Op == GreaterThanOp || x.Op == GreaterEqualOp) &&
+ (y.Op == LessThanOp || y.Op == LessEqualOp):
+ // <(>=3)
+ // Something smaller than an arbitrarily large number is any number.
+ return &BasicType{K: y.Kind()}
+
+ case x.Op == LessThanOp &&
+ (y.Op == LessEqualOp || y.Op == LessThanOp),
+ x.Op == GreaterThanOp &&
+ (y.Op == GreaterEqualOp || y.Op == GreaterThanOp):
+ // <(<=x) => <x
+ // <(<x) => <x
+ // Less than something that is less or equal to x is less than x.
+ return &BoundValue{x.Src, x.Op, y.Value}
+
+ case x.Op == LessEqualOp &&
+ (y.Op == LessEqualOp || y.Op == LessThanOp),
+ x.Op == GreaterEqualOp &&
+ (y.Op == GreaterEqualOp || y.Op == GreaterThanOp):
+ // <=(<x) => <x
+ // <=(<=x) => <=x
+ // Less or equal than something that is less than x is less than x.
+ return y
+ }
+
+ case *BasicType:
+ switch x.Op {
+ case LessEqualOp, LessThanOp, GreaterEqualOp, GreaterThanOp:
+ return y
+ }
}
if v.Concreteness() > Concrete {
ctx.addErrf(IncompleteError, ctx.pos(),
