evalv3 "undefined field" regression using cyclic conditionals

[mvdan]

# evalv2
env CUE_EXPERIMENT=evalv3=0
exec cue export in.cue

# evalv3
env CUE_EXPERIMENT=evalv3=1
exec cue export in.cue

-- in.cue --
out: #Application & {
	oneOrTwo: two: {}
}
#Application: X={
	oneOrTwo: #OneOrTwo & {
		if X.oneOrTwo.kind == "two" {
			two: {
				if X.oneOrTwo.two.optional == _|_ {
					optional: "optional set"
				}
			}
		}
	}
	if X.oneOrTwo.kind == "never" {
	}
}
#OneOrTwo: #One | #Two
#One: {
	kind: "one"
	one: oneField: "value"
}

#Two: {
	kind: "two"
	two: optional?: string
}

As of eb5408288c9c671c9738d525860dc0f6bf5522b4:

# evalv2 (0.006s)
> env CUE_EXPERIMENT=evalv3=0
> exec cue export in.cue
[stdout]
{
    "out": {
        "oneOrTwo": {
            "kind": "two",
            "two": {}
        }
    }
}
# evalv3 (0.006s)
> env CUE_EXPERIMENT=evalv3=1
> exec cue export in.cue
[stderr]
out: undefined field: kind:
    ./in.cue:14:16
[exit status 1]

Note how a simplified version without the disjunction causes both evaluator versions to fail with a much more reasonable error:

# evalv2
env CUE_EXPERIMENT=evalv3=0
exec cue export in.cue

# evalv3
env CUE_EXPERIMENT=evalv3=1
exec cue export in.cue

-- in.cue --
out: #Application & {
	oneOrTwo: two: {}
}
#Application: X={
	oneOrTwo: #Two & {
		two: {
			if X.oneOrTwo.two.optional == _|_ {
				optional: "optional set"
			}
		}
	}
}
#Two: {
	kind: "two"
	two: optional?: string
}

# evalv2 (0.006s)
> env CUE_EXPERIMENT=evalv3=0
> exec cue export in.cue
[stderr]
out.oneOrTwo.two: circular dependency in evaluation of conditionals: X.oneOrTwo.two.optional changed after evaluation:
    ./in.cue:7:7
[exit status 1]
# evalv3 (0.006s)
> env CUE_EXPERIMENT=evalv3=1
> exec cue export in.cue
[stderr]
out.oneOrTwo.two: circular dependency in evaluation of conditionals: X.oneOrTwo.two.optional changed after evaluation:
    ./in.cue:7:7
[exit status 1]

So perhaps the original example is meant to fail with the same sort of error. I'm not sure. But it's odd that, with the original example, evalv3 fails with a completely different error, and how it fails when evalv2 works.

Thanks to @SafetyCulture for reporting this via Unity!

[mvdan]

Not urgent because the user found a workaround which involved less complex CUE, not using cyclic comprehensions like that.

[mpvl]

It seems that evalv3 is doing the right thing:

if X.oneOrTwo.two.optional == _|_ {
	optional: "optional set"
}

Basically, this condition tests that a value is "undefined", but than sets this value, which causes this value to exist. A test is always valid if it always holds. Evalv2 wasn't very good at enforcing this.

It may be better to write:

optional: *"optional set" | string

making the string a default value.

Also you could write #Application as :

#Application: {
	oneOrTwo: #OneOrTwo & {
		// any
	} | *{
		kind: "two"
		two: optional: *"optional set" | string
	}
}

[mpvl]

An even nicer way to write it would be:

out: #Application & {
	oneOrTwo: two: {}
}
#Application: {
	oneOrTwo: #OneOrTwo
	oneOrTwo: *_appDefaults[_kind] | _
	_kind: oneOrTwo.kind
}
_appDefaults: two: optional: *"optional set" | string

I think this should be allowed as there is a deterministic order in which this can be evaluated.

However, here I run explicitly into kind not found problem (although the disjunction gobbles the error). I think that fixing this will also improve the error message.

[mpvl]

I made an error in the above example. Of course I need to repeat the two field. That still doesn't fix it, as the closedness algo does not resolve the disjunctions before it looks up kind. In general, it is recommendable to use discriminator fields. This approach works:

out: #Application & { oneOrTwo: kind: "two" } // use Discriminator fields.
#Application: {
	oneOrTwo: #OneOrTwo
	oneOrTwo: *_appDefaults[oneOrTwo.kind] | _
}
_appDefaults: two: two: optional: *"optional set" | string // note the two times "two"

The main trigger of the bad error message in the original bug is that it is an erroneous error that hides the real error. This erroneous error message is triggered by CUE wanting to finalize references before the disjunction is resolved. We can look into fixing that.

FYI and FTR: one reason why I did not discover the error that I needed to use 2x"two" is that the disjunction gobbles up the error and then inserts _. We will soon publish a try proposal that would allow this to be written as:

out: #Application & { oneOrTwo: kind: "two" } // use Discriminator fields.
#Application: {
	oneOrTwo: #OneOrTwo
	try { oneOrTwo: _appDefaults.(oneOrTwo.kind)?  } 
}
_appDefaults: two: two: optional: *"optional set" | string // note the two times "two"

try is similar to if except that it inserts the CUE if it evaluates with no error. Only references with a ? are allowed to fail, though. Any other error will result in an error (unlike disjunctions, which allow almost all errors). So in this case, had I forgotten the second "two", I would have gotten the error "optional" field not allowed, which would be correct.

[mpvl]

Note, I've made this pattern now work in evalv3:

out: #Application & {
	oneOrTwo: two: {}
}
#Application: {
	oneOrTwo: #OneOrTwo
	oneOrTwo: *_appDefaults[_kind] | _
	_kind: oneOrTwo.kind
}
_appDefaults: two: optional: *"optional set" | string

I also have more of an insight as to why the misleading error was reported in the original post: because of the circular dependency bug, both disjuncts fails. As a result, the kind field actually does not exists. In the evaluator one can see that both errors are reported. But the "deeper" error of the child is not shown with cue export. We should clearly change it to report all these errors.

As this is now purely an error message issue, I'm demoting the priority of the issue. I'll leave it open so we will still address it, though.