Repository navigation
Support arrays, structs, and identifiers in ternary operators - #3222
mohammadfawaz wants to merge 3 commits into
Conversation
be16d8f to
f333d2e
Compare
f333d2e to
f6e5a6b
Compare
f6e5a6b to
d794d30
Compare
|
Support for this in Leo is here: aleo-labs-protocol/leo#29349 Shows on average 7.8% drop in Aleo program sizes across the Leo test suite. |
d794d30 to
fa9c2c2
Compare
3b0ed49 to
bc6da40
Compare
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
|
I reviewed this at Blocking1. Restore the console/circuit equivalence tests for literal ternary. 2. Pin the 3. Rewrite the doc comments in Non-blocking4. 5. Ternary finalize cost. 6. Build the stack once in 7. Simpler literal check. |
|
May be good to on rebase on latest staging to get full ci run. But generally looks fine to me |
Restore console and circuit equivalence coverage and pin the ternary wire format to the literal operand container. Return errors for invalid runtime branch shapes. Reuse the deployment stack, simplify literal checks, and clarify the cost and V14 test comments.
8434bdd to
933aced
Compare
Antonio95
left a comment
There was a problem hiding this comment.
Only two minor comments, one of which may merit a bit more discussion.
|
|
||
| /// Returns whether this instruction refers to an external struct. | ||
| #[inline] | ||
| pub fn contains_external_struct(&self) -> bool { |
There was a problem hiding this comment.
I am not familiar with the preexisting contains_external_struct machinery, but is it correct for this to return false in the case of instructions such as ternary r0 r1 r2 into r3; where r1 is an external struct?
| }; | ||
|
|
||
| // Ensure the second and third input types are equivalent. | ||
| if !register_types_equivalent(stack, &input_types[1], stack, &input_types[2])? { |
There was a problem hiding this comment.
This answers some of the questions I had regarding the behaviour as far as types and external structs are concerned. I think it might be good to very briefly document (maybe in code and also in the Aleo docs) what that behaviour is.
I'm not sure if register_types_equivalent is the best call here. It seems that: for local structs, it only accepts if the two struct types are the exact same (same name, in the same (local) program). Yet for (local, external), (external, local) and (external, external) combinations, it performs the recursive "same entry names and (recursively) same types" check - which is more flexible than that in the (local, local) case. If I'm not mistaken (but please correct me if I'm misunderstanding sth), the current check means one can write programs a.aleo and b.aleo such that
- in
a.aleo,ternary r0 r1 r2 into r3withr1of (local) typestruct_1andr2of (local) typestruct_2, is rejected - in
b.aleo,ternary r0 r1 r2 into r3withr1of (ext) typea.aleo/struct_1andr2of (ext) typea.aleo/struct_2is allowed
This feels a bit odd. Is that the case? If so, we can use a variant of register_types_equivalent which treats the (local, local) case like the rest. In any case (and even if my understanding is wrong), it would be good to have a test for that specific case in order to pin (and show) the expected behaviour.
|
|
||
| /// Returns `first` if `condition` is `true`, otherwise returns `second`. | ||
| /// The `first` and `second` literals must be the same variant, and must be a variant for which | ||
| /// ternary selection is supported. Callers are expected to enforce this via type-checking |
There was a problem hiding this comment.
I have double-checked with AI that this is indeed the case 👌
Extends
ternaryto accept arrays, structs, and identifier literals. This change requiresConsensusVersion::V23.Before V23, deployment verification rejects the new operand types in functions, finalize scopes, closures, constructors, and views. Existing literal operands remain supported.
The console and circuit implementations select array elements and struct members recursively. Type checking requires matching array shapes and struct layouts. Strings, including string fields inside arrays or structs, remain unsupported.
Literal ternary operations retain the existing finalize cost of 500 microcredits. Array and struct ternary costs scale with the recursive size of the branch operands.