Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eval: default value resolution issue #770

Closed
cueckoo opened this issue Jul 3, 2021 · 4 comments
Closed

eval: default value resolution issue #770

cueckoo opened this issue Jul 3, 2021 · 4 comments
Assignees
Labels
disjunctions evalv3-win Issues resolved by switching from evalv2 to evalv3 NeedsFix

Comments

@cueckoo
Copy link
Collaborator

cueckoo commented Jul 3, 2021

Originally opened by @eonpatapon in cuelang/cue#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 cueckoo added this to the v0.5.x milestone Jul 3, 2021
@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @mpvl in cuelang/cue#770 (comment)

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
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @eonpatapon in cuelang/cue#770 (comment)

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

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @eonpatapon in cuelang/cue#770 (comment)

Thanks for the quick intial fix btw :)

@mvdan
Copy link
Member

mvdan commented May 21, 2024

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 mvdan self-assigned this May 21, 2024
cueckoo pushed a commit that referenced this issue May 21, 2024
As can be seen in the diff, the old evaluator resulted in the fields:

    h: a: v: *"a" | "b" | "c"
    h: b: v: *"a" | "b" | "c"
    h: c: v: *"a" | *"b" | *"c"

Which, when exporting, would understandably result in an error:

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

The new evaluator correctly keeps "a" as the only default in all fields:

    h: a: v: *"a" | "b" | "c"
    h: b: v: *"a" | "b" | "c"
    h: c: v: *"a" | "b" | "c"

Closes #770.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: Id3e5e9c522307900331dc0fa29b936f773750497
@mvdan mvdan added the evalv3-win Issues resolved by switching from evalv2 to evalv3 label Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disjunctions evalv3-win Issues resolved by switching from evalv2 to evalv3 NeedsFix
Projects
None yet
Development

No branches or pull requests

4 participants