internal/core/compile: make `>10 <20` a compile error
Or in general, fail on any expression that must be concrete,
but can never evaluate to a concrete value.
Although technically correct CUE, it can never resolve to
a valid value and allowing it gives too much confusion to
the user.
At evaluation time expressions of the form `int + 1` were
already flagged. Now it also flags `>a + 1`, as `>a` will
never be concrete, even if `a` resolves fine.
Note that `(>10 & <a) + 1` is fine, though, as the bounds
can still resolve to concrete value when unified.
Making it a compile error, instead of a evaluation error,
ensures that this error doesn't go unnoticed and give
unexpected results.
Fixes #742
Fixes #405
Change-Id: Ib2032778ca5dcb6ef1681b4e03498ff039d37c7f
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8665
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
diff --git a/cue/testdata/eval/incomplete.txtar b/cue/testdata/eval/incomplete.txtar
index 5d8e170..c075991 100644
--- a/cue/testdata/eval/incomplete.txtar
+++ b/cue/testdata/eval/incomplete.txtar
@@ -1,5 +1,3 @@
-These should all be incomplete errors.
-
-- in.cue --
s: string
@@ -17,23 +15,11 @@
c: a+b & >=5
}
-// Issue #129
-permanentlyIncompleteOperands: {
- a: string + ":" + string
- a: "golang/go:1.13.5"
-}
-
-issue680: (>10 * 2) & 0
-
+a: int
+okay: (>10 & <a) + 3
-- out/eval --
-Errors:
-permanentlyIncompleteOperands.a: invalid operand string ('+' requires concrete value):
- ./in.cue:19:8
-
-Result:
-(_|_){
- // [eval]
+(struct){
s: (string){ string }
e1: (_|_){
// [incomplete] e1: non-concrete value string in operand to +:
@@ -65,16 +51,10 @@
// ./in.cue:13:6
}
}
- permanentlyIncompleteOperands: (_|_){
- // [eval]
- a: (_|_){
- // [eval] permanentlyIncompleteOperands.a: invalid operand string ('+' requires concrete value):
- // ./in.cue:19:8
- }
- }
- issue680: (_|_){
- // [incomplete] issue680: non-concrete value >10 in operand to *:
- // ./in.cue:23:12
+ a: (int){ int }
+ okay: (_|_){
+ // [incomplete] okay: non-concrete value >10 & int in operand to +:
+ // ./in.cue:18:7
}
}
-- out/compile --
@@ -92,9 +72,6 @@
b: (〈0;c〉 - 〈0;a〉)
c: ((〈0;a〉 + 〈0;b〉) & >=5)
}
- permanentlyIncompleteOperands: {
- a: ((string + ":") + string)
- a: "golang/go:1.13.5"
- }
- issue680: ((>10 * 2) & 0)
+ a: int
+ okay: ((>10 & <〈0;a〉) + 3)
}
diff --git a/cue/testdata/eval/incompleteperm.txtar b/cue/testdata/eval/incompleteperm.txtar
new file mode 100644
index 0000000..d3ba612
--- /dev/null
+++ b/cue/testdata/eval/incompleteperm.txtar
@@ -0,0 +1,60 @@
+// Issue #742
+// Issue #405
+
+-- in.cue --
+// Issue #129
+permanentlyIncompleteOperands: {
+ a: string + ":" + string
+ a: "golang/go:1.13.5"
+}
+
+permanentlyIncompleteOperandsNested: {
+ a: (int + 1) + (int + 1)
+}
+
+permanentlyIncompleteOperandsDisjunct: {
+ a: (int + 1) | (int + 1)
+}
+
+
+issue680: (>10 * 2) & 0
+
+issue405: >=100 <=200
+
+-- out/eval --
+permanentlyIncompleteOperands.a: invalid operand string ('+' requires concrete value):
+ ./in.cue:3:8
+permanentlyIncompleteOperandsNested.a: invalid operand int ('+' requires concrete value):
+ ./in.cue:8:9
+permanentlyIncompleteOperandsDisjunct.a: invalid operand int ('+' requires concrete value):
+ ./in.cue:12:9
+issue680: invalid operand >10 ('*' requires concrete value):
+ ./in.cue:16:12
+issue405: invalid operand >=100 ('<=' requires concrete value):
+ ./in.cue:18:11
+-- out/compile --
+permanentlyIncompleteOperands.a: invalid operand string ('+' requires concrete value):
+ ./in.cue:3:8
+permanentlyIncompleteOperandsNested.a: invalid operand int ('+' requires concrete value):
+ ./in.cue:8:9
+permanentlyIncompleteOperandsDisjunct.a: invalid operand int ('+' requires concrete value):
+ ./in.cue:12:9
+issue680: invalid operand >10 ('*' requires concrete value):
+ ./in.cue:16:12
+issue405: invalid operand >=100 ('<=' requires concrete value):
+ ./in.cue:18:11
+--- in.cue
+{
+ permanentlyIncompleteOperands: {
+ a: ((string + ":") + string)
+ a: "golang/go:1.13.5"
+ }
+ permanentlyIncompleteOperandsNested: {
+ a: ((int + 1) + (int + 1))
+ }
+ permanentlyIncompleteOperandsDisjunct: {
+ a: ((int + 1)|(int + 1))
+ }
+ issue680: ((>10 * 2) & 0)
+ issue405: (>=100 <= 200)
+}
diff --git a/cue/testdata/export/009.txtar b/cue/testdata/export/009.txtar
index 7ab8e78..78510bb 100644
--- a/cue/testdata/export/009.txtar
+++ b/cue/testdata/export/009.txtar
@@ -4,12 +4,6 @@
{
a: 5 * [int]
a: [1, 2, ...]
- b: <=5 * [int]
- b: [1, 2, ...]
- c: (>=3 & <=5) * [int]
- c: [1, 2, ...]
- d: >=2 * [int]
- d: [1, 2, ...]
e: [...int]
e: [1, 2, ...]
f: [1, 2, ...]
@@ -33,30 +27,6 @@
2,
...,
]
- b: (<=5 * [
- int,
- ])
- b: [
- 1,
- 2,
- ...,
- ]
- c: ((>=3 & <=5) * [
- int,
- ])
- c: [
- 1,
- 2,
- ...,
- ]
- d: (>=2 * [
- int,
- ])
- d: [
- 1,
- 2,
- ...,
- ]
e: [
...int,
]
@@ -81,24 +51,6 @@
3: (int){ int }
4: (int){ int }
}
- b: (_|_){
- // [incomplete] b: non-concrete value <=5 in operand to *:
- // ./in.cue:4:5
- 0: (int){ 1 }
- 1: (int){ 2 }
- }
- c: (_|_){
- // [incomplete] c: non-concrete value >=3 & <=5 in operand to *:
- // ./in.cue:6:5
- 0: (int){ 1 }
- 1: (int){ 2 }
- }
- d: (_|_){
- // [incomplete] d: non-concrete value >=2 in operand to *:
- // ./in.cue:8:5
- 0: (int){ 1 }
- 1: (int){ 2 }
- }
e: (list){
0: (int){ 1 }
1: (int){ 2 }
diff --git a/cue/testdata/export/010.txtar b/cue/testdata/export/010.txtar
index bbaaec3..fea507f 100644
--- a/cue/testdata/export/010.txtar
+++ b/cue/testdata/export/010.txtar
@@ -5,12 +5,6 @@
{
a: 5 * [int]
a: [1, 2, ...]
- b: <=5 * [int]
- b: [1, 2, ...]
- c: (>=3 & <=5) * [int]
- c: [1, 2, ...]
- d: >=2 * [int]
- d: [1, 2, ...]
e: [...int]
e: [1, 2, ...]
f: [1, 2, ...]
@@ -34,30 +28,6 @@
2,
...,
]
- b: (<=5 * [
- int,
- ])
- b: [
- 1,
- 2,
- ...,
- ]
- c: ((>=3 & <=5) * [
- int,
- ])
- c: [
- 1,
- 2,
- ...,
- ]
- d: (>=2 * [
- int,
- ])
- d: [
- 1,
- 2,
- ...,
- ]
e: [
...int,
]
@@ -82,24 +52,6 @@
3: (int){ int }
4: (int){ int }
}
- b: (_|_){
- // [incomplete] b: non-concrete value <=5 in operand to *:
- // ./in.cue:4:5
- 0: (int){ 1 }
- 1: (int){ 2 }
- }
- c: (_|_){
- // [incomplete] c: non-concrete value >=3 & <=5 in operand to *:
- // ./in.cue:6:5
- 0: (int){ 1 }
- 1: (int){ 2 }
- }
- d: (_|_){
- // [incomplete] d: non-concrete value >=2 in operand to *:
- // ./in.cue:8:5
- 0: (int){ 1 }
- 1: (int){ 2 }
- }
e: (list){
0: (int){ 1 }
1: (int){ 2 }
diff --git a/cue/testdata/lists/020_list_arithmetic.txtar b/cue/testdata/lists/020_list_arithmetic.txtar
index 8992835..93ad628 100644
--- a/cue/testdata/lists/020_list_arithmetic.txtar
+++ b/cue/testdata/lists/020_list_arithmetic.txtar
@@ -1,14 +1,9 @@
-# DO NOT EDIT; generated by go run testdata/gen.go
-#
#name: list arithmetic
#evalPartial
-- in.cue --
l0: 3 * [1, 2, 3]
l1: 0 * [1, 2, 3]
l2: 10 * []
-l3: <=2 * []
-l4: <=2 * [int]
-l5: <=2 * (int * [int])
l6: 3 * [...int]
l7: 3 * [1, ...int]
l8: 3 * [1, 2, ...int]
@@ -112,13 +107,6 @@
3,
])
l2: (10 * [])
- l3: (<=2 * [])
- l4: (<=2 * [
- int,
- ])
- l5: (<=2 * (int * [
- int,
- ]))
l6: (3 * [
...int,
])
@@ -337,13 +325,7 @@
])
}
-- out/eval --
-Errors:
-l5: invalid operand int ('*' requires concrete value):
- ./in.cue:6:12
-
-Result:
-(_|_){
- // [eval]
+(struct){
l0: (#list){
0: (int){ 1 }
1: (int){ 2 }
@@ -359,18 +341,6 @@
}
l2: (#list){
}
- l3: (_|_){
- // [incomplete] l3: non-concrete value <=2 in operand to *:
- // ./in.cue:4:5
- }
- l4: (_|_){
- // [incomplete] l4: non-concrete value <=2 in operand to *:
- // ./in.cue:5:5
- }
- l5: (_|_){
- // [eval] l5: invalid operand int ('*' requires concrete value):
- // ./in.cue:6:12
- }
l6: (#list){
}
l7: (#list){
diff --git a/cue/testdata/lists/020_list_compilefail.txtar b/cue/testdata/lists/020_list_compilefail.txtar
new file mode 100644
index 0000000..65ec7ff
--- /dev/null
+++ b/cue/testdata/lists/020_list_compilefail.txtar
@@ -0,0 +1,69 @@
+#name: list arithmetic
+#evalPartial
+-- in.cue --
+l3: <=2 * []
+l4: <=2 * [int]
+l5: <=2 * (int * [int])
+b: <=5 * [int]
+b: [1, 2, ...]
+c: (>=3 & <=5) * [int]
+c: [1, 2, ...]
+d: >=2 * [int]
+d: [1, 2, ...]
+
+-- out/compile --
+l3: invalid operand <=2 ('*' requires concrete value):
+ ./in.cue:1:5
+l4: invalid operand <=2 ('*' requires concrete value):
+ ./in.cue:2:5
+l5: invalid operand <=2 ('*' requires concrete value):
+ ./in.cue:3:5
+b: invalid operand <=5 ('*' requires concrete value):
+ ./in.cue:4:4
+d: invalid operand >=2 ('*' requires concrete value):
+ ./in.cue:8:4
+--- in.cue
+{
+ l3: (<=2 * [])
+ l4: (<=2 * [
+ int,
+ ])
+ l5: (<=2 * (int * [
+ int,
+ ]))
+ b: (<=5 * [
+ int,
+ ])
+ b: [
+ 1,
+ 2,
+ ...,
+ ]
+ c: ((>=3 & <=5) * [
+ int,
+ ])
+ c: [
+ 1,
+ 2,
+ ...,
+ ]
+ d: (>=2 * [
+ int,
+ ])
+ d: [
+ 1,
+ 2,
+ ...,
+ ]
+}
+-- out/eval --
+l3: invalid operand <=2 ('*' requires concrete value):
+ ./in.cue:1:5
+l4: invalid operand <=2 ('*' requires concrete value):
+ ./in.cue:2:5
+l5: invalid operand <=2 ('*' requires concrete value):
+ ./in.cue:3:5
+b: invalid operand <=5 ('*' requires concrete value):
+ ./in.cue:4:4
+d: invalid operand >=2 ('*' requires concrete value):
+ ./in.cue:8:4
diff --git a/cue/types_test.go b/cue/types_test.go
index e3b7ab0..fed0acd 100644
--- a/cue/types_test.go
+++ b/cue/types_test.go
@@ -642,9 +642,6 @@
value: `[1,2,3]`,
res: "[1,2,3,]",
}, {
- value: `>=5*[1,2,3, ...int]`,
- err: "non-concrete value >=5 in operand to *",
- }, {
value: `[for x in #y if x > 1 { x }]
#y: [1,2,3]`,
res: "[2,3,]",
@@ -2326,9 +2323,6 @@
value: `[int]`,
err: `0: cannot convert incomplete value`,
}, {
- value: `(>=3 * [1, 2])`,
- err: "cue: marshal error: non-concrete value >=3 in operand to *",
- }, {
value: `{}`,
json: `{}`,
}, {
diff --git a/internal/core/adt/context.go b/internal/core/adt/context.go
index 8a7d4ea..0eacb24 100644
--- a/internal/core/adt/context.go
+++ b/internal/core/adt/context.go
@@ -273,12 +273,22 @@
}
func (c *OpContext) concreteIsPossible(op Op, x Expr) bool {
- if v, ok := x.(Value); ok {
- if v.Concreteness() > Concrete {
- c.AddErrf("invalid operand %s ('%s' requires concrete value)",
- c.Str(x), op)
- return false
- }
+ if !AssertConcreteIsPossible(op, x) {
+ c.AddErrf("invalid operand %s ('%s' requires concrete value)",
+ c.Str(x), op)
+ return false
+ }
+ return true
+}
+
+// Assert that the given expression can evaluate to a concrete value.
+func AssertConcreteIsPossible(op Op, x Expr) bool {
+ switch v := x.(type) {
+ case *Bottom:
+ case *BoundExpr:
+ return false
+ case Value:
+ return v.Concreteness() == Concrete
}
return true
}
diff --git a/internal/core/compile/compile.go b/internal/core/compile/compile.go
index 178cf6f..37769b3 100644
--- a/internal/core/compile/compile.go
+++ b/internal/core/compile/compile.go
@@ -927,13 +927,15 @@
return d
default:
+ op := adt.OpFromToken(n.Op)
+ x := c.expr(n.X)
+ y := c.expr(n.Y)
+ if op != adt.AndOp {
+ c.assertConcreteIsPossible(n.X, op, x)
+ c.assertConcreteIsPossible(n.Y, op, y)
+ }
// return updateBin(c,
- return &adt.BinaryExpr{
- Src: n,
- Op: adt.OpFromToken(n.Op), // op
- X: c.expr(n.X), // left
- Y: c.expr(n.Y), // right
- } // )
+ return &adt.BinaryExpr{Src: n, Op: op, X: x, Y: y} // )
}
default:
@@ -941,6 +943,14 @@
}
}
+func (c *compiler) assertConcreteIsPossible(src ast.Node, op adt.Op, x adt.Expr) bool {
+ if !adt.AssertConcreteIsPossible(op, x) {
+ str := internal.DebugStr(src)
+ c.errf(src, "invalid operand %s ('%s' requires concrete value)", str, op)
+ }
+ return false
+}
+
func (c *compiler) addDisjunctionElem(d *adt.DisjunctionExpr, n ast.Expr, mark bool) {
switch x := n.(type) {
case *ast.BinaryExpr:
