Skip to content

Commit

Permalink
.default support
Browse files Browse the repository at this point in the history
Adds .default support to typenames (`foo = bar .default baz`) and to map
fields (`foo = { ? 1 => bar .default baz }`).

This is not applied into nested contexts e.g.:
```
foo = uint .default 2
bar = { * str => foo }
```
as how this should be handled is extremely ambiguous.
Should this result in an extra encoding variable which is a map e.g.
`default_included: BTreeSet<K>` and having the behavior be to omit it if
it's not included in this encoding var and it's equal to the default ?

As per the CDDL RFC we only apply it to optional fields:
```
This control is
only meaningful when the control type is used in an optional context;
otherwise, there would be no way to make use of the default value.
```

We support encoding preservation for round-tripping i.e. if a default
value is explicitly present it will remain explicitly present. With
`preserve-encodings=false` it will be omitted if the value present is
equal to the default value.

No changes were done with canonical encodings as the RFC doesn't seem to
mention how that should be handled as canonical encodings are specified
in the CBOR RFC not the CDDL one, and `.default` is only present in the
CDDL one. It's that possible we could omit fields equal to the default
if needed but we need to confirm this first.

Optional fields with `.default` are no longer encoded as `Option<T>` but
instead just `T` now which should help with usability. The field will
default to the default via `new()` and deserialization.

Fixes #42
  • Loading branch information
rooooooooob committed Nov 1, 2022
1 parent cff2577 commit a995bad
Show file tree
Hide file tree
Showing 7 changed files with 261 additions and 76 deletions.
140 changes: 95 additions & 45 deletions src/generation.rs

Large diffs are not rendered by default.

91 changes: 66 additions & 25 deletions src/intermediate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,27 @@ impl FixedValue {
}.expect("Unable to serialize key for canonical ordering");
buf.finalize()
}

/// Converts a literal to a valid rust expression capable of initializing a Primitive
/// e.g. Text is an actual String, etc
pub fn to_primitive_str_assign(&self) -> String {
match self {
FixedValue::Null => "None".to_owned(),
FixedValue::Bool(b) => b.to_string(),
FixedValue::Nint(i) => i.to_string(),
FixedValue::Uint(u) => u.to_string(),
FixedValue::Text(s) => format!("\"{}\".to_owned()", s),
}
}

/// Converts a literal to a valid rust comparison valid for comparisons
/// e.g. Text can be &str to avoid creating a String
pub fn to_primitive_str_compare(&self) -> String {
match self {
FixedValue::Text(s) => format!("\"{}\"", s),
_=> self.to_primitive_str_assign(),
}
}
}

#[derive(Clone, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -498,6 +519,7 @@ impl Primitive {
})
}

