Skip to content

Commit

Permalink
internal/core/adt: fix regression in comparing to bottom
Browse files Browse the repository at this point in the history
In v0.4.3, comparing a struct with a nested incomplete error
to bottom did not pass. Even though logically this is correct,
so the "regression" was arguably a bug fix, we opt to keep
the original behavior for v0.5 and rather introduce builtins
that can help write this in a more well-defined manner
(e.g. isvalid vs exists).

Fixes #2245

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: Iaa56202d663133f00521b4f47dd568a4e2de777f
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/551004
Unity-Result: CUEcueckoo <cueckoo@cuelang.org>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
  • Loading branch information
mpvl committed Mar 14, 2023
1 parent 1a0a81b commit 9954cc2
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 17 deletions.
53 changes: 47 additions & 6 deletions cue/testdata/comprehensions/checkdefined.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ issue1969: oko: {

if Y != _|_ {Y}
}
-- issue2245.cue --
yo: { f: int, g: f < 10 }
okyo: { if yo != _|_ {a: 1} }
okyog: { if yo.g != _|_ {a: 1} }

-- incomplete.cue --
// Comparing to string is not an error. But for legacy reasons we want to still
// support this. We intend to have more clean and precise behavior with the
Expand Down Expand Up @@ -74,14 +79,14 @@ structs: {

-- out/eval/stats --
Leaks: 11
Freed: 81
Reused: 77
Freed: 89
Reused: 85
Allocs: 15
Retain: 24
Retain: 26

Unifications: 92
Conjuncts: 116
Disjuncts: 97
Unifications: 100
Conjuncts: 134
Disjuncts: 105
-- out/eval --
(struct){
xc: (#struct){
Expand All @@ -105,10 +110,12 @@ Disjuncts: 97
okc2: (struct){
}
oko2: (struct){
a: (int){ 1 }
}
okc3: (struct){
}
oko3: (struct){
a: (int){ 1 }
}
issue1969: (struct){
okc: (struct){
Expand All @@ -135,6 +142,10 @@ Disjuncts: 97
// ./in.cue:39:13
}
}
s: (_|_){
// [incomplete] issue1969.oko.X: undefined field: undefined:
// ./in.cue:39:13
}
}
}
checkIncomplete: (struct){
Expand All @@ -146,6 +157,19 @@ Disjuncts: 97
}
}
}
yo: (struct){
f: (int){ int }
g: (_|_){
// [incomplete] yo.g: non-concrete value int in operand to <:
// ./issue2245.cue:1:18
// ./issue2245.cue:1:10
}
}
okyo: (struct){
a: (int){ 1 }
}
okyog: (struct){
}
structs: (struct){
bare: (struct){
t1: (bool){ true }
Expand Down Expand Up @@ -271,6 +295,23 @@ Disjuncts: 97
}
}
}
--- issue2245.cue
{
yo: {
f: int
g: (〈0;f〉 < 10)
}
okyo: {
if (〈1;yo〉 != _|_(explicit error (_|_ literal) in source)) {
a: 1
}
}
okyog: {
if (〈1;yo〉.g != _|_(explicit error (_|_ literal) in source)) {
a: 1
}
}
}
--- structs.cue
{
structs: {
Expand Down
2 changes: 1 addition & 1 deletion cue/testdata/cycle/evaluate.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ Allocs: 71
Retain: 137

Unifications: 149
Conjuncts: 291
Conjuncts: 289
Disjuncts: 171
-- out/eval --
Errors:
Expand Down
13 changes: 3 additions & 10 deletions internal/core/adt/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -1398,7 +1398,7 @@ func (c *OpContext) validate(env *Environment, src ast.Node, x Expr, op Op, stat
// builtin.
match = op == EqualOp

case anyError(v):
case isFinalError(v):
// Need to recursively check for errors, so we need to evaluate the
// Vertex in case it hadn't been evaluated yet.
match = op == EqualOp
Expand Down Expand Up @@ -1428,18 +1428,11 @@ func (c *OpContext) validate(env *Environment, src ast.Node, x Expr, op Op, stat
return &Bool{src, match}
}

// TODO(perf): keep track of the presence of recursive errors so that we can
// avoid traversing the arcs here.
func anyError(n *Vertex) bool {
func isFinalError(n *Vertex) bool {
n = n.Indirect()
if _, ok := Unwrap(n).(*Bottom); ok {
if b, ok := Unwrap(n).(*Bottom); ok && b.Code < IncompleteError {
return true
}
for _, arc := range n.Arcs {
if anyError(arc) {
return true
}
}
return false
}

Expand Down

0 comments on commit 9954cc2

Please sign in to comment.