evaluator: list indexing does not propagate default values
nicuveo opened this issue · 11 comments
What version of CUE are you using (cue version)?
the playground (v0.5.1-0.20230413144408-f681271a38ec and v0.6.0-alpha.1.0.20230428121230-80d6ea032057)
What did you do?
["a", "b"][0 | *1] & "a"What did you expect to see?
As per the Index expressions spec:
Both the operand and index value may be a value-default pair.
va[⟨vi, di⟩] => ⟨va[vi], va[di]⟩
then, as per Default values:
⟨v1, d1⟩ & ⟨v2⟩ => ⟨v1&v2, d1&v2⟩
Consequently, i would expect this to evaluate to "a".
What did you see instead?
conflicting values "a" and "b"
This suggests that the index was resolved to a concrete value before resolving the index, instead of distributing it?
@nicuveo thanks for raising this and your other issues (we're in something of a focus week this week so we will get round to them)!
This does indeed appear to be a bug.
What version of CUE are you using (cue version)?
the playground
In the top right hand corner of the playground is the CUE version. Please can you copy this in to issues you create, to help make it specific?
Also flagging that there is https://tip.cuelang.org/play/ if you want to check things against CUE tip.
In the top right hand corner of the playground is the CUE version. Please can you copy this in to issues you create, to help make it specific?
Done! I've updated all other issues as well, after checking i could replicate them in the current playground version.
Also flagging that there is https://tip.cuelang.org/play/ if you want to check things against CUE tip.
Oh, thanks, will have a look!
Done! I've updated all other issues as well, after checking i could replicate them in the current playground version.
Oh that's fantastic, thank you!
Created a test case in https://tip.cuelang.org/play/?id=ihet6gSYHjE#cue@export@cue
Sorry, i just realized that i've made a mistake in this issue. In choosing an example, i conflated two related and overlapping but nonetheless distinct questions: whether indexing should distribute over disjunctions, and whether indexing should propagate default values. The bug is about the latter, but the example i give mistakenly relies on an assumption about the former.
AFAICT, the specification does not explicitly say whether indexing should distribute over disjunctions, but strongly suggests that it shouldn't: the index is explicitly required to be concrete. It is therefore my understanding that x[3 | 4] would be an incomplete value, and would not resolve to x[3] | x[4]. The playground seems to agree.
If that understanding is correct, then the example i provided is wrong: the only way for ["a", "b"][0 | *1] & "a" to evaluate to "a" would be for it to evaluate as follows, which contradicts the assumption:
"a" & ["a", "b"][0 | *1] // resolve 0 | *1
= "a" & ["a", "b"][⟨0 | 1, 1⟩] // va[⟨vi, di⟩] => ⟨va[vi], va[di]⟩
= "a" & ⟨["a", "b"][0 | 1], ["a", "b"][1]⟩ // distribution over disjunction (!!!)
= "a" & ⟨["a", "b"][0] | ["a", "b"][1], ["a", "b"][1]⟩ // resolve indexing
= "a" & ⟨"a" | "b", "b"⟩ // per U1 and distribution
= ⟨"a"⟩
It is therefore correct for CUE to reject "a" & ["a", "b"][0 | *1] (provided, again, that i am correct in assuming that indexing does not distribute over disjunctions).
But that doesn't mean there is no bug! Because while the example i provided was wrong, it is nonetheless true that CUE does not currently propagate default values correctly. Consider this better and simpler example: "a" | ["b"][*0 | 1]. This should evaluate to "b", but evaluates to "a" | "b" in v0.6.0-alpha.1.0.20230428121230-80d6ea032057:
"a" | ["b"][*0 | 1]
= "a" | ["b"][⟨0 | 1, 0⟩] // va[⟨vi, di⟩] => ⟨va[vi], va[di]⟩
= "a" | ⟨["b"][0 | 1], ["b"][0]⟩
= "a" | ⟨["b"][0 | 1], "b"⟩ // do NOT distribute, value remains incomplete
= ⟨"a"⟩ | ⟨["b"][0 | 1], "b"⟩ // per D1: ⟨v1, d1⟩ | ⟨v2⟩ => ⟨v1|v2, d1⟩
= ⟨"a" | ["b"][0 | 1], "b"⟩
But the playground resolves it to "a" | "b", which suggests that ["b"][⟨0 | 1, 0⟩] was resolved to ⟨"b"⟩ instead of ⟨["b"][0 | 1], "b"⟩.
Sorry for the confusion!
TL;DR: the example i provided was incorrect. Here are correct test cases for this bug:
expr: "a" | ["b"][*0 | 1]
want: "b"
got: "a" | "b"
expr: "a" | (*["b"] | ["c"])[0]
want: "b"
got: "a" | "b"
expr: "a" | (*["b"] | ["c"])[*0 | 1]
want: "b"
got: "a" | "b"
It is therefore correct for CUE to reject
"a" & ["a", "b"][0 | *1]
Thinking about it, i think i was also wrong: it shouldn't be rejected, nor should it evaluate to "a": it should simply remain incomplete.
"a" & ["a", "b"][0 | *1]
= "a" & ["a", "b"][⟨0 | 1, 1⟩]
= "a" & ⟨["a", "b"][0 | 1], ["a", "b"][1]⟩
= "a" & ⟨["a", "b"][0 | 1], "b"⟩
= ⟨"a" & ["a", "b"][0 | 1], "a" & "b"⟩
= ⟨"a" & ["a", "b"][0 | 1], _|_⟩
= ⟨"a" & ["a", "b"][0 | 1]⟩
consequently, i think it would be correct for "a" & ["a", "b"][0 | *1] to simply evaluate to.... "a" & ["a", "b"][0 | *1]. ^^'
Sorry for the multiple verbose updates, it took me a while to wrap my head around the way default values are resolved.
This should perhaps be clarified in the spec, but default values only distribute over structs, but not over operators. For instance, in
a: *1 | 2
b: *3 | 5
c: a * b
c evaluates to 3, not *3 | 5 | 6 | 10. This was a deliberate choice.
In other words, default rewriting rules do not apply in this case. Similarly, a default is selected when it is used as an index.
In general, default values get selected whenever a value is substituted in an expression, with the exception of & and |.
In other words, for "a" & ["a", "b"][0 | *1], there is no way to override the index, and it will always be 1. The value will thus always fail (`"a" & "b").
Similarly, for "a" | (*["b"] | ["c"])[0], the default is selected before selecting the index, so the result is indeed "a" | "b".
Note that the spec defines the rewrite rules only for "CUE values". Not expressions. So the spec is correct, afaict, albeit subtle. I agree we need to document this properly in the upcoming Language Guide.
I see now the original point in the spec you quote. This is indeed wrong. The original CUE indeed did this, but it resulted in tons of issues and was just quite complicated. This part of the spec should indeed be removed.
I see now the same holds for the Operators section:
Operands of unary and binary expressions may be associated with a default using the following