Skip to content

Commit

Permalink
fix: unroll types for all types (#223)
Browse files Browse the repository at this point in the history
* fix: unroll types for all types

* fix equality for IDLValue::Variant

* fix

* test

* debug print for values

* display types for numbers

* fix

* didc hash

* didc decode blob

* changelog
  • Loading branch information
chenyan-dfinity authored Apr 30, 2021
1 parent 6f3014d commit a0cc856
Show file tree
Hide file tree
Showing 13 changed files with 167 additions and 111 deletions.
5 changes: 4 additions & 1 deletion Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,19 @@
* `de::ArgumentDecoder`, `ser::ArgumentEncoder` moved to `utils::{ArgumentDecoder, ArgumentEncoder}`
* `types::subtype` returns `Result<()>` instead of `bool` for better error message
* Disable subtyping conversion for opt rules in `IDLValue.annotate_type`
* Display type annotations for number types

### Non-breaking changes

* Better error messages in deserialization
* Update test suite to conform with the new spec
* `didc hash` to compute hash of a field name
* `didc decode` can decode blob format

### Pending issues

* Update opt rule for subtyping and coercion
* Performance regression in decoding struct type
* Performance regression in decoding struct type, likely due to `Type` clone
* Integration test with production canisters

## 2021-04-07 (Rust 0.6.19 -- 0.6.21)
Expand Down
2 changes: 1 addition & 1 deletion rust/candid/src/bindings/javascript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ pub mod value {
enclose_space("{", pp_fields(&fields), "}")
}
}
Variant(v, _) => enclose_space("{", pp_field(&v), "}"),
Variant(v) => enclose_space("{", pp_field(&v.0), "}"),
_ => RcDoc::as_string(v),
}
}
Expand Down
50 changes: 25 additions & 25 deletions rust/candid/src/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,12 @@ impl<'de> Deserializer<'de> {
Ok(())
}
fn unroll_type(&mut self) -> Result<()> {
self.expect_type = self.table.trace_type(&self.expect_type)?;
self.wire_type = self.table.trace_type(&self.wire_type)?;
if matches!(self.expect_type, Type::Var(_) | Type::Knot(_)) {
self.expect_type = self.table.trace_type(&self.expect_type)?;
}
if matches!(self.wire_type, Type::Var(_) | Type::Knot(_)) {
self.wire_type = self.table.trace_type(&self.wire_type)?;
}
Ok(())
}
// Should always call set_field_name to set the field_name. After deserialize_identifier
Expand All @@ -227,6 +231,7 @@ impl<'de> Deserializer<'de> {
V: Visitor<'de>,
{
use std::convert::TryInto;
self.unroll_type()?;
assert!(self.expect_type == Type::Int);
let mut bytes = vec![0u8];
let int = match &self.wire_type {
Expand All @@ -246,6 +251,7 @@ impl<'de> Deserializer<'de> {
where
V: Visitor<'de>,
{
self.unroll_type()?;
assert!(self.expect_type == Type::Nat && self.wire_type == Type::Nat);
let mut bytes = vec![1u8];
let nat = Nat::decode(&mut self.input).map_err(Error::msg)?;
Expand All @@ -256,6 +262,7 @@ impl<'de> Deserializer<'de> {
where
V: Visitor<'de>,
{
self.unroll_type()?;
assert!(self.expect_type == Type::Principal && self.wire_type == Type::Principal);
let mut bytes = vec![2u8];
let id = PrincipalBytes::read(&mut self.input)?.inner;
Expand Down Expand Up @@ -312,6 +319,7 @@ macro_rules! primitive_impl {
paste::item! {
fn [<deserialize_ $ty>]<V>(self, visitor: V) -> Result<V::Value>
where V: Visitor<'de> {
self.unroll_type()?;
assert!(self.expect_type == $type && self.wire_type == $type);
let val = self.input.$($value)*().map_err(|_| Error::msg(format!("Cannot read {} value", stringify!($type))))?;
//let val: $ty = self.input.read_le()?;
Expand All @@ -330,8 +338,8 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
if self.field_name.is_some() {
return self.deserialize_identifier(visitor);
}
let t = self.table.trace_type(&self.expect_type)?;
match t {
self.unroll_type()?;
match &self.expect_type {
Type::Int => self.deserialize_int(visitor),
Type::Nat => self.deserialize_nat(visitor),
Type::Nat8 => self.deserialize_u8(visitor),
Expand Down Expand Up @@ -389,6 +397,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
V: Visitor<'de>,
{
use std::convert::TryInto;
self.unroll_type()?;
assert!(self.expect_type == Type::Int);
let value: i128 = match &self.wire_type {
Type::Int => {
Expand All @@ -408,6 +417,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
V: Visitor<'de>,
{
use std::convert::TryInto;
self.unroll_type()?;
assert!(self.expect_type == Type::Nat && self.wire_type == Type::Nat);
let nat = Nat::decode(&mut self.input).map_err(Error::msg)?;
let value: u128 = nat.0.try_into().map_err(Error::msg)?;
Expand All @@ -417,13 +427,15 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
where
V: Visitor<'de>,
{
self.unroll_type()?;
assert!(self.expect_type == Type::Null && self.wire_type == Type::Null);
visitor.visit_unit()
}
fn deserialize_bool<V>(self, visitor: V) -> Result<V::Value>
where
V: Visitor<'de>,
{
self.unroll_type()?;
assert!(self.expect_type == Type::Bool && self.wire_type == Type::Bool);
let res = BoolValue::read(&mut self.input)?;
visitor.visit_bool(res.0)
Expand All @@ -432,6 +444,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
where
V: Visitor<'de>,
{
self.unroll_type()?;
assert!(self.expect_type == Type::Text && self.wire_type == Type::Text);
let len = Len::read(&mut self.input)?.0;
let bytes = self.borrow_bytes(len)?.to_owned();
Expand All @@ -442,6 +455,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
where
V: Visitor<'de>,
{
self.unroll_type()?;
assert!(self.expect_type == Type::Text && self.wire_type == Type::Text);
let len = Len::read(&mut self.input)?.0;
let slice = self.borrow_bytes(len)?;
Expand Down Expand Up @@ -654,21 +668,12 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
)));
}
let wire = w[index].clone();
let e_tuple = e
let expect = e
.iter()
.enumerate()
.find(|(_, ref f)| f.id == wire.id)
.ok_or_else(|| Error::msg(format!("Unknown variant field {}", wire.id)))?;
let expect_index = e_tuple.0;
let expect = e_tuple.1.clone();
visitor.visit_enum(Compound::new(
&mut self,
Style::Enum {
expect,
expect_index,
wire,
},
))
.find(|ref f| f.id == wire.id)
.ok_or_else(|| Error::msg(format!("Unknown variant field {}", wire.id)))?
.clone();
visitor.visit_enum(Compound::new(&mut self, Style::Enum { expect, wire }))
}
_ => assert!(false),
}
Expand Down Expand Up @@ -702,7 +707,6 @@ enum Style {
},
Enum {
expect: Field,
expect_index: usize,
wire: Field,
},
Map {
Expand Down Expand Up @@ -848,11 +852,7 @@ impl<'de, 'a> de::EnumAccess<'de> for Compound<'a, 'de> {
V: de::DeserializeSeed<'de>,
{
match &self.style {
Style::Enum {
expect,
expect_index,
wire,
} => {
Style::Enum { expect, wire } => {
self.de.expect_type = expect.ty.clone();
self.de.wire_type = wire.ty.clone();
let (mut label, label_type) = match &expect.id {
Expand All @@ -865,7 +865,7 @@ impl<'de, 'a> de::EnumAccess<'de> for Compound<'a, 'de> {
Type::Record(_) => "struct",
_ => "newtype",
};
label += &format!(",{},{},{}", label_type, accessor, expect_index);
label += &format!(",{},{}", label_type, accessor);
}
self.de.set_field_name(Label::Named(label));
let field = seed.deserialize(&mut *self.de)?;
Expand Down
5 changes: 3 additions & 2 deletions rust/candid/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@
//! We also provide a parser to parse Candid values in text format.
//!
//! ```
//! use candid::IDLArgs;
//! use candid::{IDLArgs, TypeEnv};
//! // Candid values represented in text format
//! let text_value = r#"
//! (42, opt true, vec {1;2;3},
Expand All @@ -174,7 +174,8 @@
//! // Convert IDLArgs to text format
//! let output: String = decoded.to_string();
//! let parsed_args: IDLArgs = output.parse()?;
//! assert_eq!(args, parsed_args);
//! let annotated_args = args.annotate_types(true, &TypeEnv::new(), &parsed_args.get_types())?;
//! assert_eq!(annotated_args, parsed_args);
//! # Ok::<(), candid::Error>(())
//! ```
//! Note that when parsing Candid values, we assume the number literals are always of type `Int`.
Expand Down
4 changes: 2 additions & 2 deletions rust/candid/src/parser/grammar.lalrpop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::value::{IDLField, IDLValue, IDLArgs};
use super::value::{IDLField, IDLValue, IDLArgs, VariantValue};
use super::typing::{check_unique, TypeEnv};
use super::types::{IDLType, PrimType, TypeField, FuncType, FuncMode, Binding, Dec, IDLProg, IDLTypes};
use super::test::{Assert, Input, Test};
Expand Down Expand Up @@ -81,7 +81,7 @@ pub Arg: IDLValue = {
check_unique(fs.iter().map(|f| &f.id)).map_err(|e| error2(e, span))?;
Ok(IDLValue::Record(fs))
},
"variant" "{" <VariantField> "}" => IDLValue::Variant(Box::new(<>), 0),
"variant" "{" <VariantField> "}" => IDLValue::Variant(VariantValue(Box::new(<>), 0)),
"principal" <Sp<Text>> =>? Ok(IDLValue::Principal(Principal::from_text(&<>.0).map_err(|e| error2(e, <>.1))?)),
"service" <Sp<Text>> =>? Ok(IDLValue::Service(Principal::from_text(&<>.0).map_err(|e| error2(e, <>.1))?)),
"func" <id:Sp<Text>> "." <meth:Name> =>? {
Expand Down
64 changes: 44 additions & 20 deletions rust/candid/src/parser/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,28 +38,48 @@ impl fmt::Debug for IDLArgs {
}
}
}
fn has_type_annotation(v: &IDLValue) -> bool {
use IDLValue::*;
matches!(
v,
Int(_)
| Nat(_)
| Nat8(_)
| Nat16(_)
| Nat32(_)
| Nat64(_)
| Int8(_)
| Int16(_)
| Int32(_)
| Int64(_)
| Float32(_)
| Float64(_)
| Null
| Reserved
)
}
impl fmt::Debug for IDLValue {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
use IDLValue::*;
match self {
Null => write!(f, "null"),
Null => write!(f, "null : null"),
Bool(b) => write!(f, "{}", b),
Number(n) => write!(f, "{}", n),
Int(i) => write!(f, "{}", i),
Nat(n) => write!(f, "{}", n),
Nat8(n) => write!(f, "{}", n),
Nat16(n) => write!(f, "{}", pp_num_str(&n.to_string())),
Nat32(n) => write!(f, "{}", pp_num_str(&n.to_string())),
Nat64(n) => write!(f, "{}", pp_num_str(&n.to_string())),
Int8(n) => write!(f, "{}", n),
Int16(n) => write!(f, "{}", pp_num_str(&n.to_string())),
Int32(n) => write!(f, "{}", pp_num_str(&n.to_string())),
Int64(n) => write!(f, "{}", pp_num_str(&n.to_string())),
Float32(n) => write!(f, "{}", n),
Float64(n) => write!(f, "{}", n),
Int(i) => write!(f, "{} : int", i),
Nat(n) => write!(f, "{} : nat", n),
Nat8(n) => write!(f, "{} : nat8", n),
Nat16(n) => write!(f, "{} : nat16", pp_num_str(&n.to_string())),
Nat32(n) => write!(f, "{} : nat32", pp_num_str(&n.to_string())),
Nat64(n) => write!(f, "{} : nat64", pp_num_str(&n.to_string())),
Int8(n) => write!(f, "{} : int8", n),
Int16(n) => write!(f, "{} : int16", pp_num_str(&n.to_string())),
Int32(n) => write!(f, "{} : int32", pp_num_str(&n.to_string())),
Int64(n) => write!(f, "{} : int64", pp_num_str(&n.to_string())),
Float32(n) => write!(f, "{} : float32", n),
Float64(n) => write!(f, "{} : float64", n),
Text(s) => write!(f, "{:?}", s),
None => write!(f, "null"),
Reserved => write!(f, "reserved"),
Reserved => write!(f, "null : reserved"),
Principal(id) => write!(f, "principal \"{}\"", id),
Service(id) => write!(f, "service \"{}\"", id),
Func(id, meth) => write!(
Expand All @@ -68,6 +88,7 @@ impl fmt::Debug for IDLValue {
id,
crate::bindings::candid::ident_string(meth)
),
Opt(v) if has_type_annotation(v) => write!(f, "opt ({:?})", v),
Opt(v) => write!(f, "opt {:?}", v),
Vec(vs) => {
if let Some(Nat8(_)) = vs.first() {
Expand Down Expand Up @@ -98,12 +119,12 @@ impl fmt::Debug for IDLValue {
}
write!(f, "}}")
}
Variant(v, _) => {
Variant(v) => {
write!(f, "variant {{ ")?;
if v.val == Null {
write!(f, "{}", v.id)?;
if v.0.val == Null {
write!(f, "{}", v.0.id)?;
} else {
write!(f, "{:?}", v)?;
write!(f, "{:?}", v.0)?;
}
write!(f, " }}")
}
Expand Down Expand Up @@ -146,7 +167,7 @@ fn pp_fields(depth: usize, fields: &[IDLField]) -> RcDoc {
}

pub fn pp_char(v: u8) -> String {
if (0x20..=0x7e).contains(&v) && v != 0x22 && v != 0x5c {
if (0x20..=0x7e).contains(&v) && v != 0x22 && v != 0x27 && v != 0x60 && v != 0x5c {
std::char::from_u32(v as u32).unwrap().to_string()
} else {
format!("\\{:02x}", v)
Expand All @@ -160,6 +181,9 @@ pub fn pp_value(depth: usize, v: &IDLValue) -> RcDoc {
}
match v {
Text(ref s) => RcDoc::as_string(format!("\"{}\"", s)),
Opt(v) if has_type_annotation(v) => {
kwd("opt").append(enclose("(", pp_value(depth - 1, v), ")"))
}
Opt(v) => kwd("opt").append(pp_value(depth - 1, v)),
Vec(vs) => {
if let Some(Nat8(_)) = vs.first() {
Expand All @@ -179,7 +203,7 @@ pub fn pp_value(depth: usize, v: &IDLValue) -> RcDoc {
kwd("record").append(pp_fields(depth, &fields))
}
}
Variant(v, _) => kwd("variant").append(enclose_space("{", pp_field(depth, &v, true), "}")),
Variant(v) => kwd("variant").append(enclose_space("{", pp_field(depth, &v.0, true), "}")),
_ => RcDoc::as_string(format!("{:?}", v)),
}
}
Expand Down
4 changes: 2 additions & 2 deletions rust/candid/src/parser/random.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::configs::{path_name, Configs};
use super::typing::TypeEnv;
use super::value::{IDLArgs, IDLField, IDLValue};
use super::value::{IDLArgs, IDLField, IDLValue, VariantValue};
use crate::types::{Field, Type};
use crate::Deserialize;
use crate::{Error, Result};
Expand Down Expand Up @@ -201,7 +201,7 @@ impl<'a> GenState<'a> {
id: id.clone(),
val,
};
IDLValue::Variant(Box::new(field), idx as u64)
IDLValue::Variant(VariantValue(Box::new(field), idx as u64))
}
_ => unimplemented!(),
});
Expand Down
Loading

0 comments on commit a0cc856

Please sign in to comment.