From 57bdd3b0bf0ff10005d180761153fa24b376164e Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Fri, 30 Oct 2020 11:35:28 +0100 Subject: [PATCH 01/30] Some more tests related to subtyping this adds some tests to account for the new behaviour of https://github.com/dfinity/candid/pull/110f Just a first stab, untested, and kinda running out of stream. --- test/construct.test.did | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/construct.test.did b/test/construct.test.did index 59f50597f..3e43225d0 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -85,6 +85,27 @@ assert blob "DIDL\01\6c\02\00\7c\01\7e\02\7e\01\00\2a\01\00" ! assert blob "DIDL\01\6c\02\00\01\01\7e\01\00\00\00" !: (record {}) "record: type out of range"; assert blob "DIDL\01\6c\02\00\01\01\7e\01\7c\2a" !: (reserved) "record: type out of range"; +// opt +assert blob "DIDL\00\01\7f" == "(null)" : (opt empty) "opt: parsing null : null"; +assert blob "DIDL\01\6e\6f\00\00\00" == "(null)" : (opt empty) "opt: parsing null : opt empty"; +assert blob "DIDL\01\6e\7e\00\00\00" == "(null)" : (opt bool) "opt: parsing null : opt bool "; +assert blob "DIDL\01\6e\7e\00\00\01\00" == "(opt true)" : (opt bool) "opt: parsing opt true : opt bool"; +assert blob "DIDL\01\6e\7e\00\00\01\01" == "(opt false)" : (opt bool) "opt: parsing opt false : opt bool"; +assert blob "DIDL\01\6e\7e\00\00\01\02" !: (opt bool) "opt: parsing invalid bool at opt bool"; + +// opt subtype to constituent +assert blob "DIDL\00\01\7e\00" == "(opt true)" : (opt bool) "opt: parsing opt true : opt bool"; +assert blob "DIDL\00\01\7e\01" == "(opt false)" : (opt bool) "opt: parsing opt false : opt bool"; +assert blob "DIDL\00\01\7e\02" !: (opt bool) "opt: parsing invalid bool at opt bool"; +assert blob "DIDL\01\6e\7f\01\00\00" == "(null)" : (opt opt null) "opt: parsing (null : opt null) at opt opt null gives null, not opt null"; + +// special opt and record subtyping +assert blob "DIDL\01\6e\7e\01\00\00" == "(null)" : (opt nat) "null : opt bool <: opt nat"; +assert blob "DIDL\01\6e\7e\01\00\01\01" == "(null)" : (opt nat) "opt true : opt bool <: opt nat"; +assert blob "DIDL\01\6e\7e\01\00\01\02" !: (opt nat) "opt bool <: opt nat with invalid boolean value"; +assert blob "DIDL\01\6e\6e\7e\01\00\01\01\01" == "(null)" : (opt nat) "opt opt true : opt opt bool <: opt nat"; +assert blob "DIDL\01\6e\6e\7e\01\00\01\01\01" == "(opt null)" : (opt opt nat) "opt opt true : opt opt bool <: opt opt nat"; + // variant assert "(variant {})" !: (variant {}) "variant: no empty value"; assert blob "DIDL\01\6b\00\01\00" !: (variant {}) "variant: no empty value"; From 671c80fff570166febd626a70f9d1973bbd7925d Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Fri, 30 Oct 2020 11:37:51 +0100 Subject: [PATCH 02/30] I guess this one is good now --- test/construct.test.did | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/construct.test.did b/test/construct.test.did index 3e43225d0..7c5e9019b 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -111,9 +111,7 @@ assert "(variant {})" ! assert blob "DIDL\01\6b\00\01\00" !: (variant {}) "variant: no empty value"; assert blob "DIDL\01\6b\01\00\7f\01\00\00" == "(variant {0})" : (variant {0}) "variant: numbered field"; assert blob "DIDL\01\6b\01\00\7f\01\00\00\2a" !: (variant {0:int}) "variant: type mismatch"; - -// TODO Should this pass? -assert blob "DIDL\01\6b\02\00\7f\01\7c\01\00\01\2a" : (variant {0:int; 1:int}) "??? variant: type mismatch"; +assert blob "DIDL\01\6b\02\00\7f\01\7c\01\00\01\2a" : (variant {0:int; 1:int}) "variant: type mismatch in unused tag"; assert blob "DIDL\01\6b\01\00\7f\01\00\00" == "(variant {0})" : (variant {0;1}) "variant: ignore field"; assert blob "DIDL\01\6b\02\00\7f\01\7f\01\00\00" == "(variant {0})" : (variant {0}) "variant: ignore field"; assert blob "DIDL\01\6b\02\00\7f\01\7f\01\00\01" == "(variant {1})" : (variant {1;2}) "variant: change index"; From 92fa86cd0d6d1eaab5a37ca3bf5b99da7c0e3f30 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Fri, 30 Oct 2020 12:07:39 +0100 Subject: [PATCH 03/30] Allow null : opt t subtyping? --- rust/candid/src/parser/value.rs | 2 +- test/construct.test.did | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/rust/candid/src/parser/value.rs b/rust/candid/src/parser/value.rs index 6c6cc5e9f..253fc07ad 100644 --- a/rust/candid/src/parser/value.rs +++ b/rust/candid/src/parser/value.rs @@ -154,7 +154,7 @@ impl IDLValue { let ty = crate::types::internal::find_type(id).unwrap(); self.annotate_type(from_parser, env, &ty)? } - (IDLValue::Null, Type::Opt(_)) if from_parser => IDLValue::None, + (IDLValue::Null, Type::Opt(_)) => IDLValue::None, (IDLValue::Float64(n), Type::Float32) if from_parser => IDLValue::Float32(*n as f32), (IDLValue::Number(str), t) if from_parser => match t { Type::Int => IDLValue::Int(str.parse::()?), diff --git a/test/construct.test.did b/test/construct.test.did index 7c5e9019b..e82e9d764 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -87,11 +87,11 @@ assert blob "DIDL\01\6c\02\00\01\01\7e\01\7c\2a" ! // opt assert blob "DIDL\00\01\7f" == "(null)" : (opt empty) "opt: parsing null : null"; -assert blob "DIDL\01\6e\6f\00\00\00" == "(null)" : (opt empty) "opt: parsing null : opt empty"; -assert blob "DIDL\01\6e\7e\00\00\00" == "(null)" : (opt bool) "opt: parsing null : opt bool "; -assert blob "DIDL\01\6e\7e\00\00\01\00" == "(opt true)" : (opt bool) "opt: parsing opt true : opt bool"; -assert blob "DIDL\01\6e\7e\00\00\01\01" == "(opt false)" : (opt bool) "opt: parsing opt false : opt bool"; -assert blob "DIDL\01\6e\7e\00\00\01\02" !: (opt bool) "opt: parsing invalid bool at opt bool"; +assert blob "DIDL\01\6e\6f\01\00\00" == "(null)" : (opt empty) "opt: parsing null : opt empty"; +assert blob "DIDL\01\6e\7e\01\00\00" == "(null)" : (opt bool) "opt: parsing null : opt bool "; +assert blob "DIDL\01\6e\7e\01\00\01\00" == "(opt true)" : (opt bool) "opt: parsing opt true : opt bool"; +assert blob "DIDL\01\6e\7e\01\00\01\01" == "(opt false)" : (opt bool) "opt: parsing opt false : opt bool"; +assert blob "DIDL\01\6e\7e\01\00\01\02" !: (opt bool) "opt: parsing invalid bool at opt bool"; // opt subtype to constituent assert blob "DIDL\00\01\7e\00" == "(opt true)" : (opt bool) "opt: parsing opt true : opt bool"; From 23520f69853019b94cff3130749c45c75d1145e4 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Tue, 3 Nov 2020 10:43:46 +0100 Subject: [PATCH 04/30] Test case: add `nat <: int` tests --- test/prim.test.did | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/prim.test.did b/test/prim.test.did index 90d055d53..2035f9ec9 100644 --- a/test/prim.test.did +++ b/test/prim.test.did @@ -52,6 +52,12 @@ assert blob "DIDL\00\01\7c\80\7f" == "(-128)" : (int) "int: leb not overlong w assert blob "DIDL\00\01\7c\80\80\98\f4\e9\b5\ca\ea\00" == "(60000000000000000)" : (int) "int: big number"; assert blob "DIDL\00\01\7c\80\80\e8\8b\96\ca\b5\95\7f" == "(-60000000000000000)" : (int) "int: negative big number"; +assert blob "DIDL\00\01\7d\00" == "(0)" : (int) "nat <: int: 0"; +assert blob "DIDL\00\01\7d\01" == "(1)" : (int) "nat <: int: 1"; +assert blob "DIDL\00\01\7d\7f" == "(127)" : (int) "nat <: int: leb (two bytes)"; +assert blob "DIDL\00\01\7d\80\01" == "(128)" : (int) "nat <: int: leb (two bytes)"; +assert blob "DIDL\00\01\7d\ff\7f" == "(16383)" : (int) "nat <: int: leb (two bytes, all bits)"; + assert blob "DIDL\00\01\7b\00" == "(0)" : (nat8) "nat8: 0"; assert blob "DIDL\00\01\7b\01" == "(1)" : (nat8) "nat8: 1"; assert blob "DIDL\00\01\7b\ff" == "(255)" : (nat8) "nat8: 255"; From c666291fe7b7717d0cb3762e5dca44958e217c0e Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Tue, 3 Nov 2020 10:57:57 +0100 Subject: [PATCH 05/30] nat/int subtyping --- rust/candid/src/parser/value.rs | 3 ++- rust/candid/src/types/number.rs | 8 ++++++++ rust/candid/tests/test_suite.rs | 5 ++++- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/rust/candid/src/parser/value.rs b/rust/candid/src/parser/value.rs index 253fc07ad..8015e6a80 100644 --- a/rust/candid/src/parser/value.rs +++ b/rust/candid/src/parser/value.rs @@ -177,8 +177,9 @@ impl IDLValue { (_, Type::Reserved) => IDLValue::Reserved, (IDLValue::Null, Type::Null) => IDLValue::Null, (IDLValue::Bool(b), Type::Bool) => IDLValue::Bool(*b), - (IDLValue::Int(i), Type::Int) => IDLValue::Int(i.clone()), (IDLValue::Nat(n), Type::Nat) => IDLValue::Nat(n.clone()), + (IDLValue::Int(i), Type::Int) => IDLValue::Int(i.clone()), + (IDLValue::Nat(n), Type::Int) => IDLValue::Int(n.clone().into()), (IDLValue::Nat8(n), Type::Nat8) => IDLValue::Nat8(*n), (IDLValue::Nat16(n), Type::Nat16) => IDLValue::Nat16(*n), (IDLValue::Nat32(n), Type::Nat32) => IDLValue::Nat32(*n), diff --git a/rust/candid/src/types/number.rs b/rust/candid/src/types/number.rs index 605a494c7..b98b2d6c0 100644 --- a/rust/candid/src/types/number.rs +++ b/rust/candid/src/types/number.rs @@ -26,6 +26,14 @@ impl From for Nat { } } +impl From for Int { + #[inline(always)] + fn from(n: Nat) -> Self { + let i : BigInt = n.0.into(); + i.into() + } +} + impl From for BigInt { #[inline(always)] fn from(i: Int) -> Self { diff --git a/rust/candid/tests/test_suite.rs b/rust/candid/tests/test_suite.rs index e3a68fa1d..f91600d3d 100644 --- a/rust/candid/tests/test_suite.rs +++ b/rust/candid/tests/test_suite.rs @@ -8,7 +8,10 @@ fn decode_test(resource: &str) { .join(resource); let test = std::fs::read_to_string(path).unwrap(); let ast = test.parse::().unwrap(); - check(ast).unwrap(); + match check(ast) { + Ok(()) => (), + Err(err) => println!("Failed: {}", err) + } } #[test_generator::test_resources("test/*.test.did")] From 82b71f4d08c84294bdb3d8e591f6fe16a3d1062d Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Tue, 3 Nov 2020 11:02:08 +0100 Subject: [PATCH 06/30] Can the test suite be more liberal? --- rust/candid/tests/test_suite.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rust/candid/tests/test_suite.rs b/rust/candid/tests/test_suite.rs index f91600d3d..9af3bde5f 100644 --- a/rust/candid/tests/test_suite.rs +++ b/rust/candid/tests/test_suite.rs @@ -22,6 +22,8 @@ fn js_test(resource: &str) { .join("../../") .join(resource); let test = std::fs::read_to_string(path).unwrap(); - let ast = test.parse::().unwrap(); - println!("{}", test_generate(ast)); + match test.parse::() { + Ok(ast) => println!("{}", test_generate(ast)), + Err(err) => println!("Failed: {}", err) + } } From 0b3e1a9d738b97420e603e8b9e9b83d6cd35fb11 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Tue, 3 Nov 2020 11:08:05 +0100 Subject: [PATCH 07/30] stash --- rust/candid/src/parser/value.rs | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/rust/candid/src/parser/value.rs b/rust/candid/src/parser/value.rs index 8015e6a80..ef1251109 100644 --- a/rust/candid/src/parser/value.rs +++ b/rust/candid/src/parser/value.rs @@ -154,7 +154,6 @@ impl IDLValue { let ty = crate::types::internal::find_type(id).unwrap(); self.annotate_type(from_parser, env, &ty)? } - (IDLValue::Null, Type::Opt(_)) => IDLValue::None, (IDLValue::Float64(n), Type::Float32) if from_parser => IDLValue::Float32(*n as f32), (IDLValue::Number(str), t) if from_parser => match t { Type::Int => IDLValue::Int(str.parse::()?), @@ -191,10 +190,27 @@ impl IDLValue { (IDLValue::Float64(n), Type::Float64) => IDLValue::Float64(*n), (IDLValue::Float32(n), Type::Float32) => IDLValue::Float32(*n), (IDLValue::Text(s), Type::Text) => IDLValue::Text(s.to_owned()), + (IDLValue::Null, Type::Opt(_)) => IDLValue::None, + (IDLValue::Reserved, Type::Opt(_)) => { + return Err(Error::msg(format!( + "type mismatch: reserved cannot be of type {}", + t + ))) + } (IDLValue::None, Type::Opt(_)) => IDLValue::None, (IDLValue::Opt(v), Type::Opt(ty)) => { - let v = v.annotate_type(from_parser, env, ty)?; - IDLValue::Opt(Box::new(v)) + // liberal decoding of optionals + match v.annotate_type(from_parser, env, ty) { + Ok(v) => IDLValue::Opt(Box::new(v)), + Err(_) => IDLValue::None + } + } + // Try consituent type + (v, Type::Opt(ty)) => { + match v.annotate_type(from_parser, env, ty) { + Ok(v) => IDLValue::Opt(Box::new(v)), + Err(_) => IDLValue::None + } } (IDLValue::Vec(vec), Type::Vec(ty)) => { let mut res = Vec::new(); From 688b2360409beb23425a353b28f0c6e3a79995cc Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Tue, 3 Nov 2020 11:08:31 +0100 Subject: [PATCH 08/30] Undo changes to test suite runnter --- rust/candid/tests/test_suite.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/rust/candid/tests/test_suite.rs b/rust/candid/tests/test_suite.rs index 9af3bde5f..e3a68fa1d 100644 --- a/rust/candid/tests/test_suite.rs +++ b/rust/candid/tests/test_suite.rs @@ -8,10 +8,7 @@ fn decode_test(resource: &str) { .join(resource); let test = std::fs::read_to_string(path).unwrap(); let ast = test.parse::().unwrap(); - match check(ast) { - Ok(()) => (), - Err(err) => println!("Failed: {}", err) - } + check(ast).unwrap(); } #[test_generator::test_resources("test/*.test.did")] @@ -22,8 +19,6 @@ fn js_test(resource: &str) { .join("../../") .join(resource); let test = std::fs::read_to_string(path).unwrap(); - match test.parse::() { - Ok(ast) => println!("{}", test_generate(ast)), - Err(err) => println!("Failed: {}", err) - } + let ast = test.parse::().unwrap(); + println!("{}", test_generate(ast)); } From 2173a5105212b357687297f3b11004404ec67296 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Tue, 3 Nov 2020 11:10:28 +0100 Subject: [PATCH 09/30] comment --- rust/candid/src/parser/value.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/candid/src/parser/value.rs b/rust/candid/src/parser/value.rs index ef1251109..6a5793456 100644 --- a/rust/candid/src/parser/value.rs +++ b/rust/candid/src/parser/value.rs @@ -190,6 +190,7 @@ impl IDLValue { (IDLValue::Float64(n), Type::Float64) => IDLValue::Float64(*n), (IDLValue::Float32(n), Type::Float32) => IDLValue::Float32(*n), (IDLValue::Text(s), Type::Text) => IDLValue::Text(s.to_owned()), + // opt parsing. NB: Always succeds! (IDLValue::Null, Type::Opt(_)) => IDLValue::None, (IDLValue::Reserved, Type::Opt(_)) => { return Err(Error::msg(format!( From 7196f9bb3054a2cfd7588de994b74907e733553c Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Tue, 3 Nov 2020 11:12:14 +0100 Subject: [PATCH 10/30] reserved <: opt t --- rust/candid/src/parser/value.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/rust/candid/src/parser/value.rs b/rust/candid/src/parser/value.rs index 6a5793456..4fdd5a4ff 100644 --- a/rust/candid/src/parser/value.rs +++ b/rust/candid/src/parser/value.rs @@ -192,12 +192,7 @@ impl IDLValue { (IDLValue::Text(s), Type::Text) => IDLValue::Text(s.to_owned()), // opt parsing. NB: Always succeds! (IDLValue::Null, Type::Opt(_)) => IDLValue::None, - (IDLValue::Reserved, Type::Opt(_)) => { - return Err(Error::msg(format!( - "type mismatch: reserved cannot be of type {}", - t - ))) - } + (IDLValue::Reserved, Type::Opt(_)) => IDLValue::None, (IDLValue::None, Type::Opt(_)) => IDLValue::None, (IDLValue::Opt(v), Type::Opt(ty)) => { // liberal decoding of optionals From 66624c2e44882e562b436532221e3599cf40bc99 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Tue, 3 Nov 2020 11:22:05 +0100 Subject: [PATCH 11/30] Fix test cases (forgot about a-normal-form) --- test/construct.test.did | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/construct.test.did b/test/construct.test.did index e82e9d764..b88237e0d 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -93,6 +93,11 @@ assert blob "DIDL\01\6e\7e\01\00\01\00" == "(opt true)" assert blob "DIDL\01\6e\7e\01\00\01\01" == "(opt false)" : (opt bool) "opt: parsing opt false : opt bool"; assert blob "DIDL\01\6e\7e\01\00\01\02" !: (opt bool) "opt: parsing invalid bool at opt bool"; +// nested opt +assert blob "DIDL\02\6e\01\6e\7e\01\00\00" == "(null)" : (opt opt bool) "opt: parsing null : opt opt bool"; +assert blob "DIDL\02\6e\01\6e\7e\01\00\01\00" == "(opt null)" : (opt opt bool) "opt: parsing opt null : opt opt bool"; +assert blob "DIDL\02\6e\01\6e\7e\01\00\01\01\00" == "(opt opt false)" : (opt opt bool) "opt: parsing opt opt false : opt opt bool"; + // opt subtype to constituent assert blob "DIDL\00\01\7e\00" == "(opt true)" : (opt bool) "opt: parsing opt true : opt bool"; assert blob "DIDL\00\01\7e\01" == "(opt false)" : (opt bool) "opt: parsing opt false : opt bool"; @@ -103,8 +108,8 @@ assert blob "DIDL\01\6e\7f\01\00\00" == "(null)" assert blob "DIDL\01\6e\7e\01\00\00" == "(null)" : (opt nat) "null : opt bool <: opt nat"; assert blob "DIDL\01\6e\7e\01\00\01\01" == "(null)" : (opt nat) "opt true : opt bool <: opt nat"; assert blob "DIDL\01\6e\7e\01\00\01\02" !: (opt nat) "opt bool <: opt nat with invalid boolean value"; -assert blob "DIDL\01\6e\6e\7e\01\00\01\01\01" == "(null)" : (opt nat) "opt opt true : opt opt bool <: opt nat"; -assert blob "DIDL\01\6e\6e\7e\01\00\01\01\01" == "(opt null)" : (opt opt nat) "opt opt true : opt opt bool <: opt opt nat"; +assert blob "DIDL\02\6e\01\6e\7e\01\00\01\01\01" == "(null)" : (opt nat) "opt opt true : opt opt bool <: opt nat"; +assert blob "DIDL\02\6e\01\6e\7e\01\00\01\01\01" == "(opt null)" : (opt opt nat) "opt opt true : opt opt bool <: opt opt nat"; // variant assert "(variant {})" !: (variant {}) "variant: no empty value"; From 4cb1d32b156e9043125f4d93b39ccb90c0ac3159 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Tue, 3 Nov 2020 11:48:16 +0100 Subject: [PATCH 12/30] Fix more test cases --- test/construct.test.did | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/construct.test.did b/test/construct.test.did index b88237e0d..51c49fd6c 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -89,8 +89,8 @@ assert blob "DIDL\01\6c\02\00\01\01\7e\01\7c\2a" ! assert blob "DIDL\00\01\7f" == "(null)" : (opt empty) "opt: parsing null : null"; assert blob "DIDL\01\6e\6f\01\00\00" == "(null)" : (opt empty) "opt: parsing null : opt empty"; assert blob "DIDL\01\6e\7e\01\00\00" == "(null)" : (opt bool) "opt: parsing null : opt bool "; -assert blob "DIDL\01\6e\7e\01\00\01\00" == "(opt true)" : (opt bool) "opt: parsing opt true : opt bool"; -assert blob "DIDL\01\6e\7e\01\00\01\01" == "(opt false)" : (opt bool) "opt: parsing opt false : opt bool"; +assert blob "DIDL\01\6e\7e\01\00\01\00" == "(opt false)" : (opt bool) "opt: parsing opt false : opt bool"; +assert blob "DIDL\01\6e\7e\01\00\01\01" == "(opt true)" : (opt bool) "opt: parsing opt true : opt bool"; assert blob "DIDL\01\6e\7e\01\00\01\02" !: (opt bool) "opt: parsing invalid bool at opt bool"; // nested opt @@ -99,8 +99,8 @@ assert blob "DIDL\02\6e\01\6e\7e\01\00\01\00" == "(opt null)" assert blob "DIDL\02\6e\01\6e\7e\01\00\01\01\00" == "(opt opt false)" : (opt opt bool) "opt: parsing opt opt false : opt opt bool"; // opt subtype to constituent -assert blob "DIDL\00\01\7e\00" == "(opt true)" : (opt bool) "opt: parsing opt true : opt bool"; -assert blob "DIDL\00\01\7e\01" == "(opt false)" : (opt bool) "opt: parsing opt false : opt bool"; +assert blob "DIDL\00\01\7e\00" == "(opt false)" : (opt bool) "opt: parsing true : opt bool"; +assert blob "DIDL\00\01\7e\01" == "(opt true)" : (opt bool) "opt: parsing false : opt bool"; assert blob "DIDL\00\01\7e\02" !: (opt bool) "opt: parsing invalid bool at opt bool"; assert blob "DIDL\01\6e\7f\01\00\00" == "(null)" : (opt opt null) "opt: parsing (null : opt null) at opt opt null gives null, not opt null"; From 47d5fc505f2d357bc58f94aa7e0bf082c1f5e651 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Tue, 3 Nov 2020 12:08:14 +0100 Subject: [PATCH 13/30] Optional record fiels are optional --- rust/candid/src/parser/value.rs | 9 ++++++--- rust/candid/src/types/internal.rs | 7 +++++++ test/construct.test.did | 1 + 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/rust/candid/src/parser/value.rs b/rust/candid/src/parser/value.rs index 4fdd5a4ff..64a8015f0 100644 --- a/rust/candid/src/parser/value.rs +++ b/rust/candid/src/parser/value.rs @@ -221,9 +221,12 @@ impl IDLValue { vec.iter().map(|IDLField { id, val }| (id, val)).collect(); let mut res = Vec::new(); for Field { id, ty } in fs.iter() { - let val = fields - .get(&id) - .ok_or_else(|| Error::msg(format!("field {} not found", id)))?; + let val = fields.get(&id); + let val = if ty.is_opt() { + val.unwrap_or(&&IDLValue::Null) + } else { + val.ok_or_else(|| Error::msg(format!("required field {} not found", id)))? + }; let val = val.annotate_type(from_parser, env, ty)?; res.push(IDLField { id: id.clone(), diff --git a/rust/candid/src/types/internal.rs b/rust/candid/src/types/internal.rs index 00531bab9..13b497483 100644 --- a/rust/candid/src/types/internal.rs +++ b/rust/candid/src/types/internal.rs @@ -205,6 +205,13 @@ impl Type { _ => false, } } + + pub(crate) fn is_opt(&self) -> bool { + match self { + Type::Opt(_) =>true, + _ => false, + } + } } impl fmt::Display for Type { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { diff --git a/test/construct.test.did b/test/construct.test.did index 51c49fd6c..2a6f89519 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -110,6 +110,7 @@ assert blob "DIDL\01\6e\7e\01\00\01\01" == "(null)" : (opt nat) "opt true : opt assert blob "DIDL\01\6e\7e\01\00\01\02" !: (opt nat) "opt bool <: opt nat with invalid boolean value"; assert blob "DIDL\02\6e\01\6e\7e\01\00\01\01\01" == "(null)" : (opt nat) "opt opt true : opt opt bool <: opt nat"; assert blob "DIDL\02\6e\01\6e\7e\01\00\01\01\01" == "(opt null)" : (opt opt nat) "opt opt true : opt opt bool <: opt opt nat"; +assert blob "DIDL\01\6c\00\01\00" == "(record { foo = null })" : (record { foo : opt bool }) "missing optional field"; // variant assert "(variant {})" !: (variant {}) "variant: no empty value"; From 429ea4a7ec81d9eb12130209cea759d188fadc8d Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Tue, 3 Nov 2020 16:11:31 +0100 Subject: [PATCH 14/30] Reserved fields may be missing (see #131, spec update in #128) --- rust/candid/src/parser/value.rs | 2 ++ rust/candid/src/types/internal.rs | 7 +++++++ test/construct.test.did | 5 ++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/rust/candid/src/parser/value.rs b/rust/candid/src/parser/value.rs index 64a8015f0..4e16c0078 100644 --- a/rust/candid/src/parser/value.rs +++ b/rust/candid/src/parser/value.rs @@ -224,6 +224,8 @@ impl IDLValue { let val = fields.get(&id); let val = if ty.is_opt() { val.unwrap_or(&&IDLValue::Null) + } else if ty.is_reserved() { + val.unwrap_or(&&IDLValue::Reserved) } else { val.ok_or_else(|| Error::msg(format!("required field {} not found", id)))? }; diff --git a/rust/candid/src/types/internal.rs b/rust/candid/src/types/internal.rs index 13b497483..cae8e3d72 100644 --- a/rust/candid/src/types/internal.rs +++ b/rust/candid/src/types/internal.rs @@ -212,6 +212,13 @@ impl Type { _ => false, } } + + pub(crate) fn is_reserved(&self) -> bool { + match self { + Type::Reserved =>true, + _ => false, + } + } } impl fmt::Display for Type { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { diff --git a/test/construct.test.did b/test/construct.test.did index 2a6f89519..2fcc3c13a 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -104,13 +104,16 @@ assert blob "DIDL\00\01\7e\01" == "(opt true)" assert blob "DIDL\00\01\7e\02" !: (opt bool) "opt: parsing invalid bool at opt bool"; assert blob "DIDL\01\6e\7f\01\00\00" == "(null)" : (opt opt null) "opt: parsing (null : opt null) at opt opt null gives null, not opt null"; -// special opt and record subtyping +// special opt subtyping assert blob "DIDL\01\6e\7e\01\00\00" == "(null)" : (opt nat) "null : opt bool <: opt nat"; assert blob "DIDL\01\6e\7e\01\00\01\01" == "(null)" : (opt nat) "opt true : opt bool <: opt nat"; assert blob "DIDL\01\6e\7e\01\00\01\02" !: (opt nat) "opt bool <: opt nat with invalid boolean value"; assert blob "DIDL\02\6e\01\6e\7e\01\00\01\01\01" == "(null)" : (opt nat) "opt opt true : opt opt bool <: opt nat"; assert blob "DIDL\02\6e\01\6e\7e\01\00\01\01\01" == "(opt null)" : (opt opt nat) "opt opt true : opt opt bool <: opt opt nat"; + +// special record field rules assert blob "DIDL\01\6c\00\01\00" == "(record { foo = null })" : (record { foo : opt bool }) "missing optional field"; +assert blob "DIDL\01\6c\00\01\00" == "(record { foo = \"☃\" })" : (record { foo : reserved }) "missing reserved field"; // variant assert "(variant {})" !: (variant {}) "variant: no empty value"; From b6a246e1f672345519cbfa14370d3c0b0cc029cb Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Fri, 13 Nov 2020 14:19:03 +0100 Subject: [PATCH 15/30] Run cargo-fmt --- rust/candid/src/parser/value.rs | 12 +++++------- rust/candid/src/types/internal.rs | 4 ++-- rust/candid/src/types/number.rs | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/rust/candid/src/parser/value.rs b/rust/candid/src/parser/value.rs index 4e16c0078..4b4c0268d 100644 --- a/rust/candid/src/parser/value.rs +++ b/rust/candid/src/parser/value.rs @@ -198,16 +198,14 @@ impl IDLValue { // liberal decoding of optionals match v.annotate_type(from_parser, env, ty) { Ok(v) => IDLValue::Opt(Box::new(v)), - Err(_) => IDLValue::None + Err(_) => IDLValue::None, } } // Try consituent type - (v, Type::Opt(ty)) => { - match v.annotate_type(from_parser, env, ty) { - Ok(v) => IDLValue::Opt(Box::new(v)), - Err(_) => IDLValue::None - } - } + (v, Type::Opt(ty)) => match v.annotate_type(from_parser, env, ty) { + Ok(v) => IDLValue::Opt(Box::new(v)), + Err(_) => IDLValue::None, + }, (IDLValue::Vec(vec), Type::Vec(ty)) => { let mut res = Vec::new(); for e in vec.iter() { diff --git a/rust/candid/src/types/internal.rs b/rust/candid/src/types/internal.rs index cae8e3d72..336be7842 100644 --- a/rust/candid/src/types/internal.rs +++ b/rust/candid/src/types/internal.rs @@ -208,14 +208,14 @@ impl Type { pub(crate) fn is_opt(&self) -> bool { match self { - Type::Opt(_) =>true, + Type::Opt(_) => true, _ => false, } } pub(crate) fn is_reserved(&self) -> bool { match self { - Type::Reserved =>true, + Type::Reserved => true, _ => false, } } diff --git a/rust/candid/src/types/number.rs b/rust/candid/src/types/number.rs index b98b2d6c0..7546345a1 100644 --- a/rust/candid/src/types/number.rs +++ b/rust/candid/src/types/number.rs @@ -29,7 +29,7 @@ impl From for Nat { impl From for Int { #[inline(always)] fn from(n: Nat) -> Self { - let i : BigInt = n.0.into(); + let i: BigInt = n.0.into(); i.into() } } From e96249ccb595fe6586bfc1bc9d915c1037d98d1a Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Fri, 13 Nov 2020 22:54:22 +0100 Subject: [PATCH 16/30] Test case for reserved <: opt nat as per https://github.com/dfinity/candid/pull/134 --- test/construct.test.did | 1 + 1 file changed, 1 insertion(+) diff --git a/test/construct.test.did b/test/construct.test.did index 2fcc3c13a..59e009b5a 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -105,6 +105,7 @@ assert blob "DIDL\00\01\7e\02" ! assert blob "DIDL\01\6e\7f\01\00\00" == "(null)" : (opt opt null) "opt: parsing (null : opt null) at opt opt null gives null, not opt null"; // special opt subtyping +assert blob "DIDL\00\01\70" == "(null)" : (opt nat) "reserved <: opt nat"; assert blob "DIDL\01\6e\7e\01\00\00" == "(null)" : (opt nat) "null : opt bool <: opt nat"; assert blob "DIDL\01\6e\7e\01\00\01\01" == "(null)" : (opt nat) "opt true : opt bool <: opt nat"; assert blob "DIDL\01\6e\7e\01\00\01\02" !: (opt nat) "opt bool <: opt nat with invalid boolean value"; From 7aae87f55b98677d6834384b9eb35b4d1cbffaf3 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Tue, 17 Nov 2020 11:27:25 +0100 Subject: [PATCH 17/30] Add `true : fix opt` test causes a stack overflow. also see discussion in https://github.com/dfinity/candid/issues/135 --- test/construct.test.did | 1 + 1 file changed, 1 insertion(+) diff --git a/test/construct.test.did b/test/construct.test.did index 59e009b5a..db5f412f8 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -103,6 +103,7 @@ assert blob "DIDL\00\01\7e\00" == "(opt false)" assert blob "DIDL\00\01\7e\01" == "(opt true)" : (opt bool) "opt: parsing false : opt bool"; assert blob "DIDL\00\01\7e\02" !: (opt bool) "opt: parsing invalid bool at opt bool"; assert blob "DIDL\01\6e\7f\01\00\00" == "(null)" : (opt opt null) "opt: parsing (null : opt null) at opt opt null gives null, not opt null"; +assert blob "DIDL\00\01\7e\01" == "(null)" : (Opt) "opt: parsing (true : bool) at fix opt works"; // special opt subtyping assert blob "DIDL\00\01\70" == "(null)" : (opt nat) "reserved <: opt nat"; From 3924e4cd4f39269d35a499ec8eee16fead4f750a Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Tue, 17 Nov 2020 16:07:54 +0100 Subject: [PATCH 18/30] Update tests for opt-via-consituent type rules this updates the test suite as per https://github.com/dfinity/candid/pull/137 and implements them (in a shoddy way) --- rust/candid/src/parser/value.rs | 16 ++++++++++------ rust/candid/src/types/internal.rs | 25 +++++++++++++++++++++---- test/construct.test.did | 11 ++++++----- 3 files changed, 37 insertions(+), 15 deletions(-) diff --git a/rust/candid/src/parser/value.rs b/rust/candid/src/parser/value.rs index 4b4c0268d..b3b184f89 100644 --- a/rust/candid/src/parser/value.rs +++ b/rust/candid/src/parser/value.rs @@ -202,10 +202,14 @@ impl IDLValue { } } // Try consituent type - (v, Type::Opt(ty)) => match v.annotate_type(from_parser, env, ty) { - Ok(v) => IDLValue::Opt(Box::new(v)), - Err(_) => IDLValue::None, - }, + (v, Type::Opt(ty)) if !ty.is_opt(env) && !ty.is_null(env) && !ty.is_reserved(env) => { + match v.annotate_type(from_parser, env, ty) { + Ok(v) => IDLValue::Opt(Box::new(v)), + Err(_) => IDLValue::None, + } + } + // Fallback + (_, Type::Opt(_)) => IDLValue::None, (IDLValue::Vec(vec), Type::Vec(ty)) => { let mut res = Vec::new(); for e in vec.iter() { @@ -220,9 +224,9 @@ impl IDLValue { let mut res = Vec::new(); for Field { id, ty } in fs.iter() { let val = fields.get(&id); - let val = if ty.is_opt() { + let val = if ty.is_opt(env) { val.unwrap_or(&&IDLValue::Null) - } else if ty.is_reserved() { + } else if ty.is_reserved(env) { val.unwrap_or(&&IDLValue::Reserved) } else { val.ok_or_else(|| Error::msg(format!("required field {} not found", id)))? diff --git a/rust/candid/src/types/internal.rs b/rust/candid/src/types/internal.rs index 336be7842..fc3e54154 100644 --- a/rust/candid/src/types/internal.rs +++ b/rust/candid/src/types/internal.rs @@ -206,15 +206,22 @@ impl Type { } } - pub(crate) fn is_opt(&self) -> bool { - match self { + pub(crate) fn is_null(&self, env : &crate::TypeEnv) -> bool { + match unroll1(self, env) { + Type::Null => true, + _ => false, + } + } + + pub(crate) fn is_opt(&self, env : &crate::TypeEnv) -> bool { + match unroll1(self, env) { Type::Opt(_) => true, _ => false, } } - pub(crate) fn is_reserved(&self) -> bool { - match self { + pub(crate) fn is_reserved(&self, env : &crate::TypeEnv) -> bool { + match unroll1(self, env) { Type::Reserved => true, _ => false, } @@ -329,6 +336,16 @@ pub fn is_primitive(t: &Type) -> bool { } } +pub fn unroll1(t: &Type, env : &crate::TypeEnv) -> Type { + use self::Type::*; + // This is only safe after type-checking and ruling out non-productive type recursion + match t { + Knot(ref id) => find_type(id).unwrap(), + Var(id) => unroll1(env.rec_find_type(id).unwrap(), env), + t => (*t).clone() + } +} + pub fn unroll(t: &Type) -> Type { use self::Type::*; match t { diff --git a/test/construct.test.did b/test/construct.test.did index db5f412f8..a9826a1b4 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -99,11 +99,12 @@ assert blob "DIDL\02\6e\01\6e\7e\01\00\01\00" == "(opt null)" assert blob "DIDL\02\6e\01\6e\7e\01\00\01\01\00" == "(opt opt false)" : (opt opt bool) "opt: parsing opt opt false : opt opt bool"; // opt subtype to constituent -assert blob "DIDL\00\01\7e\00" == "(opt false)" : (opt bool) "opt: parsing true : opt bool"; -assert blob "DIDL\00\01\7e\01" == "(opt true)" : (opt bool) "opt: parsing false : opt bool"; -assert blob "DIDL\00\01\7e\02" !: (opt bool) "opt: parsing invalid bool at opt bool"; -assert blob "DIDL\01\6e\7f\01\00\00" == "(null)" : (opt opt null) "opt: parsing (null : opt null) at opt opt null gives null, not opt null"; -assert blob "DIDL\00\01\7e\01" == "(null)" : (Opt) "opt: parsing (true : bool) at fix opt works"; +assert blob "DIDL\00\01\7e\00" == "(opt false)" : (opt bool) "opt: parsing false : opt bool"; +assert blob "DIDL\00\01\7e\01" == "(opt true)" : (opt bool) "opt: parsing true : opt bool"; +assert blob "DIDL\00\01\7e\02" !: (opt bool) "opt: parsing invalid bool at opt bool"; +assert blob "DIDL\01\6e\7f\01\00\00" == "(null)" : (opt opt null) "opt: parsing (null : opt null) at opt opt null gives null, not opt null"; +assert blob "DIDL\00\01\7e\01" == "(null)" : (opt opt bool) "opt: parsing true : opt opt bool gives null"; +assert blob "DIDL\00\01\7e\01" == "(null)" : (opt opt Opt) "opt: parsing (true : bool) at fix opt gives null"; // special opt subtyping assert blob "DIDL\00\01\70" == "(null)" : (opt nat) "reserved <: opt nat"; From 463433129160d16eedf1751ee1fe8c6be6673871 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Tue, 17 Nov 2020 16:10:47 +0100 Subject: [PATCH 19/30] cargo-fmt --- rust/candid/src/types/internal.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rust/candid/src/types/internal.rs b/rust/candid/src/types/internal.rs index fc3e54154..83df8a02b 100644 --- a/rust/candid/src/types/internal.rs +++ b/rust/candid/src/types/internal.rs @@ -206,21 +206,21 @@ impl Type { } } - pub(crate) fn is_null(&self, env : &crate::TypeEnv) -> bool { + pub(crate) fn is_null(&self, env: &crate::TypeEnv) -> bool { match unroll1(self, env) { Type::Null => true, _ => false, } } - pub(crate) fn is_opt(&self, env : &crate::TypeEnv) -> bool { + pub(crate) fn is_opt(&self, env: &crate::TypeEnv) -> bool { match unroll1(self, env) { Type::Opt(_) => true, _ => false, } } - pub(crate) fn is_reserved(&self, env : &crate::TypeEnv) -> bool { + pub(crate) fn is_reserved(&self, env: &crate::TypeEnv) -> bool { match unroll1(self, env) { Type::Reserved => true, _ => false, @@ -336,13 +336,13 @@ pub fn is_primitive(t: &Type) -> bool { } } -pub fn unroll1(t: &Type, env : &crate::TypeEnv) -> Type { +pub fn unroll1(t: &Type, env: &crate::TypeEnv) -> Type { use self::Type::*; // This is only safe after type-checking and ruling out non-productive type recursion match t { Knot(ref id) => find_type(id).unwrap(), Var(id) => unroll1(env.rec_find_type(id).unwrap(), env), - t => (*t).clone() + t => (*t).clone(), } } From 55f0e8c250d9ff54051102344a1a1e0f20e343a6 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Tue, 17 Nov 2020 18:25:22 +0100 Subject: [PATCH 20/30] More tests about variants --- test/construct.test.did | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/construct.test.did b/test/construct.test.did index a9826a1b4..4795ccf84 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -132,6 +132,8 @@ assert blob "DIDL\01\6b\01\00\7f\01\00\01" ! assert blob "DIDL\01\6b\02\00\7f\00\7f\01\00\00" !: (variant {0}) "variant: duplicate fields"; assert blob "DIDL\01\6b\03\00\7f\01\7f\01\7f\01\00\00" !: (variant {0}) "variant: duplicate fields"; assert blob "DIDL\01\6b\02\01\7f\00\7f\01\00\00" !: (variant {0;1}) "variant: unsorted"; +assert blob "DIDL\01\6b\02\00\7f\01\7f\ff\ff\ff\ff\0F\7f\01\00\00" : (variant {0;1;4294967295}) "variant: ok with maxInt"; +assert blob "DIDL\01\6b\03\00\7f\ff\ff\ff\ff\0F\7f\01\7f\01\00\00" !: (variant {0;1;4294967295}) "variant: unsorted with maxInt"; assert blob "DIDL\01\6b\02\b3\d3\c9\01\7f\e6\fd\d5\01\7f\01\00\00" == "(variant { Bar })" : (variant { Foo; Bar; }) "variant: enum"; assert blob "DIDL\01\6b\01\cd\84\b0\05\7f\01\00\00" == "(variant { \"☃\" })" : (variant { "☃" }) "variant: unicode field"; assert blob "DIDL\01\6b\02\bc\8a\01\71\c5\fe\d2\01\71\01\00\00\04\67\6f\6f\64" From eb1547d7e987fea5250be5997dd14f4ea36253ac Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Wed, 18 Nov 2020 10:28:04 +0100 Subject: [PATCH 21/30] fix maxint tests --- test/construct.test.did | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/construct.test.did b/test/construct.test.did index 4795ccf84..2cd0a1386 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -132,8 +132,8 @@ assert blob "DIDL\01\6b\01\00\7f\01\00\01" ! assert blob "DIDL\01\6b\02\00\7f\00\7f\01\00\00" !: (variant {0}) "variant: duplicate fields"; assert blob "DIDL\01\6b\03\00\7f\01\7f\01\7f\01\00\00" !: (variant {0}) "variant: duplicate fields"; assert blob "DIDL\01\6b\02\01\7f\00\7f\01\00\00" !: (variant {0;1}) "variant: unsorted"; -assert blob "DIDL\01\6b\02\00\7f\01\7f\ff\ff\ff\ff\0F\7f\01\00\00" : (variant {0;1;4294967295}) "variant: ok with maxInt"; -assert blob "DIDL\01\6b\03\00\7f\ff\ff\ff\ff\0F\7f\01\7f\01\00\00" !: (variant {0;1;4294967295}) "variant: unsorted with maxInt"; +assert blob "DIDL\01\6b\03\00\7f\01\7f\ff\ff\ff\ff\0f\7f\01\00\00" : (variant {0;1;4294967295}) "variant: ok with maxInt"; +assert blob "DIDL\01\6b\03\00\7f\ff\ff\ff\ff\0f\7f\01\7f\01\00\00" !: (variant {0;1;4294967295}) "variant: unsorted with maxInt"; assert blob "DIDL\01\6b\02\b3\d3\c9\01\7f\e6\fd\d5\01\7f\01\00\00" == "(variant { Bar })" : (variant { Foo; Bar; }) "variant: enum"; assert blob "DIDL\01\6b\01\cd\84\b0\05\7f\01\00\00" == "(variant { \"☃\" })" : (variant { "☃" }) "variant: unicode field"; assert blob "DIDL\01\6b\02\bc\8a\01\71\c5\fe\d2\01\71\01\00\00\04\67\6f\6f\64" From f037f1d6210e399ce0ee1d2af3463ed71a94001f Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Thu, 19 Nov 2020 12:17:42 +0100 Subject: [PATCH 22/30] Fun test case with `fix record <: fix (opt . record)` --- test/construct.test.did | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/construct.test.did b/test/construct.test.did index 2cd0a1386..f5062f2dd 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -6,6 +6,8 @@ Encoding tests for construct types type Opt = opt Opt; type Vec = vec Vec; type EmptyRecord = record { EmptyRecord }; +type OptRec = opt record { OptRec }; + type List = opt record { head : int; tail : List }; type List1 = opt List2; type List2 = record { head : int; tail : List1 }; @@ -104,7 +106,8 @@ assert blob "DIDL\00\01\7e\01" == "(opt true)" : (opt bool) "opt: parsing tru assert blob "DIDL\00\01\7e\02" !: (opt bool) "opt: parsing invalid bool at opt bool"; assert blob "DIDL\01\6e\7f\01\00\00" == "(null)" : (opt opt null) "opt: parsing (null : opt null) at opt opt null gives null, not opt null"; assert blob "DIDL\00\01\7e\01" == "(null)" : (opt opt bool) "opt: parsing true : opt opt bool gives null"; -assert blob "DIDL\00\01\7e\01" == "(null)" : (opt opt Opt) "opt: parsing (true : bool) at fix opt gives null"; +assert blob "DIDL\00\01\7e\01" == "(null)" : (Opt) "opt: parsing (true : bool) at \"fix opt\" gives null"; +assert blob "DIDL\01\6c\01\00\00\01\00" == "(null)" : (OptRec) "opt: parsing \"fix record\" at \"fix record opt\" gives null"; // special opt subtyping assert blob "DIDL\00\01\70" == "(null)" : (opt nat) "reserved <: opt nat"; From dc5deec25c586a4fab97b54487ceaa425dcc283f Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Thu, 19 Nov 2020 12:22:14 +0100 Subject: [PATCH 23/30] =?UTF-8?q?It=20shouldn=E2=80=99t=20actually=20decod?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/construct.test.did | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/construct.test.did b/test/construct.test.did index f5062f2dd..e7e85930e 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -107,7 +107,7 @@ assert blob "DIDL\00\01\7e\02" !: (opt bool) "opt: parsing inv assert blob "DIDL\01\6e\7f\01\00\00" == "(null)" : (opt opt null) "opt: parsing (null : opt null) at opt opt null gives null, not opt null"; assert blob "DIDL\00\01\7e\01" == "(null)" : (opt opt bool) "opt: parsing true : opt opt bool gives null"; assert blob "DIDL\00\01\7e\01" == "(null)" : (Opt) "opt: parsing (true : bool) at \"fix opt\" gives null"; -assert blob "DIDL\01\6c\01\00\00\01\00" == "(null)" : (OptRec) "opt: parsing \"fix record\" at \"fix record opt\" gives null"; +assert blob "DIDL\01\6c\01\00\00\01\00" !: (OptRec) "opt: parsing \"fix record\" at \"fix record opt\" fails"; // special opt subtyping assert blob "DIDL\00\01\70" == "(null)" : (opt nat) "reserved <: opt nat"; From 007e7bd7810d733a7185412f07e56c800eaab042 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Thu, 19 Nov 2020 16:10:34 +0100 Subject: [PATCH 24/30] Run cargo fix --- rust/candid/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/candid/src/lib.rs b/rust/candid/src/lib.rs index d9df4e0d0..c22779d0c 100644 --- a/rust/candid/src/lib.rs +++ b/rust/candid/src/lib.rs @@ -288,7 +288,7 @@ macro_rules! Encode { Encode!(@PutValue builder $($x,)*) }}; ( @PutValue $builder:ident $x:expr, $($tail:expr,)* ) => {{ - $builder.arg($x).and_then(|mut builder| Encode!(@PutValue builder $($tail,)*)) + $builder.arg($x).and_then(|builder| Encode!(@PutValue builder $($tail,)*)) }}; ( @PutValue $builder:ident ) => {{ $builder.serialize_to_vec() From e6492aba76a6892fc9d63942c217919cd69517c6 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Thu, 19 Nov 2020 16:12:10 +0100 Subject: [PATCH 25/30] Fix cargo clippy --- rust/candid/src/types/internal.rs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/rust/candid/src/types/internal.rs b/rust/candid/src/types/internal.rs index 83df8a02b..41b81de4b 100644 --- a/rust/candid/src/types/internal.rs +++ b/rust/candid/src/types/internal.rs @@ -207,24 +207,15 @@ impl Type { } pub(crate) fn is_null(&self, env: &crate::TypeEnv) -> bool { - match unroll1(self, env) { - Type::Null => true, - _ => false, - } + matches!(unroll1(self, env), Type::Null) } pub(crate) fn is_opt(&self, env: &crate::TypeEnv) -> bool { - match unroll1(self, env) { - Type::Opt(_) => true, - _ => false, - } + matches!(unroll1(self, env), Type::Opt(_)) } pub(crate) fn is_reserved(&self, env: &crate::TypeEnv) -> bool { - match unroll1(self, env) { - Type::Reserved => true, - _ => false, - } + matches!(unroll1(self, env), Type::Reserved) } } impl fmt::Display for Type { From 5e2813acec8d6f294d123b1819b1efbd94642e1e Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Thu, 19 Nov 2020 16:23:48 +0100 Subject: [PATCH 26/30] More clippy --- rust/candid_derive/src/func.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/candid_derive/src/func.rs b/rust/candid_derive/src/func.rs index 201d0f007..70d267097 100644 --- a/rust/candid_derive/src/func.rs +++ b/rust/candid_derive/src/func.rs @@ -31,7 +31,7 @@ pub(crate) fn candid_method(attrs: AttributeArgs, fun: ItemFn) -> Result = args From 1d841b94a1e8dd2ea24b91cda1eed620427f1d76 Mon Sep 17 00:00:00 2001 From: chenyan-dfinity Date: Thu, 19 Nov 2020 22:48:06 -0800 Subject: [PATCH 27/30] refactor --- rust/candid/src/parser/typing.rs | 7 +++++++ rust/candid/src/parser/value.rs | 21 +++++++++++---------- rust/candid/src/types/internal.rs | 24 +----------------------- 3 files changed, 19 insertions(+), 33 deletions(-) diff --git a/rust/candid/src/parser/typing.rs b/rust/candid/src/parser/typing.rs index 1beb0c9d5..82165ba67 100644 --- a/rust/candid/src/parser/typing.rs +++ b/rust/candid/src/parser/typing.rs @@ -44,6 +44,13 @@ impl TypeEnv { t => Ok(t), } } + pub fn trace_type<'a>(&'a self, t: &'a Type) -> Result { + match t { + Type::Var(id) => self.trace_type(self.find_type(id)?), + Type::Knot(ref id) => self.trace_type(&crate::types::internal::find_type(id).unwrap()), + t => Ok(t.clone()), + } + } pub fn as_func<'a>(&'a self, t: &'a Type) -> Result<&'a Function> { match t { Type::Func(f) => Ok(f), diff --git a/rust/candid/src/parser/value.rs b/rust/candid/src/parser/value.rs index b3b184f89..246e3c715 100644 --- a/rust/candid/src/parser/value.rs +++ b/rust/candid/src/parser/value.rs @@ -190,7 +190,7 @@ impl IDLValue { (IDLValue::Float64(n), Type::Float64) => IDLValue::Float64(*n), (IDLValue::Float32(n), Type::Float32) => IDLValue::Float32(*n), (IDLValue::Text(s), Type::Text) => IDLValue::Text(s.to_owned()), - // opt parsing. NB: Always succeds! + // opt parsing. NB: Always succeeds! (IDLValue::Null, Type::Opt(_)) => IDLValue::None, (IDLValue::Reserved, Type::Opt(_)) => IDLValue::None, (IDLValue::None, Type::Opt(_)) => IDLValue::None, @@ -202,7 +202,7 @@ impl IDLValue { } } // Try consituent type - (v, Type::Opt(ty)) if !ty.is_opt(env) && !ty.is_null(env) && !ty.is_reserved(env) => { + (v, Type::Opt(ty)) if !matches!(env.trace_type(ty)?, Type::Null|Type::Reserved|Type::Opt(_)) => { match v.annotate_type(from_parser, env, ty) { Ok(v) => IDLValue::Opt(Box::new(v)), Err(_) => IDLValue::None, @@ -223,14 +223,15 @@ impl IDLValue { vec.iter().map(|IDLField { id, val }| (id, val)).collect(); let mut res = Vec::new(); for Field { id, ty } in fs.iter() { - let val = fields.get(&id); - let val = if ty.is_opt(env) { - val.unwrap_or(&&IDLValue::Null) - } else if ty.is_reserved(env) { - val.unwrap_or(&&IDLValue::Reserved) - } else { - val.ok_or_else(|| Error::msg(format!("required field {} not found", id)))? - }; + let val = fields + .get(&id) + .cloned() + .or_else(|| match env.trace_type(ty) { + Ok(Type::Opt(_)) => Some(&IDLValue::Null), + Ok(Type::Reserved) => Some(&IDLValue::Reserved), + _ => None, + }) + .ok_or_else(|| Error::msg(format!("required field {} not found", id)))?; let val = val.annotate_type(from_parser, env, ty)?; res.push(IDLField { id: id.clone(), diff --git a/rust/candid/src/types/internal.rs b/rust/candid/src/types/internal.rs index 41b81de4b..ae4445ece 100644 --- a/rust/candid/src/types/internal.rs +++ b/rust/candid/src/types/internal.rs @@ -205,18 +205,6 @@ impl Type { _ => false, } } - - pub(crate) fn is_null(&self, env: &crate::TypeEnv) -> bool { - matches!(unroll1(self, env), Type::Null) - } - - pub(crate) fn is_opt(&self, env: &crate::TypeEnv) -> bool { - matches!(unroll1(self, env), Type::Opt(_)) - } - - pub(crate) fn is_reserved(&self, env: &crate::TypeEnv) -> bool { - matches!(unroll1(self, env), Type::Reserved) - } } impl fmt::Display for Type { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -327,16 +315,6 @@ pub fn is_primitive(t: &Type) -> bool { } } -pub fn unroll1(t: &Type, env: &crate::TypeEnv) -> Type { - use self::Type::*; - // This is only safe after type-checking and ruling out non-productive type recursion - match t { - Knot(ref id) => find_type(id).unwrap(), - Var(id) => unroll1(env.rec_find_type(id).unwrap(), env), - t => (*t).clone(), - } -} - pub fn unroll(t: &Type) -> Type { use self::Type::*; match t { @@ -373,7 +351,7 @@ thread_local! { pub(crate) fn find_type(id: &TypeId) -> Option { ENV.with(|e| match e.borrow().get(id) { None => None, - Some(t) => Some((*t).clone()), + Some(t) => Some(t.clone()), }) } From a5a7f5ad190d8ee20229f4a6366b52276935f161 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Fri, 20 Nov 2020 15:33:44 +0100 Subject: [PATCH 28/30] More tests (this one showed a bug in Motoko) --- test/construct.test.did | 1 + 1 file changed, 1 insertion(+) diff --git a/test/construct.test.did b/test/construct.test.did index e7e85930e..79f2528db 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -116,6 +116,7 @@ assert blob "DIDL\01\6e\7e\01\00\01\01" == "(null)" : (opt nat) "opt true : opt assert blob "DIDL\01\6e\7e\01\00\01\02" !: (opt nat) "opt bool <: opt nat with invalid boolean value"; assert blob "DIDL\02\6e\01\6e\7e\01\00\01\01\01" == "(null)" : (opt nat) "opt opt true : opt opt bool <: opt nat"; assert blob "DIDL\02\6e\01\6e\7e\01\00\01\01\01" == "(opt null)" : (opt opt nat) "opt opt true : opt opt bool <: opt opt nat"; +assert blob "DIDL\02\6e\01\6b\01\00\7e\01\00\01\00\01" == "(null)" : (opt variant { 0 : int }) "opt: recovered coercion error under variant"; // special record field rules assert blob "DIDL\01\6c\00\01\00" == "(record { foo = null })" : (record { foo : opt bool }) "missing optional field"; From a259759425bb886d163354436b9fa70b4e0907e1 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Fri, 20 Nov 2020 19:25:12 +0100 Subject: [PATCH 29/30] Fix escaping of escapes --- test/construct.test.did | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/construct.test.did b/test/construct.test.did index 79f2528db..a517a7ea2 100644 --- a/test/construct.test.did +++ b/test/construct.test.did @@ -49,7 +49,7 @@ assert blob "DIDL\02\6e\01\6e\00\01\00\01\01\00" == "(opt opt null)" : (Opt) "op // vector assert blob "DIDL\01\6d\7c\01\00\00" == "(vec {})" : (vec int) "vec: empty"; assert blob "DIDL\01\6d\7c\01\00\02\01\02" == "(vec { 1; 2 })" : (vec int) "vec"; -assert blob "DIDL\01\6d\7b\01\00\02\01\02" == "(blob \"\01\02\")" : (vec nat8) "vec: blob"; +assert blob "DIDL\01\6d\7b\01\00\02\01\02" == "(blob \"\\01\\02\")" : (vec nat8) "vec: blob"; assert blob "DIDL\01\6d\00\01\00\00" == "(vec {})" : (Vec) "vec: recursive vector"; assert blob "DIDL\01\6d\00\01\00\02\00\00" == "(vec { vec {}; vec {} })" : (Vec) "vec: tree"; assert blob "DIDL\01\6d\00\01\00\02\00\00" == "(vec { vec {}; vec {} })" : (vec vec empty) "vec: non-recursive tree"; From 5337a14590fce307bd08d3831375aa5a2dcb86d2 Mon Sep 17 00:00:00 2001 From: chenyan-dfinity Date: Fri, 20 Nov 2020 10:33:16 -0800 Subject: [PATCH 30/30] fix --- rust/candid/src/parser/value.rs | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/rust/candid/src/parser/value.rs b/rust/candid/src/parser/value.rs index 246e3c715..605a41d9d 100644 --- a/rust/candid/src/parser/value.rs +++ b/rust/candid/src/parser/value.rs @@ -194,21 +194,18 @@ impl IDLValue { (IDLValue::Null, Type::Opt(_)) => IDLValue::None, (IDLValue::Reserved, Type::Opt(_)) => IDLValue::None, (IDLValue::None, Type::Opt(_)) => IDLValue::None, - (IDLValue::Opt(v), Type::Opt(ty)) => { - // liberal decoding of optionals - match v.annotate_type(from_parser, env, ty) { - Ok(v) => IDLValue::Opt(Box::new(v)), - Err(_) => IDLValue::None, - } - } - // Try consituent type + // liberal decoding of optionals + (IDLValue::Opt(v), Type::Opt(ty)) => v + .annotate_type(from_parser, env, ty) + .map(|v| IDLValue::Opt(Box::new(v))) + .unwrap_or(IDLValue::None), + // try consituent type (v, Type::Opt(ty)) if !matches!(env.trace_type(ty)?, Type::Null|Type::Reserved|Type::Opt(_)) => { - match v.annotate_type(from_parser, env, ty) { - Ok(v) => IDLValue::Opt(Box::new(v)), - Err(_) => IDLValue::None, - } + v.annotate_type(from_parser, env, ty) + .map(|v| IDLValue::Opt(Box::new(v))) + .unwrap_or(IDLValue::None) } - // Fallback + // fallback (_, Type::Opt(_)) => IDLValue::None, (IDLValue::Vec(vec), Type::Vec(ty)) => { let mut res = Vec::new(); @@ -226,9 +223,9 @@ impl IDLValue { let val = fields .get(&id) .cloned() - .or_else(|| match env.trace_type(ty) { - Ok(Type::Opt(_)) => Some(&IDLValue::Null), - Ok(Type::Reserved) => Some(&IDLValue::Reserved), + .or_else(|| match env.trace_type(ty).unwrap() { + Type::Opt(_) => Some(&IDLValue::None), + Type::Reserved => Some(&IDLValue::Reserved), _ => None, }) .ok_or_else(|| Error::msg(format!("required field {} not found", id)))?;