Dynamic Int generation + integer primitive fixes (#97)

* Dynamic Int generation

Previously the `Int` struct was statically defined. This causes issues
because some traits, wrappers, etc are only needed for certain runtime
configurations (e.g. preserve-encodings) or usages (e.g. if it's used as a key).

This also introduces more functionality like a wasm wrapper again and
conversion traits.

Fixes #55

* Fixes for different int precisions + Tests

Fixes #57

Also addresses issues with other primtive types e.g. i8, i64, u8, etc
and adds unit tests to cover all of them.

Fixes a few issues with wasm bindings for `Int`.

* Misc fixes

* Option<T> for non-primitive T compiles properly now (doesn't use a
ref)

* removed TODO that was actually already implemented

* fix for maps in keys (OrderedHashMap now derives comparisons when
needed)

* Full nint support

Now even with `preserve-encodings=false` we use the
`*negative_integer_sz()` functions in `cbor_event` as the non-`_sz` ones
have two issues: 1) they use `i64` as an interface and thus only cover
half of `nint`'s range and 2) there is a panic when serializing
`i64::MIN` so even for `i64` it is not complete.

See primetype/cbor_event#9

This commit also changes the `nint` type to be an unsigned `u64` to
cover the complete range and not allow in the rust code values from both
sides of the sign. This might be a little awkward it is represents
exactly the values possible with `nint`.

Negative constants are also added by this.

* link cbor_event upstream issue

Co-authored-by: Sebastien Guillemot <[email protected]>

Author: rooooooooob

src/generation.rs

@@ -366,7 +366,7 @@ pub enum Representation {
 pub enum FixedValue {
     Null,
     Bool(bool),
-    Int(isize),
+    Nint(isize),
     Uint(usize),
     // float not supported right now - doesn't appear to be in cbor_event
     //Float(f64),
@@ -386,7 +386,7 @@ impl FixedValue {
                 true => "True",
                 false => "False",
             }),
-            FixedValue::Int(i) => VariantIdent::new_custom(format!("U{}", i)),
+            FixedValue::Nint(i) => VariantIdent::new_custom(format!("U{}", i)),
             FixedValue::Uint(u) => VariantIdent::new_custom(format!("I{}", u)),
             //FixedValue::Float(f) => format!("F{}", f),
             FixedValue::Text(s) => VariantIdent::new_custom(convert_to_alphanumeric(&convert_to_camel_case(&s))),
@@ -398,7 +398,7 @@ impl FixedValue {
         match self {
             FixedValue::Null => buf.write_special(cbor_event::Special::Null),
             FixedValue::Bool(b) => buf.write_special(cbor_event::Special::Bool(*b)),
-            FixedValue::Int(i) => buf.write_negative_integer(*i as i64),
+            FixedValue::Nint(i) => buf.write_negative_integer(*i as i64),
             FixedValue::Uint(u) => buf.write_unsigned_integer(*u as u64),
             FixedValue::Text(s) => buf.write_text(s),
         }.expect("Unable to serialize key for canonical ordering");
@@ -444,8 +444,7 @@ impl Primitive {
             Primitive::I32 => "i32",
             Primitive::U64 => "u64",
             Primitive::I64 => "i64",
-            // TODO: this should ideally be a u64 but the cbor_event lib takes i64 anyway so we don't get that extra precision
-            Primitive::N64 => "i64",
+            Primitive::N64 => "u64",
             Primitive::Str => "String",
             Primitive::Bytes => "Vec<u8>",
         })
@@ -672,11 +671,10 @@ impl RustType {
                         Primitive::I16 |
                         Primitive::U16 |
                         Primitive::I32 |
-                        Primitive::U32 => true,
-                        // since we generate these as BigNum/Int wrappers we can't nest them
+                        Primitive::U32 |
                         Primitive::I64 |
                         Primitive::N64 |
-                        Primitive::U64 => false,
+                        Primitive::U64 => true,
                         // Bytes is already implemented as Vec<u8> so we can't nest it
                         Primitive::Bytes => false,
                         // Vec<String> is not supported by wasm-bindgen
@@ -743,25 +741,34 @@ impl RustType {
 
     /// Function parameter TYPE from wasm (i.e. ref for non-primitives, value for supported primitives)
     pub fn for_wasm_param(&self) -> String {
+        self.for_wasm_param_impl(false)
+    }
+
+    fn for_wasm_param_impl(&self, force_not_ref: bool) -> String {
+        let opt_ref = if force_not_ref {
+            ""
+        } else {
+            "&"
+        };
         match self {
             RustType::Fixed(_) => panic!("should not expose Fixed type to wasm, only here for serialization: {:?}", self),
             RustType::Primitive(p) => p.to_string(),
-            RustType::Rust(ident) => format!("&{}", ident),
+            RustType::Rust(ident) => format!("{}{}", opt_ref, ident),
             RustType::Array(ty) => if self.directly_wasm_exposable() {
                 ty.name_as_wasm_array()
             } else {
-                format!("&{}", ty.name_as_wasm_array())
+                format!("{}{}", opt_ref, ty.name_as_wasm_array())
             },
-            RustType::Tagged(_tag, ty) => ty.for_wasm_param(),
-            RustType::Optional(ty) => format!("Option<{}>", ty.for_wasm_param()),
-            RustType::Map(_k, _v) => format!("&{}", self.for_wasm_member()),
+            RustType::Tagged(_tag, ty) => ty.for_wasm_param_impl(force_not_ref),
+            RustType::Optional(ty) => format!("Option<{}>", ty.for_wasm_param_impl(true)),
+            RustType::Map(_k, _v) => format!("{}{}", opt_ref, self.for_wasm_member()),
             // it might not be worth generating this as alises are ignored by wasm-pack build, but
             // that could change in the future so as long as it doens't cause issues we'll leave it
             RustType::Alias(ident, ty) => match &**ty {
-                RustType::Rust(_) => format!("&{}", ident),
+                RustType::Rust(_) => format!("{}{}", opt_ref, ident),
                 _ => ident.to_string(),
             }
-            RustType::CBORBytes(ty) => ty.for_wasm_member(),
+            RustType::CBORBytes(ty) => ty.for_wasm_param(),
         }
     }
 
@@ -1005,7 +1012,7 @@ impl RustType {
         match self {
             RustType::Fixed(f) => vec![match f {
                 FixedValue::Uint(_) => CBORType::UnsignedInteger,
-                FixedValue::Int(_) => CBORType::NegativeInteger,
+                FixedValue::Nint(_) => CBORType::NegativeInteger,
                 FixedValue::Text(_) => CBORType::Text,
                 FixedValue::Null => CBORType::Special,
                 FixedValue::Bool(_) => CBORType::Special,

src/intermediate.rs

@@ -101,7 +101,6 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             gen_scope.wasm().import("wasm_bindgen::prelude", "*");
             gen_scope
                 .wasm()
-                .raw("mod prelude;")
                 .raw("use std::collections::BTreeMap;");
 
             if CLI_ARGS.preserve_encodings {
@@ -193,7 +192,6 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             wasm_toml.push_str("serde_json = \"1.0.57\"\n");
         }
         std::fs::write(rust_dir.join("wasm/Cargo.toml"), wasm_toml)?;
-        std::fs::copy("static/prelude_wasm.rs", rust_dir.join("wasm/src/prelude.rs"))?;
     }
 
     // json-gen crate for exporting JSON schemas

src/main.rs

@@ -353,7 +353,7 @@ fn parse_type(types: &mut IntermediateTypes, type_name: &RustIdent, type_choice:
         },
         // Note: bool constants are handled via Type2::Typename
         Type2::IntValue{ value, .. } => {
-            let fallback_type = RustType::Fixed(FixedValue::Int(*value));
+            let fallback_type = RustType::Fixed(FixedValue::Nint(*value));
 
             let control = type1.operator.as_ref().map(|op| parse_control_operator(types, &Type2AndParent { parent: type1, type2: &type1.type2 }, op));
             let base_type = match control {
@@ -588,7 +588,7 @@ fn rust_type_from_type2(types: &mut IntermediateTypes, type2: &Type2AndParent) -
     // TODO: socket plugs (used in hash type)
     match &type2.type2 {
         Type2::UintValue{ value, .. } => RustType::Fixed(FixedValue::Uint(*value)),
-        Type2::IntValue{ value, .. } => RustType::Fixed(FixedValue::Int(*value)),
+        Type2::IntValue{ value, .. } => RustType::Fixed(FixedValue::Nint(*value)),
         //Type2::FloatValue{ value, .. } => RustType::Fixed(FixedValue::Float(*value)),
         Type2::TextValue{ value, .. } => RustType::Fixed(FixedValue::Text(value.to_string())),
         Type2::Typename{ ident, generic_args, .. } => {
@@ -751,14 +751,14 @@ fn group_entry_to_key(entry: &GroupEntry) -> Option<FixedValue> {
             match ge.member_key.as_ref()? {
                 MemberKey::Value{ value, .. } => match value {
                     cddl::token::Value::UINT(x) => Some(FixedValue::Uint(*x)),
-                    cddl::token::Value::INT(x) => Some(FixedValue::Int(*x)),
+                    cddl::token::Value::INT(x) => Some(FixedValue::Nint(*x)),
                     cddl::token::Value::TEXT(x) => Some(FixedValue::Text(x.to_string())),
                     _ => panic!("unsupported map identifier(1): {:?}", value),
                 },
                 MemberKey::Bareword{ ident, .. } => Some(FixedValue::Text(ident.to_string())),
                 MemberKey::Type1{ t1, .. } => match &t1.type2 {
                     Type2::UintValue{ value, .. } => Some(FixedValue::Uint(*value)),
-                    Type2::IntValue{ value, .. } => Some(FixedValue::Int(*value)),
+                    Type2::IntValue{ value, .. } => Some(FixedValue::Nint(*value)),
                     Type2::TextValue{ value, .. } => Some(FixedValue::Text(value.to_string())),
                     _ => panic!("unsupported map identifier(2): {:?}", entry),
                 },

src/parsing.rs

@@ -1,6 +1,6 @@
-use core::hash::Hash;
+use core::hash::{Hash, Hasher};
 
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, Ord, Eq, PartialEq, PartialOrd)]
 pub struct OrderedHashMap<K, V>(linked_hash_map::LinkedHashMap<K, V>) where
     K : Hash + Eq + Ord;
 
@@ -24,3 +24,9 @@ impl<K, V> OrderedHashMap<K, V> where K : Hash + Eq + Ord {
     }
 }
 
+impl<K, V> Hash for OrderedHashMap<K, V> where K : Hash + Eq + Ord, V : Hash {
+    fn hash<H: Hasher>(&self, h: &mut H) {
+        self.0.hash(h);
+    }
+}
+

static/ordered_hash_map.rs

@@ -144,29 +144,3 @@ impl<T: Deserialize + Sized> FromBytes for T {
     }
 }
 
-#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
-pub enum Int {
-    Uint(u64),
-    Nint(u64),
-}
-
-impl cbor_event::se::Serialize for Int {
-    fn serialize<'se, W: Write>(&self, serializer: &'se mut Serializer<W>) -> cbor_event::Result<&'se mut Serializer<W>> {
-        match self {
-            Self::Uint(x) => serializer.write_unsigned_integer(*x),
-            Self::Nint(x) => serializer.write_negative_integer(-(*x as i128) as i64),
-        }
-    }
-}
-
-impl Deserialize for Int {
-    fn deserialize<R: BufRead + Seek>(raw: &mut Deserializer<R>) -> Result<Self, DeserializeError> {
-        (|| -> Result<_, DeserializeError> {
-            match raw.cbor_type()? {
-                cbor_event::Type::UnsignedInteger => Ok(Self::Uint(raw.unsigned_integer()?)),
-                cbor_event::Type::NegativeInteger => Ok(Self::Nint(-raw.negative_integer()? as u64)),
-                _ => Err(DeserializeFailure::NoVariantMatched.into()),
-            }
-        })().map_err(|e| e.annotate("Int"))
-    }
-}

static/prelude.rs

@@ -42,4 +42,23 @@ u16 = uint .le 65535
 u32 = 0..4294967295
 u64 = uint .size 8 ; 8 bytes
 i8 = -128..127
-i64 = int .size 8 ; 8 bytes
+i64 = int .size 8 ; 8 bytes
+
+signed_ints = [
+	u_8: uint .size 1,
+	u_16: uint .size 2,
+	u_32: uint .size 4,
+	u_64: uint .size 8,
+	i_8: int .size 1,
+	i_16: int .size 2,
+	i_32: int .size 4,
+	i_64: int .size 8,
+	n_64: nint
+	u64_max: 18446744073709551615,
+	; this test assumes i64::BITS == isize::BITS (i.e. 64-bit programs) or else the cddl parsing lib would mess up
+	; if this test fails on your platform we might need to either remove this part
+	; or make a fix for the cddl library.
+	; 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
+]

static/prelude_wasm.rs

@@ -118,4 +118,12 @@ mod tests {
         assert_eq!(0u64, U64::from(0u64));
         assert_eq!(0i64, I64::from(0i64));
     }
+
+    #[test]
+    fn signed_ints() {
+        let min = SignedInts::new(u8::MIN, u16::MIN, u32::MIN, u64::MIN, i8::MIN, i16::MIN, i32::MIN, i64::MIN, u64::MIN);
+        deser_test(&min);
+        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);
+    }
 }

tests/core/input.cddl

@@ -38,3 +38,22 @@ foo_bytes = bytes .cbor foo
 
 ; since we don't generate code for definitions like the above (should we if no one refers to it?)
 cbor_in_cbor = [foo_bytes, uint_bytes: bytes .cbor uint]
+
+signed_ints = [
+	u_8: uint .size 1,
+	u_16: uint .size 2,
+	u_32: uint .size 4,
+	u_64: uint .size 8,
+	i_8: int .size 1,
+	i_16: int .size 2,
+	i_32: int .size 4,
+	i_64: int .size 8,
+	n_64: nint
+	u64_max: 18446744073709551615,
+	; this test assumes i64::BITS == isize::BITS or else the cddl parsing lib would mess up
+	; if this test fails on your platform we might need to either remove this part
+	; or make a fix for the cddl library.
+	; The fix would be ideal as even though the true min in CBOR would be -u64::MAX
+	; we can't support since isize::BITS is never > 64 in any normal system and likely never will be
+	i64_min: -9223372036854775808
+]