From 7f3363156362ceac4f62dcf6f8c9c1580b6bcfcf Mon Sep 17 00:00:00 2001 From: Yan Chen Date: Sat, 26 Aug 2023 22:28:05 -0700 Subject: [PATCH 1/4] spec: allow record {} <: record {null} --- rust/candid/src/de.rs | 7 ++++--- rust/candid/tests/serde.rs | 4 ++-- spec/Candid.md | 10 +++++----- test/construct.test.did | 3 ++- test/prim.test.did | 3 ++- 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/rust/candid/src/de.rs b/rust/candid/src/de.rs index 0a0d0dc0..2c2f89f4 100644 --- a/rust/candid/src/de.rs +++ b/rust/candid/src/de.rs @@ -53,14 +53,14 @@ impl<'de> IDLDeserialize<'de> { if self.de.types.is_empty() { if matches!( expected_type.as_ref(), - TypeInner::Opt(_) | TypeInner::Reserved + TypeInner::Opt(_) | TypeInner::Reserved | TypeInner::Null ) { self.de.expect_type = expected_type; self.de.wire_type = TypeInner::Reserved.into(); return T::deserialize(&mut self.de); } else { return Err(Error::msg(format!( - "No more values on the wire, the expected type {expected_type} is not opt or reserved" + "No more values on the wire, the expected type {expected_type} is not opt, reserved or null" ))); } } @@ -548,7 +548,8 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> { { self.unroll_type()?; check!( - *self.expect_type == TypeInner::Null && *self.wire_type == TypeInner::Null, + *self.expect_type == TypeInner::Null + && matches!(*self.wire_type, TypeInner::Null | TypeInner::Reserved), "unit" ); visitor.visit_unit() diff --git a/rust/candid/tests/serde.rs b/rust/candid/tests/serde.rs index 0bccb579..3a09abe9 100644 --- a/rust/candid/tests/serde.rs +++ b/rust/candid/tests/serde.rs @@ -697,12 +697,12 @@ fn test_multiargs() { Vec<(Int, &str)>, (Int, String), Option, - candid::Reserved, + (), candid::Reserved ) .unwrap(); assert_eq!(tuple.2, None); - assert_eq!(tuple.3, candid::Reserved); + assert_eq!(tuple.3, ()); assert_eq!(tuple.4, candid::Reserved); } diff --git a/spec/Candid.md b/spec/Candid.md index ae7effca..e6f161ed 100644 --- a/spec/Candid.md +++ b/spec/Candid.md @@ -832,11 +832,11 @@ record { ;* } <: record { ;* } record { : ; ;* } <: record { : ; ;* } ``` -In order to be able to evolve and extend record types that also occur in inbound position (i.e., are used both as function results and function parameters), the subtype relation also supports *removing* fields from records, provided they are optional (or `reserved`). +In order to be able to evolve and extend record types that also occur in inbound position (i.e., are used both as function results and function parameters), the subtype relation also supports *removing* fields from records, provided they are optional, null, or `reserved`. ``` not in ;* record { ;* } <: record { ;* } -opt empty <: +null <: ------------------------------------------------------------------------------ record { ;* } <: record { : ; ;* } ``` @@ -980,11 +980,11 @@ In the following rule: * The `*` field names are those present in both the actual and expected type. The values are coerced. * the `*` field names those only in the actual type. The values are ignored. - * the `*` field names are those only in the expected type, which therefore must be of optional or reserved type. The `null` value is used for these. + * the `*` field names are those only in the expected type, which therefore must be of null, optional or reserved type. The `null` value is used for these. ``` : ~> : -opt empty <: +null <: ------------------------------------------------------------------------------------------- record { = ;* = ;* } : record { : ;* : ;* } ~> record { = ;* = null;* } : record { : ;* : ;* } @@ -1023,7 +1023,7 @@ NOTE: These rules above imply that a Candid decoder has to be able to decide the #### Tuple types -Whole argument and result sequences are coerced with the same rules as tuple-like records. In particular, extra arguments are ignored, and optional parameters read as as `null` if the argument is missing or fails to coerce: +Whole argument and result sequences are coerced with the same rules as tuple-like records. In particular, extra arguments are ignored, and optional/null parameters read as `null` if the argument is missing or fails to coerce: ``` record { ;* } : record { ;* } ~> record { ;* } : record { ;* } diff --git a/test/construct.test.did b/test/construct.test.did index 03a96771..bdaff825 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -82,6 +82,7 @@ assert blob "DIDL\01\6c\01\01\7c\01\00\2a" == "(record {})" assert "(record { whatever = 0 })" == "(record {})" : (record {}) "record: ignore fields (textual)"; assert blob "DIDL\01\6c\01\01\7c\01\00\2a" !: (record {2:int}) "record: missing field"; assert blob "DIDL\01\6c\01\01\7c\01\00\2a" == "(record { 2 = null })" : (record {2:opt int}) "record: missing opt field"; +assert blob "DIDL\01\6c\00\01\00" == "(record { 2 = null })" : (record {2:null}) "record: missing null field"; assert blob "DIDL\01\6c\02\00\7c\01\7e\01\00\2a\01" == "(record {42; true})" : (record {int; bool}) "record: tuple"; assert blob "DIDL\01\6c\02\00\7c\01\7e\01\00\2a\01" == "(record {1 = true})" : (record {1:bool}) "record: ignore fields"; assert blob "DIDL\01\6c\02\00\7c\01\7e\01\00\2a\01" !: (record {bool; int}) "record: type mismatch"; @@ -193,7 +194,7 @@ assert blob "DIDL\02\6b\02\d1\a7\cf\02\7f\f1\f3\92\8e\04\01\6c\02\a0\d2\ac\a8\04 == "(variant { cons = record { head = 1; tail = variant { cons = record { head = 2; tail = variant { nil } } } } })" : (VariantList) "variant: list"; assert blob "DIDL\02\6b\02\d1\a7\cf\02\7f\f1\f3\92\8e\04\01\6c\02\a0\d2\ac\a8\04\7c\90\ed\da\e7\04\00\01\00\00" - == "(variant {nil}, null, null, null, null)" : (VariantList, opt VariantList, opt empty, reserved, opt int) "variant: extra args"; + == "(variant {nil}, null, null, null, null)" : (VariantList, opt VariantList, null, reserved, opt int) "variant: extra args"; assert blob "DIDL\02\6b\02\d1\a7\cf\02\7f\f1\f3\92\8e\04\01\6c\02\a0\d2\ac\a8\04\7c\90\ed\da\e7\04\00\01\00\00" !: (VariantList, opt int, vec int) "non-null extra args"; diff --git a/test/prim.test.did b/test/prim.test.did index eff4c832..925dd64c 100644 --- a/test/prim.test.did +++ b/test/prim.test.did @@ -22,7 +22,7 @@ assert blob "DIDL\00\01\5e" !: () "Out of range type"; // Missing arguments assert blob "DIDL\00\00" !: (nat) "missing argument: nat fails"; assert blob "DIDL\00\00" !: (empty) "missing argument: empty fails"; -assert blob "DIDL\00\00" !: (null) "missing argument: null fails"; +assert blob "DIDL\00\00" == "(null)" : (null) "missing argument: null"; assert blob "DIDL\00\00" == "(null)" : (opt empty) "missing argument: opt empty"; assert blob "DIDL\00\00" == "(null)" : (opt null) "missing argument: opt null"; assert blob "DIDL\00\00" == "(null)" : (opt nat) "missing argument: opt nat"; @@ -189,6 +189,7 @@ assert blob "DIDL\00\01\70" == blob "DIDL\00\01\7f" : (reserved) "reser assert blob "DIDL\00\01\70" == blob "DIDL\00\01\7e\01" : (reserved) "reserved from bool"; assert blob "DIDL\00\01\70" == blob "DIDL\00\01\7d\80\01" : (reserved) "reserved from nat"; assert blob "DIDL\00\01\70" == blob "DIDL\00\01\71\06Motoko" : (reserved) "reserved from text"; +assert blob "DIDL\00\00" : (reserved) "reserved extra value"; assert blob "DIDL\00\01\71\05Motoko" !: (reserved) "reserved from too short text"; assert blob "DIDL\00\01\71\03\e2\28\a1" !: (reserved) "reserved from invalid utf8 text"; From 8cab4c048e615a921d17e9882a6053033c1aa1e0 Mon Sep 17 00:00:00 2001 From: Yan Chen Date: Sun, 27 Aug 2023 09:58:06 -0700 Subject: [PATCH 2/4] fix lint --- rust/candid/tests/serde.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rust/candid/tests/serde.rs b/rust/candid/tests/serde.rs index 3a09abe9..e1d6dcf6 100644 --- a/rust/candid/tests/serde.rs +++ b/rust/candid/tests/serde.rs @@ -702,7 +702,10 @@ fn test_multiargs() { ) .unwrap(); assert_eq!(tuple.2, None); - assert_eq!(tuple.3, ()); + #[allow(clippy::unit_cmp)] + { + assert_eq!(tuple.3, ()); + } assert_eq!(tuple.4, candid::Reserved); } From 11c443c0cb3588ee48f62aadb42f08fe3eb896ec Mon Sep 17 00:00:00 2001 From: Yan Chen Date: Tue, 29 Aug 2023 18:23:02 -0700 Subject: [PATCH 3/4] update subtype --- rust/candid/src/de.rs | 2 +- rust/candid/src/types/subtype.rs | 10 ++++++++-- test/subtypes.test.did | 6 +++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/rust/candid/src/de.rs b/rust/candid/src/de.rs index 2c2f89f4..96fe94fb 100644 --- a/rust/candid/src/de.rs +++ b/rust/candid/src/de.rs @@ -60,7 +60,7 @@ impl<'de> IDLDeserialize<'de> { return T::deserialize(&mut self.de); } else { return Err(Error::msg(format!( - "No more values on the wire, the expected type {expected_type} is not opt, reserved or null" + "No more values on the wire, the expected type {expected_type} is not opt, null, or reserved" ))); } } diff --git a/rust/candid/src/types/subtype.rs b/rust/candid/src/types/subtype.rs index d5362823..552c9b85 100644 --- a/rust/candid/src/types/subtype.rs +++ b/rust/candid/src/types/subtype.rs @@ -51,8 +51,14 @@ pub fn subtype(gamma: &mut Gamma, env: &TypeEnv, t1: &Type, t2: &Type) -> Result let fields: HashMap<_, _> = fs1.iter().map(|Field { id, ty }| (id, ty)).collect(); for Field { id, ty: ty2 } in fs2.iter() { match fields.get(id) { - Some(ty1) => subtype(gamma, env, ty1, ty2).with_context(|| format!("Record field {id}: {ty1} is not a subtype of {ty2}"))?, - None => subtype(gamma, env, &Opt(Empty.into()).into(), ty2).map_err(|_| anyhow::anyhow!("Record field {id}: {ty2} is only in the expected type and is not of opt or reserved type"))?, + Some(ty1) => subtype(gamma, env, ty1, ty2).with_context(|| { + format!("Record field {id}: {ty1} is not a subtype of {ty2}") + })?, + None => { + if !matches!(env.trace_type(ty2)?.as_ref(), Null | Reserved | Opt(_)) { + return Err(Error::msg(format!("Record field {id}: {ty2} is only in the expected type and is not of type opt, null or reserved"))); + } + } } } Ok(()) diff --git a/test/subtypes.test.did b/test/subtypes.test.did index 973f0207..71b3db18 100644 --- a/test/subtypes.test.did +++ b/test/subtypes.test.did @@ -76,7 +76,7 @@ assert blob "DIDL\02\6a\00\01\01\00\6c\00\01\00\01\01\00\01m" assert blob "DIDL\02\6a\00\01\01\00\6c\00\01\00\01\01\00\01m" == "(null)" : (opt func () -> (record { a : nat })) "record {} (record { a : null })) "record {} (record { a : null })) "record {} <: record { a : null }"; // optional func results assert blob "DIDL\02\6a\00\01\01\00\6a\00\00\00\01\00\01\01\00\01m" @@ -92,7 +92,7 @@ assert blob "DIDL\02\6a\00\01\01\00\6a\00\00\00\01\00\01\01\00\01m" assert blob "DIDL\02\6a\00\01\01\00\6a\00\00\00\01\00\01\01\00\01m" == "(null)" : (opt func () -> (func () -> (nat))) "func () -> () (nat)"; assert blob "DIDL\02\6a\00\01\01\00\6a\00\00\00\01\00\01\01\00\01m" - == "(null)" : (opt func () -> (func () -> (null))) "func () -> () (null)"; + == "(opt func \"aaaaa-aa\".m)" : (opt func () -> (func () -> (null))) "func () -> () <: func () -> (null)"; // optional func arguments assert blob "DIDL\03\6a\00\01\01\00\6a\01\02\00\00\6e\6f\01\00\01\01\00\01m" @@ -106,7 +106,7 @@ assert blob "DIDL\02\6a\00\01\01\00\6a\01\6f\00\00\01\00\01\01\00\01m" assert blob "DIDL\02\6a\00\01\01\00\6a\01\7d\00\00\01\00\01\01\00\01m" == "(null)" : (opt func () -> (func () -> ())) "func (nat) -> () ()"; assert blob "DIDL\02\6a\00\01\01\00\6a\01\7f\00\00\01\00\01\01\00\01m" - == "(null)" : (opt func () -> (func () -> ())) "func (null) -> () ()"; + == "(opt func \"aaaaa-aa\".m)" : (opt func () -> (func () -> ())) "func (null) -> () <: func () -> ()"; // variants assert blob "DIDL\02\6a\00\01\01\00\6b\00\01\00\01\01\00\01m" From db0ed9ca0d1b43f69807d6b931ddc858fbb8f618 Mon Sep 17 00:00:00 2001 From: Yan Chen Date: Tue, 29 Aug 2023 18:24:46 -0700 Subject: [PATCH 4/4] bump spec version --- spec/Candid.md | 4 ++-- test/construct.test.did | 2 +- test/overshoot.test.did | 2 +- test/prim.test.did | 2 +- test/reference.test.did | 2 +- test/subtypes.test.did | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/Candid.md b/spec/Candid.md index e6f161ed..c96c38fd 100644 --- a/spec/Candid.md +++ b/spec/Candid.md @@ -1,8 +1,8 @@ # Candid Specification -Version: 0.1.5 +Version: 0.1.6 -Date: June 1, 2023 +Date: August 29, 2023 ## Motivation diff --git a/test/construct.test.did b/test/construct.test.did index bdaff825..a7db8f7d 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -1,7 +1,7 @@ /* Encoding tests for construct types -Corresponding to spec version version 0.1.4 +Corresponding to spec version version 0.1.6 */ // Type definitions diff --git a/test/overshoot.test.did b/test/overshoot.test.did index 3e04ffd3..46a29a48 100644 --- a/test/overshoot.test.did +++ b/test/overshoot.test.did @@ -1,5 +1,5 @@ /* -Corresponding to spec version version 0.1.4 +Corresponding to spec version version 0.1.6 The tests in this file involve large leb-encoded values that indicate sizes (of tables, arrays, blobs, etc.). In all cases, a decode should be able to catch diff --git a/test/prim.test.did b/test/prim.test.did index 925dd64c..bfd3a4b9 100644 --- a/test/prim.test.did +++ b/test/prim.test.did @@ -1,7 +1,7 @@ /* Encoding tests for primitive types -Corresponding to spec version version 0.1.4 +Corresponding to spec version version 0.1.6 */ // fundamentally wrong diff --git a/test/reference.test.did b/test/reference.test.did index d2158724..51ff72ea 100644 --- a/test/reference.test.did +++ b/test/reference.test.did @@ -1,7 +1,7 @@ /* Encoding tests for reference types -Corresponding to spec version version 0.1.4 +Corresponding to spec version version 0.1.6 */ // principal diff --git a/test/subtypes.test.did b/test/subtypes.test.did index 71b3db18..cd92605a 100644 --- a/test/subtypes.test.did +++ b/test/subtypes.test.did @@ -1,7 +1,7 @@ /* Encoding tests for subtype tests in decoders -Corresponding to spec version version 0.1.4 +Corresponding to spec version version 0.1.6 This test file contains tests for the subtype check that decoders are expected to perform upon references.