cue icon indicating copy to clipboard operation
cue copied to clipboard

eval: default value resolution issue

Open cueckoo opened this issue 4 years ago • 3 comments

Originally opened by @eonpatapon in https://github.com/cuelang/cue/issues/770

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

$ cue version
cue version 0.3.0-beta.5 linux/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

Following on #763, there is still some funking things going on.

While the repro case of #763 is fixed if I add another level it fails the same:

-- x.cue --
package x

#A: {
	v: "a" | "b" | "c" // change to string to fix
}

h: [string]: #A

h: a: {
	v: *"a" | string
}

h: [=~"^b"]: {
	v: *h.a.v | string
}

h: [=~"^c"]: {
	v: *h.b.v | string
}

h: b:   _
h: boo: _

h: c:   _
h: coo: _

What did you expect to see?

h: {
    a: {
        v: "a"
    }
    b: {
        v: "a"
    }
    boo: {
        v: "a"
    }
    c: {
        v: "a"
    }
    coo: {
        v: "a"
    }
}

What did you see instead?

beta.4

h.b.v: incomplete value "a" | "b" | "c"
h.boo.v: incomplete value "a" | "b" | "c"
h.c.v: incomplete value "a" | "b" | "c"
h.coo.v: incomplete value "a" | "b" | "c"

beta.5

h.c.v: incomplete value "a" | "b" | "c"
h.coo.v: incomplete value "a" | "b" | "c"

cueckoo avatar Jul 03 '21 10:07 cueckoo

Original reply by @mpvl in https://github.com/cuelang/cue/issues/770#issuecomment-779291340

This could be. As the release notes remarked (IIRC), the implementation is not entirely correct.

We have slightly changed the definition and reimplemented it. Funnily enough the bug of #763 was fixed initially by this change as a result of the restructuring, but then the new semantics reintroduced the bug (completely different mechanism, same result). The issue is that an existing optimization is no longer valid under the new semantics. We plan to undo that optimization anyway as part of a different effort, so we decided to do this later. The commit message of the fix and comments in the code explain it in a bit more depth.

We didn't expect that users would go "a level deeper" that quickly. :) To help us prioritize, is this a blocker for you?

cueckoo avatar Jul 03 '21 10:07 cueckoo

Original reply by @eonpatapon in https://github.com/cuelang/cue/issues/770#issuecomment-779293341

Not really as I can just use string instead of the disjunction in the meantime. And that will work

cueckoo avatar Jul 03 '21 10:07 cueckoo

Original reply by @eonpatapon in https://github.com/cuelang/cue/issues/770#issuecomment-779298450

Thanks for the quick intial fix btw :)

cueckoo avatar Jul 03 '21 10:07 cueckoo

The original program still fails as of master today:

> exec cue export x.cue
[stderr]
h.c.v: incomplete value "a" | "b" | "c"
h.coo.v: incomplete value "a" | "b" | "c"
[exit status 1]

However, the new evaluator with the new disjunctions algorithm correctly handles this:

> env CUE_EXPERIMENT=evalv3
> exec cue export x.cue
[stdout]
{
    "h": {
        "a": {
            "v": "a"
        },
        "b": {
            "v": "a"
        },
        "boo": {
            "v": "a"
        },
        "c": {
            "v": "a"
        },
        "coo": {
            "v": "a"
        }
    }
}

I'll close this issue by adding a regression test for the new evaluator :)

mvdan avatar May 21 '24 11:05 mvdan