From e35b12c7f019503c84cdbe180d2261ccfa2289e8 Mon Sep 17 00:00:00 2001 From: Sebastien Guillemot Date: Wed, 2 Nov 2022 23:42:12 +0900 Subject: [PATCH 1/3] Fix handling of standalone .cbor and tagged types --- src/parsing.rs | 104 ++++++++++++++++++++++++++++++++++++++++-- tests/core/input.cddl | 3 ++ tests/core/tests.rs | 12 +++++ 3 files changed, 115 insertions(+), 4 deletions(-) diff --git a/src/parsing.rs b/src/parsing.rs index 7962d7f..5c97b69 100644 --- a/src/parsing.rs +++ b/src/parsing.rs @@ -1,7 +1,8 @@ use cddl::ast::parent::ParentVisitor; use cddl::{ast::*, token}; use either::{Either}; -use std::collections::{BTreeMap}; +use std::collections::{BTreeMap, HashSet}; +use std::mem::Discriminant; use crate::comment_ast::{RuleMetadata, metadata_from_comments}; use crate::intermediate::{ @@ -279,7 +280,11 @@ fn parse_type(types: &mut IntermediateTypes, parent_visitor: &ParentVisitor, typ }, ControlOperator::CBOR(ty) => match field_type() { RustType::Primitive(Primitive::Bytes) => { - types.register_type_alias(type_name.clone(), RustType::CBORBytes(Box::new(ty)), true, true); + if has_embed(parent_visitor, get_rule(parent_visitor, &CDDLType::from(type1))) { + types.register_type_alias(type_name.clone(), RustType::CBORBytes(Box::new(ty)), true, true); + } else { + types.register_rust_struct(parent_visitor, RustStruct::new_wrapper(type_name.clone(), None, RustType::CBORBytes(Box::new(ty)), None)); + } }, _ => panic!(".cbor is only allowed on bytes as per CDDL spec"), }, @@ -351,7 +356,11 @@ fn parse_type(types: &mut IntermediateTypes, parent_visitor: &ParentVisitor, typ let base_type = types .apply_type_aliases(&AliasIdent::new(CDDLIdent::new(ident.to_string()))) .expect(&format!("Please move definition for {} above {}", type_name, ident)); - types.register_type_alias(type_name.clone(), RustType::Tagged(tag_unwrap, Box::new(RustType::CBORBytes(Box::new(base_type)))), true, true); + if has_embed(parent_visitor, get_rule(parent_visitor, &CDDLType::from(&inner_type.type1.type2))) { + types.register_type_alias(type_name.clone(), RustType::Tagged(tag_unwrap, Box::new(RustType::CBORBytes(Box::new(base_type)))), true, true) + } else { + types.register_rust_struct(parent_visitor, RustStruct::new_wrapper(type_name.clone(), Some(tag_unwrap), RustType::CBORBytes(Box::new(base_type)), None)); + } }, Some(ControlOperator::Range(min_max)) => { match ident.to_string().as_str() { @@ -368,7 +377,11 @@ fn parse_type(types: &mut IntermediateTypes, parent_visitor: &ParentVisitor, typ let base_type = types .apply_type_aliases(&AliasIdent::new(CDDLIdent::new(ident.to_string()))) .expect(&format!("Please move definition for {} above {}", type_name, ident)); - types.register_type_alias(type_name.clone(), RustType::Tagged(tag_unwrap, Box::new(base_type)), true, true); + if has_embed(parent_visitor, get_rule(parent_visitor, &CDDLType::from(&inner_type.type1.type2))) { + types.register_type_alias(type_name.clone(), RustType::Tagged(tag_unwrap, Box::new(base_type)), false, true); + } else { + types.register_rust_struct(parent_visitor, RustStruct::new_wrapper(type_name.clone(), Some(tag_unwrap), base_type, None)); + } }, } }, @@ -1033,6 +1046,13 @@ fn get_comment_after<'a>(parent_visitor: &'a ParentVisitor<'a, 'a>, cddl_type: & } } +fn get_rule<'a, 'b>(parent_visitor: &'a ParentVisitor, cddl_type: &CDDLType<'a, 'b>) -> &'a Rule<'a> { + match cddl_type { + CDDLType::CDDL(_) => panic!("Cannot get the rule name of a top-level CDDL node"), + CDDLType::Rule(rule) => rule, + other => get_rule(parent_visitor, other.parent(parent_visitor).unwrap()), + } +} fn get_rule_name<'a, 'b>(parent_visitor: &'a ParentVisitor, cddl_type: &CDDLType<'a, 'b>) -> Identifier<'a> { match cddl_type { CDDLType::CDDL(_) => panic!("Cannot get the rule name of a top-level CDDL node"), @@ -1046,3 +1066,79 @@ fn get_rule_name<'a, 'b>(parent_visitor: &'a ParentVisitor, cddl_type: &CDDLType other => get_rule_name(parent_visitor, other.parent(parent_visitor).unwrap()), } } + +/// An embedded type is a type embedded inside a larger structure (group, array, etc) +fn has_embed<'a>(parent_visitor: &'a ParentVisitor, rule: &Rule<'a>) -> bool { + _has_embed(parent_visitor, &CDDLType::from(rule)) +} +fn _has_embed<'a, 'b>(parent_visitor: &'a ParentVisitor, cddl_type: &CDDLType<'a, 'b>) -> bool { + match cddl_type { + CDDLType::CDDL(_) => panic!("has_embed cannot be called on a root CDDL type"), + CDDLType::Rule(rule) => match rule { + Rule::Type { rule, .. } => _has_embed(parent_visitor, &CDDLType::from(rule)), + Rule::Group { rule, .. } => _has_embed(parent_visitor, &CDDLType::from(&rule.entry)) + }, + CDDLType::TypeRule(rule) => _has_embed(parent_visitor, &CDDLType::from(&rule.value)), + CDDLType::GroupRule(_) => true, + CDDLType::Group(_) => true, + CDDLType::GroupChoice(_) => true, + CDDLType::GenericParams(_) => false, + CDDLType::GenericParam(_) => false, + CDDLType::GenericArgs(t) => { + match t.args.len() { + 1 => _has_embed(parent_visitor, &CDDLType::from(t.args.first().unwrap())), + _ => true, + } + }, + CDDLType::GenericArg(t) => _has_embed(parent_visitor, &CDDLType::from(t.arg.as_ref())), + CDDLType::GroupEntry(_) => true, + CDDLType::Identifier(_) => false, + CDDLType::Type(t) => { + match t.type_choices.len() { + 1 => _has_embed(parent_visitor, &CDDLType::from(t.type_choices.first().unwrap())), + _ => true, + } + }, + CDDLType::TypeChoice(_) => false, + CDDLType::Type1(t) => { + match _has_embed(parent_visitor, &CDDLType::from(&t.type2)) { + true => true, + false => match &t.operator { + None => false, + Some(op) => _has_embed(parent_visitor, &CDDLType::from(op)), + } + } + + }, + CDDLType::Type2(t) => match t { + Type2::ParenthesizedType { pt, .. } => _has_embed(parent_visitor, &CDDLType::from(pt)), + Type2::Map { .. } => true, + Type2::Array { .. } => true, + Type2::Unwrap { .. } => true, + Type2::ChoiceFromInlineGroup { .. } => true, + Type2::ChoiceFromGroup { .. } => true, + Type2::TaggedData { t, .. } => _has_embed(parent_visitor, &CDDLType::from(t)), + _ => false, + }, + CDDLType::Operator(op) => _has_embed(parent_visitor, &CDDLType::from(&op.type2)), + CDDLType::Occurrence(_) => true, + CDDLType::Occur(_) => true, + CDDLType::Value(_) => false, + CDDLType::ValueMemberKeyEntry(_) => true, + CDDLType::TypeGroupnameEntry(_) => true, + CDDLType::MemberKey(_) => true, + CDDLType::NonMemberKey(_) => true, + _ => false + } +} + +/// A recursive type is defied as one that has a node of the same type in its parent hierarchy +fn is_recursive<'a, 'b>(parent_visitor: &'a ParentVisitor, cddl_type: &CDDLType<'a, 'b>) -> bool { + _is_recursive(parent_visitor, &mut HashSet::new(), cddl_type) +} +fn _is_recursive<'a, 'b>(parent_visitor: &'a ParentVisitor, seen_set: &mut HashSet>>, cddl_type: &CDDLType<'a, 'b>) -> bool { + match seen_set.insert(std::mem::discriminant(cddl_type)) { + true => _is_recursive(parent_visitor, seen_set, cddl_type.parent(parent_visitor).unwrap()), + false => false + } +} \ No newline at end of file diff --git a/tests/core/input.cddl b/tests/core/input.cddl index bf7cf78..e7eb8e2 100644 --- a/tests/core/input.cddl +++ b/tests/core/input.cddl @@ -65,3 +65,6 @@ signed_ints = [ paren_size = uint .size (1) paren_cbor = bytes .cbor (text) + +no_embed_tag = #6.24(uint) +no_embed_cbor = bytes .cbor uint diff --git a/tests/core/tests.rs b/tests/core/tests.rs index 512c9cb..6b93a34 100644 --- a/tests/core/tests.rs +++ b/tests/core/tests.rs @@ -126,4 +126,16 @@ 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 toplevel_types() { + { + let tag = NoEmbedTag::new(5); + assert_eq!(tag.to_bytes(), vec![0xd8, 0x18, 0x05]); + } + { + let cbor = NoEmbedCbor::new(5); + assert_eq!(cbor.to_bytes(), vec![0x41, 0x05]); + } + } } From 3cd111b321ee1618da68e76291457ea6fda7f292 Mon Sep 17 00:00:00 2001 From: Sebastien Guillemot Date: Wed, 2 Nov 2022 23:43:22 +0900 Subject: [PATCH 2/3] Remove unused utility functions --- src/intermediate.rs | 25 ------------------------- src/parsing.rs | 11 ----------- 2 files changed, 36 deletions(-) diff --git a/src/intermediate.rs b/src/intermediate.rs index 4983b2a..78e8064 100644 --- a/src/intermediate.rs +++ b/src/intermediate.rs @@ -52,22 +52,6 @@ impl<'a> IntermediateTypes<'a> { } } - pub fn has_ident(&self, ident: &RustIdent) -> bool { - let foo: Vec = self.type_aliases.keys().fold(vec![], |mut acc, alias| { - match alias { - AliasIdent::Reserved(_) => {}, - AliasIdent::Rust(ident) => { acc.push(ident.clone()) } - }; - acc - }); - println!("{:?}", self.plain_groups.keys().chain(foo.iter()).chain(self.rust_structs.keys()).chain(self.generic_defs.keys()).chain(self.generic_instances.keys())); - self.plain_groups.contains_key(ident) - || self.type_aliases.contains_key(&AliasIdent::Rust(ident.clone())) - || self.rust_structs.contains_key(ident) - || self.generic_defs.contains_key(ident) - || self.generic_instances.contains_key(ident) - } - pub fn type_aliases(&self) -> &BTreeMap { &self.type_aliases } @@ -1755,12 +1739,3 @@ impl GenericInstance { orig.clone() } } - -fn try_ident_with_id(intermediate_types: &IntermediateTypes, name: &CDDLIdent, value: u32) -> CDDLIdent { - let new_ident = CDDLIdent::new(format!("{}{}", name, value)); - let rust_ident = RustIdent::new(new_ident.clone()); - match intermediate_types.has_ident(&rust_ident) { - false => new_ident, - true => try_ident_with_id(intermediate_types, name, value + 1) - } -} diff --git a/src/parsing.rs b/src/parsing.rs index 5c97b69..0031663 100644 --- a/src/parsing.rs +++ b/src/parsing.rs @@ -1131,14 +1131,3 @@ fn _has_embed<'a, 'b>(parent_visitor: &'a ParentVisitor, cddl_type: &CDDLType<'a _ => false } } - -/// A recursive type is defied as one that has a node of the same type in its parent hierarchy -fn is_recursive<'a, 'b>(parent_visitor: &'a ParentVisitor, cddl_type: &CDDLType<'a, 'b>) -> bool { - _is_recursive(parent_visitor, &mut HashSet::new(), cddl_type) -} -fn _is_recursive<'a, 'b>(parent_visitor: &'a ParentVisitor, seen_set: &mut HashSet>>, cddl_type: &CDDLType<'a, 'b>) -> bool { - match seen_set.insert(std::mem::discriminant(cddl_type)) { - true => _is_recursive(parent_visitor, seen_set, cddl_type.parent(parent_visitor).unwrap()), - false => false - } -} \ No newline at end of file From f78cd042b5f34d37a08505cee7f0755bfeb6a8eb Mon Sep 17 00:00:00 2001 From: Sebastien Guillemot Date: Thu, 3 Nov 2022 13:08:31 +0900 Subject: [PATCH 3/3] Add missing paren tests --- tests/core/tests.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/core/tests.rs b/tests/core/tests.rs index 6b93a34..3a354df 100644 --- a/tests/core/tests.rs +++ b/tests/core/tests.rs @@ -127,6 +127,18 @@ mod tests { deser_test(&max); } + #[test] + fn paren_types() { + { + let _: ParenSize = 5u8; + } + { + let cbor = ParenCbor::new("foo".to_string()); + // TODO: fix this assert by inputting the right value + assert_eq!(cbor.to_bytes(), vec![]); + } + } + #[test] fn toplevel_types() { {