cue: embedded disjunction of optional fields is incomplete

[rogpeppe]

commit 6f37a71

The following code fails, but it seems like it should be OK.

e: Example

e: a: "hello"

Example :: {
	a: string
	{
		value?: string
	} | {
		externalValue?: string
	}
}

The error from cue vet -c is:

e: incomplete value ((C{a: (string & "hello"), value?: string} | C{a: (string & "hello"), externalValue?: string})):
    ./x.cue:1:4
    ./x.cue:3:4
    ./x.cue:5:12
    ./x.cue:7:2

[mpvl]

Actually, the "fix" was an introduction of a bug. This should indeed be an incomplete value: the embedded disjunction results in two different structs. When made concrete they indeed will appear as the same value to the user. But for evaluation purposes, these are two different structs and both should be retained: at a later point they may still be unified with either optional value.

The simple solution is to mark on of the conjuncts as default.

One could argue that evaluation always reinserts the raw conjuncts anyway, and it doesn't matter to drop the optional fields. This is an implementation detail though, and that may not happen in the future.

Another convenience may be to say at at the point of concretizing a value, we first unique disjuncts disregarding optional values. But that should be an explicit decision.

Either way, if we do want this to not be an incomplete value, it needs to be carefully considered why.

[mpvl]

To add other possible avenues FTR: in general it is not a good idea to have a disjunction of structs without a mandatory disambiguation field. If this is not possible, using a default value is highly encouraged. In this case this could be

*{} | { value?: string } | { externalValue?: string }

We could have a linter make such a suggestion.

[mpvl]

Another thought is to do another pass of duplicate elimination at the time a value is made concrete, but at that time not considering optional fields.

We would have to closely consider what the impact would be if we allow dropping regular non-concrete fields by default, as proposed. Would we do the same thing there? Any difference in behavior there would likely be perceived as weird.

Also, that solution would perhaps suggest that int | 1 should be disambiguated to 1, which feels not right and dangerous. If that feeling is shared, we should strongly reconsider doing a similar disambiguation for structs with optional fields, as logically this is the same thing.

OTOH, allowing such a disambiguation step at manifestation time would eliminate the need to write * in many cases. Again, this is potentially a treacherous road to go in to, so this needs to be considered with care.

[mpvl]

FTR, this can probably be solved by allowing to disambiguate based on concrete values only and then adding the unification of all optional fields of all disjuncts (a failed optional field just means the field is not allowed).

This would be somewhat similar to closing an open list upon manifestation.

Just dropping optionals is problematic as when resolving this as operant to a selection would drop types.

[cueckoo]

This issue has been migrated to cue-lang/cue#353.

For more details about CUE's migration to a new home, please see cue-lang/cue#1078.