Skip to content

evalv3: empty disjunction regression involving let fields #3801

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

Closed
mvdan opened this issue Mar 7, 2025 · 4 comments
Closed

evalv3: empty disjunction regression involving let fields #3801

mvdan opened this issue Mar 7, 2025 · 4 comments
Assignees
Labels
evaluator evalv3 issues affecting only the evaluator version 3 unity-win bugs found thanks to projects added to Unity

Comments

@mvdan
Copy link
Member

mvdan commented Mar 7, 2025

# With the old evaluator.
env CUE_EXPERIMENT=evalv3=0
exec cue vet -c

# With the new evaluator.
env CUE_EXPERIMENT=evalv3=1
env CUE_DEBUG=openinline=0
exec cue vet -c

-- input.cue --
package p

#Resource: {
    someMsg: string
    obs: {} | *{missing: true}

    let pickMsg = [
        if obs.missing {msg: "\(someMsg)"},
        {msg: "bar"},
    ][0]
    patches: [{
        op:    "add"
        path:  "/metadata"
        value: pickMsg.msg
    }]
}

#Patches: [string]: _
#JSONPatch: {
    namespace?: string
    patch: [...#JSONOp]
    output: #Patches & {(namespace): patch}
}
#JSONOp: {
    op:    "add"
    path:  string
    value: _
} | {
    op:   "remove"
    path: string
}

#Main: {
    NS=namespace: string

    output: jsonPatch.output

    let jsonPatch = #JSONPatch & {
        let base = #Resource & {}
        let withMsg = base & {someMsg: "foo"}

        namespace: NS

        patch: withMsg.patches
    }
}
out: (#Main & {namespace: "ns1"}).output

As of 5d2da07, this gives:

# With the old evaluator. (0.010s)
> env CUE_EXPERIMENT=evalv3=0
> exec cue vet -c
# With the new evaluator. (0.047s)
> env CUE_EXPERIMENT=evalv3=1
> env CUE_DEBUG=openinline=0
> exec cue vet -c
[stderr]
jsonPatch.output.ns1.0: 2 errors in empty disjunction:
jsonPatch.output.ns1.0.op: conflicting values "remove" and "add":
    ./input.cue:12:16
    ./input.cue:29:11
jsonPatch.output.ns1.0.value: invalid interpolation: non-concrete value string (type string):
    ./input.cue:8:30
    ./input.cue:4:14
[exit status 1]

Reduced from @nyarly's project in Unity.

@mvdan mvdan added evaluator evalv3 issues affecting only the evaluator version 3 unity-win bugs found thanks to projects added to Unity labels Mar 7, 2025
@mvdan
Copy link
Member Author

mvdan commented Mar 7, 2025

Note that I left the reproducer relatively large so that it retains much of the original structure and logic, so that it can act as a "full" reproducer for both the "conflicting values" and "non-concrete value" errors.

@mpvl
Copy link
Member

mpvl commented Mar 7, 2025

FTR, here is a reduction:

let F = { // must be a let.
    // moving this one level up fixes it.
    base: {
        in: string
        let X = [ {msg: "\(in)"} ][0]
        out: X.msg
    }
    XXX: base & {in: "foo"} // failure is here
}
output: F.XXX.out // confirm that it does not work outside let.

@mvdan
Copy link
Member Author

mvdan commented Mar 17, 2025

The reduction I shared here was resolved by https://review.gerrithub.io/c/cue-lang/cue/+/1211647, but Judson's original code still fails. I have reduced the regression once again after your fix:

# With the old evaluator.
env CUE_EXPERIMENT=evalv3=0
exec cue export

# With the new evaluator.
env CUE_EXPERIMENT=evalv3=1
exec cue export

-- input.cue --
package p

#Output: {
	NS=namespace: string
	output: (#SubOutput & {
		namespace: NS
		let base2 = {
			let base = #Base & {}
			output: base & {val0: "foo"}
		}
		patch: base2.output.value
	}).output
}
out: (#Output & {namespace: "ns1"}).output

#SubOutput: {
	namespace?: string

	patch: _
	output: (namespace): patch
}

#Base: {
	val0: string
	let msgObj = [val0][0]
	value: msgObj
}
# With the old evaluator. (0.011s)
> env CUE_EXPERIMENT=evalv3=0
> exec cue export
[stdout]
{
    "out": {
        "ns1": "foo"
    }
}
# With the new evaluator. (0.024s)
> env CUE_EXPERIMENT=evalv3=1
> exec cue export
[stderr]
output.ns1: incomplete value string:
    ./input.cue:24:8
[exit status 1]

@mpvl
Copy link
Member

mpvl commented Mar 17, 2025

I reduced it to this:

#Output: output: {
	#base: {
		val0: string
		msgObj: {a: val0}.a
		value: msgObj
	}
	out: #base & {val0: "foo"}
}
out: (#Output & {}).output // non-concrete

The thing it has in common with the let variant is that most of the intermediate values are marked as "non-rooted".

@mpvl mpvl self-assigned this Mar 18, 2025
cueckoo pushed a commit that referenced this issue Mar 20, 2025
Issue #3801

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: Ia0b24e3b7585b9b03bf9a89917c5dc31333cf803
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1211646
Reviewed-by: Daniel Martí <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
cueckoo pushed a commit that referenced this issue Mar 20, 2025
The bug was caused because lets were treated
as "too special" compared to normal fields.
By removing some of this exclusion, this bug
goes away.

Issue #3801

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: I975c47de1a0ee51678e402534c2a576d6b180748
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1211647
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
cueckoo pushed a commit that referenced this issue Mar 20, 2025
Structure sharing for non-rooted values resulted
in partially evaluated values. We disable it for
now and revisit the issue later.

Issue #3801
Fixes #3832

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: Ie520f9141dba17706628c40a3540335b8dbc98c4
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1211742
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evaluator evalv3 issues affecting only the evaluator version 3 unity-win bugs found thanks to projects added to Unity
Projects
None yet
Development

No branches or pull requests

2 participants