cue icon indicating copy to clipboard operation
cue copied to clipboard

evaluator: disjunction containing incomplete comprehension gives inappropriate error

Open rogpeppe opened this issue 3 years ago • 1 comments

What version of CUE are you using (cue version)?

$ cue version
cue version v0.0.0-20220801114602-5a08d2f7a9a7

       -compiler gc
     CGO_ENABLED 1
          GOARCH amd64
            GOOS linux
         GOAMD64 v1
             vcs git
    vcs.revision 5a08d2f7a9a7595def05c5486cbb5276e66391cb
        vcs.time 2022-08-01T11:46:02Z
    vcs.modified false

Does this issue reproduce with the latest release?

yes

What did you do?

exec cue export main.cue
cmp stdout expect.json

-- main.cue --
p?: []
a: [for k in p {k}] | null

-- expect.json --
{
    "a": null
}

What did you expect to see?

The above testscript should pass.

What did you see instead?

> exec cue export main.cue
[stderr]
a: incomplete value _|_(a: cannot reference optional field: p (and 1 more errors)) | null
[exit status 1]
FAIL: /tmp/testscript506054148/x.txtar/script.txtar:1: unexpected command failure
error running /tmp/x.txtar in /tmp/testscript506054148/x.txtar

Some exploration with git bisect points to https://cue-review.googlesource.com/c/cue/+/8282 as the place where this behaviour changed. Just before that, the above test passed.

The behaviour is quite sensitive. For example, this works as it should:

p?: []
a: {for k in p {(k): true}} | null

but this fails:

p?: []
a: {for k in p {x: 1}} | null

rogpeppe avatar Aug 05 '22 09:08 rogpeppe

Adding the following after speaking with @rogpeppe earlier. This takes @rogpeppe's example from above and also shows the issues that exist via the API:

go mod tidy
go run main.go
cmp stdout stdout.golden

-- go.mod --
module mod.com

go 1.18

require cuelang.org/go v0.4.4-0.20220808083940-b4d1b16ccf89

-- main.go --
package main

import (
	"fmt"

	"cuelang.org/go/cue"
	"cuelang.org/go/cue/cuecontext"
)

var cases = []struct {
	key string
	cue string
}{
	{
		// disjunction with list value and default marker
		key: "list-with-default",
		cue: `
p?: []
x: *[ for k in p {k} ] | true
	`,
	},
	{
		// disjunction with list value and but no default marker
		key: "list-no-default",
		cue: `
p?: []
x: [ for k in p {k} ] | true
	`,
	},
	{
		// disjunction with struct value and default marker
		key: "struct-with-default",
		cue: `
p?: []
x: *{ for k in p {k} } | true
	`},
	{
		// disjunction with struct value but no default marker
		key: "struct-no-default",
		cue: `
p?: []
x: { for k in p {k} } | true
	`,
	},
}

func main() {
	ctx := cuecontext.New()
	for _, c := range cases {
		v := ctx.CompileString(c.cue)
		x := v.LookupPath(cue.ParsePath("x"))
		var i interface{}
		if err := x.Decode(&i); err != nil {
			fmt.Printf("%v decoder error: %v\n", c.key, err)
			continue
		}
		fmt.Printf("%v decoded value: %v\n", c.key, i)
	}
}
-- stdout.golden --
list-with-default decoder error: x: true
list-no-default decoded value: true
struct-with-default decoded value: true
struct-no-default decoded value: true

This is expected to pass, because in all cases the non-true element of the disjunction are in an incomplete error state. The decode step should eliminate that element, leaving only true. Hence, we should always decode true in all cases. Instead we see:

> go mod tidy
> go run main.go
[stdout]
list-with-default decoder error: x: cannot reference optional field: p (and 1 more errors)
list-no-default decoded value: <nil>
struct-with-default decoded value: true
struct-no-default decoded value: true

> cmp stdout stdout.golden
--- stdout
+++ stdout.golden
@@ -1,4 +1,4 @@
-list-with-default decoder error: x: cannot reference optional field: p (and 1 more errors)
-list-no-default decoded value: <nil>
+list-with-default decoder error: x: true
+list-no-default decoded value: true
 struct-with-default decoded value: true
 struct-no-default decoded value: true

FAIL: /tmp/testscript972749543/repro.txtar/script.txt:3: stdout and stdout.golden differ

myitcv avatar Aug 08 '22 12:08 myitcv

This appears to have been fixed. Please verify.

mpvl avatar Sep 15 '22 18:09 mpvl

https://review.gerrithub.io/c/cue-lang/cue/+/543575

mpvl avatar Sep 17 '22 08:09 mpvl

I've verified that @myitcv's testscript above passes (with a couple of modifications because the expected output wasn't quite right). Here's the testscript that passes:

go mod tidy
go run main.go
cmp stdout stdout.golden

-- go.mod --
module mod.com

go 1.18

require cuelang.org/go v0.4.4-0.20220915174651-ad253ed099e9

-- main.go --
package main

import (
	"fmt"

	"cuelang.org/go/cue"
	"cuelang.org/go/cue/cuecontext"
)

var cases = []struct {
	key string
	cue string
}{
	{
		// disjunction with list value and default marker
		key: "list-with-default",
		cue: `
p?: []
x: *[ for k in p {k} ] | true
	`,
	},
	{
		// disjunction with list value and but no default marker
		key: "list-no-default",
		cue: `
p?: []
x: [ for k in p {k} ] | true
	`,
	},
	{
		// disjunction with struct value and default marker
		key: "struct-with-default",
		cue: `
p?: []
x: *{ for k in p {k} } | true
	`},
	{
		// disjunction with struct value but no default marker
		key: "struct-no-default",
		cue: `
p?: []
x: { for k in p {k} } | true
	`,
	},
}

func main() {
	ctx := cuecontext.New()
	for _, c := range cases {
		v := ctx.CompileString(c.cue)
		x := v.LookupPath(cue.ParsePath("x"))
		var i interface{}
		if err := x.Decode(&i); err != nil {
			fmt.Printf("%v: decoder error: %v\n", c.key, err)
			continue
		}
		fmt.Printf("%v: decoded value: %v\n", c.key, i)
	}
}
-- stdout.golden --
list-with-default: decoded value: true
list-no-default: decoded value: true
struct-with-default: decoded value: true
struct-no-default: decoded value: true

I'm going to close this as fixed now.

rogpeppe avatar Sep 22 '22 09:09 rogpeppe