-
Notifications
You must be signed in to change notification settings - Fork 109
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
SATS: Flatten AlgebraicType (Nix BuiltinType) #379
Conversation
940e117
to
90ec74f
Compare
8617bc0
to
b68b081
Compare
9943679
to
17640ac
Compare
5a6cc7c
to
9399995
Compare
9a27676
to
d8faed1
Compare
d8faed1
to
21c9ed5
Compare
a6cc5f8
to
09bfc68
Compare
e5b42ac
to
8105110
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Note: We don't have a way to assert the changes in the bindings are correct
@@ -342,7 +394,7 @@ mod tests { | |||
fn algebraic_type() { | |||
let algebraic_type = AlgebraicType::meta_type(); | |||
assert_eq!( | |||
"(sum: (variants: Array<(name: (some: String | none: ()), algebraic_type: &0)>) | product: (elements: Array<(name: (some: String | none: ()), algebraic_type: &0)>) | builtin: (bool: () | i8: () | u8: () | i16: () | u16: () | i32: () | u32: () | i64: () | u64: () | i128: () | u128: () | f32: () | f64: () | string: () | array: &0 | map: (key_ty: &0, ty: &0)) | ref: U32)", | |||
"(ref: U32 | sum: (variants: Array<(name: (some: String | none: ()), algebraic_type: &0)>) | product: (elements: Array<(name: (some: String | none: ()), algebraic_type: &0)>) | array: &0 | map: (key_ty: &0, ty: &0) | bool: () | i8: () | u8: () | i16: () | u16: () | i32: () | u32: () | i64: () | u64: () | i128: () | u128: () | f32: () | f64: () | string: ())", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Because the type is structural is not the order significant?
8105110
to
2869549
Compare
Closing in favor of: #1559 |
Description of Changes
Flattens
BuiltinType
intoAlgebraicType
.See note below on BSATN ABI breakage.
API and ABI
If the API is breaking, please state below what will break
BSATN does not change for
AlgebraicValue
but it does forAlgebraicType
as this includes tags for each variant so the indirection ofBuiltinType
was present in the codec but is no longer.