/// All POSSIBLE outermost CBOR types this can encode to
pub fn cbor_types(&self) -> Vec<CBORType> {
match self {
Primitive::Bool => vec![CBORType::Special],
Expand Down Expand Up @@ -689,7 +711,20 @@ impl RustType {

pub fn default(mut self, default_value: FixedValue) -> Self {
assert!(self.default.is_none());
// TODO: verify that the fixed value makes sense for the conceptual_type
let matches = if let ConceptualRustType::Primitive(p) = self.conceptual_type.clone().resolve_aliases() {
match &default_value {
FixedValue::Bool(_) => p == Primitive::Bool,
FixedValue::Nint(_) => p.cbor_types().contains(&CBORType::NegativeInteger),
FixedValue::Uint(_) => p.cbor_types().contains(&CBORType::UnsignedInteger),
FixedValue::Null => false,
FixedValue::Text(_) => p == Primitive::Str,
}
} else {
false
};
if !matches {
panic!(".default {:?} invalid for type {:?}", default_value, self.conceptual_type);
}
self.default = Some(default_value);
self
}
Expand All @@ -707,6 +742,7 @@ impl RustType {
}
}

/// All POSSIBLE outermost CBOR types this can encode to
pub fn cbor_types(&self) -> Vec<CBORType> {
match self.encodings.last() {
Some(CBOREncodingOperation::Tagged(_)) => vec![CBORType::Tag],
Expand Down Expand Up @@ -970,7 +1006,7 @@ impl ConceptualRustType {
}
}

/// IDENTIFIER for an enum variant. (Use for_rust_member() for the )
/// IDENTIFIER for an enum variant. (Use for_rust_member() for the enum value)
pub fn for_variant(&self) -> VariantIdent {
match self {
Self::Fixed(f) => f.for_variant(),
Expand All @@ -991,17 +1027,7 @@ impl ConceptualRustType {
/// can_fail is for cases where checks (e.g. range checks) are done if there
/// is a type transformation (i.e. wrapper types) like text (wasm) -> #6.14(text) (rust)
pub fn from_wasm_boundary_clone(&self, expr: &str, can_fail: bool) -> Vec<ToWasmBoundaryOperations> {
//assert!(matches!(self, Self::Tagged(_, _)) || !can_fail);
match self {
// Self::Tagged(_tag, ty) => {
// let mut inner = ty.from_wasm_boundary_clone(expr, can_fail);
// if can_fail {
// inner.push(ToWasmBoundaryOperations::TryInto);
// } else {
// inner.push(ToWasmBoundaryOperations::Into);
// }
// inner
// },
let mut ops = match self {
Self::Rust(_ident) => vec![
ToWasmBoundaryOperations::Code(format!("{}.clone()", expr)),
ToWasmBoundaryOperations::Into,
Expand All @@ -1021,22 +1047,16 @@ impl ConceptualRustType {
ToWasmBoundaryOperations::Into,
],
_ => vec![ToWasmBoundaryOperations::Code(expr.to_owned())],
};
if can_fail {
ops.push(ToWasmBoundaryOperations::TryInto);
}
ops
}

fn from_wasm_boundary_clone_optional(&self, expr: &str, can_fail: bool) -> Vec<ToWasmBoundaryOperations> {
//assert!(matches!(self, Self::Tagged(_, _)) || !can_fail);
match self {
let mut ops = match self {
Self::Primitive(_p) => vec![ToWasmBoundaryOperations::Code(expr.to_owned())],
// Self::Tagged(_tag, ty) => {
// let mut inner = ty.from_wasm_boundary_clone_optional(expr, can_fail);
// if can_fail {
// inner.push(ToWasmBoundaryOperations::TryInto);
// } else {
// inner.push(ToWasmBoundaryOperations::Into);
// }
// inner
// },
Self::Alias(_ident, ty) => ty.from_wasm_boundary_clone_optional(expr, can_fail),
Self::Array(..) |
Self::Rust(..) |
Expand All @@ -1049,7 +1069,11 @@ impl ConceptualRustType {
},
],
_ => panic!("unsupported or unexpected"),
};
if can_fail {
ops.push(ToWasmBoundaryOperations::TryInto);
}
ops
}

/// for non-owning parameter TYPES from wasm
Expand Down Expand Up @@ -1612,7 +1636,24 @@ impl RustRecord {
// maps are defined by their keys instead (although they shouldn't have multi-length values either...)
Representation::Map => ("_", String::from("1")),
};
conditional_field_expr.push_str(&format!("match &self.{} {{ Some({}) => {}, None => 0 }}", field.name, field_expr, field_contribution));
if let Some(default_value) = &field.rust_type.default {
if CLI_ARGS.preserve_encodings {
conditional_field_expr.push_str(&format!(
"if self.{} != {} || self.encodings.as_ref().map(|encs| encs.{}_default_present).unwrap_or(false) {{ {} }} else {{ 0 }}",
field.name,
default_value.to_primitive_str_compare(),
field.name,
field_contribution));
} else {
conditional_field_expr.push_str(&format!(
"if self.{} != {} {{ {} }} else {{ 0 }}",
field.name,
default_value.to_primitive_str_compare(),
field_contribution));
}
} else {
conditional_field_expr.push_str(&format!("match &self.{} {{ Some({}) => {}, None => 0 }}", field.name, field_expr, field_contribution));
}
} else {
match self.rep {
Representation::Array => match field.rust_type.conceptual_type.expanded_field_count(types) {
Expand Down
34 changes: 30 additions & 4 deletions src/parsing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use crate::utils::{
enum ControlOperator {
Range((Option<i128>, Option<i128>)),
CBOR(RustType),
Default(FixedValue),
}

struct Type2AndParent<'a> {
Expand Down Expand Up @@ -143,6 +144,15 @@ fn type2_to_number_literal(type2: &Type2) -> isize {
}
}

fn type2_to_fixed_value(type2: &Type2) -> FixedValue {
match type2 {
Type2::UintValue{ value, .. } => FixedValue::Uint(*value),
Type2::IntValue{ value, .. } => FixedValue::Nint(*value),
Type2::TextValue{ value, .. } => FixedValue::Text(value.to_string()),
_ => panic!("Type2: {:?} does not correspond to a supported FixedValue", type2),
}
}

fn parse_control_operator(types: &mut IntermediateTypes, parent: &Type2AndParent, operator: &Operator) -> ControlOperator {
let lower_bound = match parent.type2 {
Type2::Typename{ ident, .. } if ident.to_string() == "uint" => Some(0),
Expand All @@ -165,10 +175,10 @@ fn parse_control_operator(types: &mut IntermediateTypes, parent: &Type2AndParent
ControlOperator::Range((Some(range_start as i128), Some(if is_inclusive { range_end as i128 } else { (range_end + 1) as i128 })))
},
RangeCtlOp::CtlOp{ ctrl, .. } => match ctrl {
cddl::token::ControlOperator::DEFAULT |
cddl::token::ControlOperator::CBORSEQ |
cddl::token::ControlOperator::WITHIN |
cddl::token::ControlOperator::AND => todo!("control operator {} not supported", ctrl),
cddl::token::ControlOperator::DEFAULT => ControlOperator::Default(type2_to_fixed_value(&operator.type2)),
cddl::token::ControlOperator::CBOR => ControlOperator::CBOR(rust_type_from_type2(types, &Type2AndParent { type2: &operator.type2, parent: parent.parent, })),
cddl::token::ControlOperator::EQ => ControlOperator::Range((Some(type2_to_number_literal(&operator.type2) as i128), Some(type2_to_number_literal(&operator.type2) as i128))),
// TODO: this would be MUCH nicer (for error displaying, etc) to handle this in its own dedicated way
Expand Down Expand Up @@ -286,6 +296,11 @@ fn parse_type(types: &mut IntermediateTypes, type_name: &RustIdent, type_choice:
},
_ => panic!(".cbor is only allowed on bytes as per CDDL spec"),
},
ControlOperator::Default(default_value) => {
let default_type = rust_type_from_type2(types, &Type2AndParent { type2: &type1.type2, parent: &type1 })
.default(default_value);
types.register_type_alias(type_name.clone(), default_type, true, true);
},
}
},
None => {
Expand Down Expand Up @@ -365,6 +380,12 @@ fn parse_type(types: &mut IntermediateTypes, type_name: &RustIdent, type_choice:
_ => types.register_rust_struct(RustStruct::new_wrapper(type_name.clone(), *tag, new_type, Some(min_max)))
}
},
Some(ControlOperator::Default(default_value)) => {
let default_tagged_type = rust_type_from_type2(types, &Type2AndParent { parent: &inner_type.type1, type2: &inner_type.type1.type2 })
.default(default_value)
.tag(tag_unwrap);
types.register_type_alias(type_name.clone(), default_tagged_type, true, true);
},
None => {
// TODO: this would be fixed if we ordered definitions via a dependency graph to begin with
// which would also allow us to do a single pass instead of many like we do now
Expand Down Expand Up @@ -599,19 +620,24 @@ fn group_entry_to_raw_field_name(entry: &GroupEntry) -> Option<String> {

fn rust_type_from_type1(types: &mut IntermediateTypes, type1: &Type1) -> RustType {
let control = type1.operator.as_ref().map(|op| parse_control_operator(types, &Type2AndParent { parent: type1, type2: &type1.type2 }, op));
let base_type = rust_type_from_type2(types, &Type2AndParent { type2: &type1.type2, parent: type1, });
// println!("type1: {:#?}", type1);
match control {
Some(ControlOperator::CBOR(ty)) => ty.as_bytes(),
Some(ControlOperator::CBOR(ty)) => {
assert!(matches!(base_type.conceptual_type.resolve_aliases(), ConceptualRustType::Primitive(Primitive::Bytes)));
ty.as_bytes()
},
Some(ControlOperator::Range(min_max)) => {
match &type1.type2 {
Type2::Typename{ ident, .. } if ident.to_string() == "uint" || ident.to_string() == "int" => match range_to_primitive(min_max.0, min_max.1) {
Some(t) => t.into(),
None => panic!("unsupported range for {:?}: {:?}", ident.to_string().as_str(), control)
},
_ => rust_type_from_type2(types, &Type2AndParent { type2: &type1.type2, parent: type1, })
_ => base_type
}
},
_ => rust_type_from_type2(types, &Type2AndParent { type2: &type1.type2, parent: type1, })
Some(ControlOperator::Default(default_value)) => base_type.default(default_value),
None => base_type,
}
}

Expand Down
9 changes: 8 additions & 1 deletion tests/core/input.cddl
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,11 @@ signed_ints = [
; The fix would be ideal as even though the true min in CBOR would be -u64::MAX
; we can't test that since isize::BITS is never > 64 in any normal system and likely never will be
i64_min: -9223372036854775808
]
]

default_uint = uint .default 1337

map_with_defaults = {
? 1 : default_uint
? 2 : text .default "two"
}
10 changes: 10 additions & 0 deletions tests/core/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,14 @@ mod tests {
let max = SignedInts::new(u8::MAX, u16::MAX, u32::MAX, u64::MAX, i8::MAX, i16::MAX, i32::MAX, i64::MAX, u64::MAX);
deser_test(&max);
}

#[test]
fn defaults() {
let mut md = MapWithDefaults::new();
deser_test(&md);
md.key_1 = 0;
deser_test(&md);
md.key_2 = "not two".into();
deser_test(&md);
}
}
9 changes: 8 additions & 1 deletion tests/preserve-encodings/input.cddl
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,11 @@ signed_ints = [
; The fix would be ideal as even though the true min in CBOR would be -u64::MAX
; we can't test that since isize::BITS is never > 64 in any normal system and likely never will be
i64_min: -9223372036854775808
]
]

default_uint = uint .default 1337

map_with_defaults = {
? 1 : default_uint
? 2 : text .default "two"
}
44 changes: 44 additions & 0 deletions tests/preserve-encodings/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,4 +506,48 @@ mod tests {
assert_eq!(irregular_bytes_max, irregular_max.to_bytes());
}
}

#[test]
fn defaults() {
let def_encodings = vec![Sz::Inline, Sz::One, Sz::Two, Sz::Four, Sz::Eight];
let str_3_encodings = vec![
StringLenSz::Len(Sz::Eight),
StringLenSz::Len(Sz::Inline),
StringLenSz::Indefinite(vec![(1, Sz::Two), (2, Sz::One)]),
StringLenSz::Indefinite(vec![(2, Sz::Inline), (0, Sz::Inline), (1, Sz::Four)]),
];
let bools = [(false, true), (true, false), (true, true)];
for str_enc in &str_3_encodings {
for def_enc in &def_encodings {
for ((key_1_present, key_1_default), (key_2_present, key_2_default)) in bools.iter().zip(bools.iter()) {
let value_1: u64 = if *key_1_default { 1337 } else { 2 };
let value_2 = if *key_2_default { "two" } else { "one" };
let irregular_bytes = vec![
vec![MAP_INDEF],
if *key_1_present {
vec![
cbor_int(1, *def_enc),
cbor_int(value_1 as i128, Sz::Two),
].into_iter().flatten().clone().collect::<Vec<u8>>()
} else {
vec![]
},
if *key_2_present {
vec![
cbor_int(2, *def_enc),
cbor_str_sz(value_2, str_enc.clone()),
].into_iter().flatten().clone().collect::<Vec<u8>>()
} else {
vec![]
},
vec![BREAK],
].into_iter().flatten().clone().collect::<Vec<u8>>();
let irregular = MapWithDefaults::from_bytes(irregular_bytes.clone()).unwrap();
assert_eq!(irregular_bytes, irregular.to_bytes());
assert_eq!(irregular.key_1, value_1);
assert_eq!(irregular.key_2, value_2);
}
}
}
}
}

0 comments on commit a995bad

Please sign in to comment.