wires: fight with macros, write some tests, and banish a bug

i, a recovering C programmer, have not yet learned to write hygienic macros

hdl/src/wires/conversions.rs

@@ -175,14 +175,29 @@ macro_rules! into_bits_impl {
 }
 
 macro_rules! impl_for_size {
-    ($type:ty, $marker_trait:path, $nom:expr) => {
-        #[doc = "Wires with 0 to 8 bits (0 to 1 bytes) can be represented by a `"]
+    // ($type:ty, $marker_trait:path, $nom:expr, $mod_name:ident) => {
+    ($type:ty, $marker_trait:path, $nom:expr, $mod_name:ident) => {
+        // mod $mod_name {
+        //     const BITS: usize = core::mem::size_of::<$type>() * 8;
+        //     const BYTES: usize = core::mem::size_of::<$type>();
+        // }
+
+        // #[doc = "Wires with 0 to "]
+        // #[doc = $mod_name::BITS]
+        // #[doc = " bits (0 to "]
+        // #[doc = $mod_name::BYTES]
+        // #[doc = " bytes) can be represented by a `"]
+        // #[doc = $nom]
+        // #[doc = "`."]
+        #[doc = "Wires with 0 to B bits (0 to S bytes) can be represented by a `"]
         #[doc = $nom]
         #[doc = "`."]
         impl<const B: BitCountType, const S: usize> From<Wire<{ B }, { S }>> for $type
         where
             Wire<{ B }, { S }>: $marker_trait,
         {
+            // Any impl of this within the `hdl` crate should use trait marker bounds so
+            // that S >= size_of::<$type>.
             fn from(wire: Wire<{ B }, { S }>) -> Self {
                 // This causes an ICE:
                 // let _bytes = wire.get_bytes::<{core::mem::size_of::<Self>()}>();
@@ -191,7 +206,9 @@ macro_rules! impl_for_size {
                 const SIZE: usize = core::mem::size_of::<$type>();
                 let mut bytes = crate::util::ConstU8Arr::<{SIZE}>::new();
 
-                bytes[(SIZE - S)..SIZE].copy_from_slice(&wire.repr);
+                // This is wrong! We're little endian!
+                // bytes[(SIZE - S)..SIZE].copy_from_slice(&wire.repr);
+                bytes[0..S].copy_from_slice(&wire.repr);
 
                 // There currently doesn't seem to be a way to get the typechecker to
                 // understand that a const generic value is the same as another constant
@@ -206,11 +223,28 @@ macro_rules! impl_for_size {
             }
         }
 
+        #[doc = "Wires with 0 to B bits (0 to S bytes) can be represented by a `"]
+        #[doc = $nom]
+        #[doc = "`."]
+        impl<const B: BitCountType, const S: usize> From<&Wire<{ B }, { S }>> for $type
+        where
+            Wire<{ B }, { S }>: $marker_trait,
+        {
+            fn from(wire: &Wire<{ B }, { S }>) -> Self {
+                const SIZE: usize = core::mem::size_of::<$type>();
+                let mut bytes = crate::util::ConstU8Arr::<{SIZE}>::new();
+
+                bytes[(SIZE - S)..SIZE].copy_from_slice(&wire.repr);
+
+                Self::from_le_bytes(*bytes)
+            }
+        }
+
         into_bits_impl!($type);
     };
 
-    ($type:ty, $marker_trait:path) => {
-        impl_for_size!($type, $marker_trait, stringify!($type));
+    ($id:ident, $type:ty, $marker_trait:path) => {
+        impl_for_size!($type, $marker_trait, stringify!($type), $id);
     }
 }
 
@@ -274,11 +308,11 @@ pub trait IntoBits/*: NumBytes where Self: NumBytes*/ {
 // impl_for_size!(u64, FitsInU64, IntoBits::U64);
 // impl_for_size!(u128, FitsInU128, IntoBits::U128);
 
-impl_for_size!(u8, FitsInU8);
-impl_for_size!(u16, FitsInU16);
-impl_for_size!(u32, FitsInU32);
-impl_for_size!(u64, FitsInU64);
-impl_for_size!(u128, FitsInU128);
+impl_for_size!(_u8, u8, FitsInU8);
+impl_for_size!(_u16, u16, FitsInU16);
+impl_for_size!(_u32, u32, FitsInU32);
+impl_for_size!(_u64, u64, FitsInU64);
+impl_for_size!(_u128, u128, FitsInU128);
 
 into_bits_impl!(usize);
 
@@ -354,3 +388,88 @@ into_bits_impl!(usize);
 // impl FitsInU128 for Wire<{1}, {num_bytes(2)}> {
 
 // }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::new_wire_with_val;
+
+    #[test]
+    fn u8_to_8b_roundtrip() {
+        for val in core::u8::MIN..=core::u8::MAX {
+            let w = new_wire_with_val!(8, val);
+            assert_eq!(val, w.into());
+        }
+    }
+
+    #[test]
+    fn u8_to_17b_roundtrip() {
+        for val in core::u8::MIN..=core::u8::MAX {
+            let w = new_wire_with_val!(17, val);
+            assert_eq!(val as u32, w.into());
+        }
+    }
+
+    #[test]
+    fn u8_to_47b_roundtrip() {
+        for val in core::u8::MIN..=core::u8::MAX {
+            let w = new_wire_with_val!(47, val);
+            assert_eq!(val as u64, w.into());
+        }
+    }
+
+    #[test]
+    fn u16_to_8b_roundtrip() {
+        for val in core::u16::MIN..=(core::u8::MAX as u16) {
+            let w = new_wire_with_val!(8, val);
+            let z: u8 = w.into();
+            println!("{}", z);
+            assert_eq!(val, w.into());
+        }
+    }
+
+    #[test]
+    fn u16_to_16b_roundtrip() {
+        for val in core::u16::MIN..=core::u16::MAX {
+            let w = new_wire_with_val!(16, val);
+            assert_eq!(val, w.into());
+        }
+    }
+
+    #[test]
+    fn u16_to_19b_roundtrip() {
+        for val in core::u16::MIN..=core::u16::MAX {
+            let w = new_wire_with_val!(17, val);
+            assert_eq!(val as u32, w.into());
+        }
+    }
+
+    #[test]
+    fn u16_to_98b_roundtrip() {
+        for val in core::u16::MIN..=core::u16::MAX {
+            let w = new_wire_with_val!(98, val);
+            assert_eq!(val as u128, w.into());
+        }
+    }
+
+    #[test]
+    fn other_nums() {
+        macro_rules! val_test {
+            ($bits:literal, $num:expr) => {
+                assert_eq!($num, new_wire_with_val!($bits, $num).into())
+            };
+        }
+
+        val_test!(32, core::u32::MIN);
+        val_test!(32, core::u32::MAX);
+
+        val_test!(64, core::u64::MIN);
+        val_test!(64, core::u64::MAX);
+
+        val_test!(128, core::u128::MIN);
+        val_test!(128, core::u128::MAX);
+
+        // assert_eq!(core::u32::MAX, new_wire_with_val!(32, core::u32::MAX).into());
+    }
+
+}

hdl/src/wires/mod.rs

@@ -12,9 +12,9 @@ mod conversions;
 
 use core::mem::{size_of, MaybeUninit};
 
+use crate::util::ConstU8Arr;
 use conversions::IntoBits;
 use core::convert::TryInto;
-use crate::util::ConstU8Arr;
 
 /// The type used to count the number of bits a wire contains.
 ///
@@ -67,10 +67,10 @@ const_assert!(redundant_check; core::usize::MAX >= BIT_COUNT_MAX as usize / 8);
 ///    8  |   1
 ///    9  |   2
 ///
-/// Equivalently, add 7 and divide by 8 (truncating or flooring):
+/// Equivalently, add 7 and divide by 8 (truncating or flooring).
 const fn num_bytes(bits: BitCountType) -> usize {
-    //! To be thorough, we'll use checked operations even though the only real
-    //! danger is the add potentially overflowing.
+    //! To be thorough, we'll try to use checked operations even though the only
+    //! real danger is the add operation potentially overflowing.
     // Hopefully these will be stripped out in most cases, but this needs to be
     // tested (TODO).
     //
@@ -128,6 +128,7 @@ const fn byte_and_offset(bit: BitCountType) -> (usize, usize) {
     ((bit as usize) / 8, (bit as usize) % 8)
 }
 
+#[derive(Copy, Clone)]
 pub struct Wire<const B: BitCountType, const S: usize> {
     repr: [u8; S],
     // repr: [u8; (B as usize + 7) / 8],
@@ -146,15 +147,15 @@ pub struct Wire<const B: BitCountType, const S: usize> {
 //     Wire::<{ uno }, { dos }>::new()
 // }
 
-impl<const B: BitCountType, const S: usize> Wire<{B}, {S}> {
+impl<const B: BitCountType, const S: usize> Wire<{ B }, { S }> {
     #[inline]
     pub fn new() -> Self {
         // Make sure our number of bytes matches our number of bits:
         debug_assert!(S == num_bytes(B));
 
         Wire {
-            // 0s for our u8 arrays are a valid initialized state and not UB.
             #![allow(unsafe_code)]
+            // 0s for our u8 arrays are a valid initialized state and not UB.
             repr: unsafe { MaybeUninit::zeroed().assume_init() },
         }
     }
@@ -181,14 +182,14 @@ impl<const B: BitCountType, const S: usize> Wire<{B}, {S}> {
     /// but _only in debug mode_. If a value has fewer bits than a wire, the
     /// remaining bits will be set to 0.
     #[inline]
-    pub fn set<C: IntoBits>(&mut self, val: C) -> &Self {
+    pub fn set<C: IntoBits>(&mut self, val: C) -> &mut Self {
         // Check that the value we're trying to represent fits in the number of
         // bits we've got:
         // N.B. checking the number of leading zeros is a valid way to know how
         // many bits `val` requires since we only accept unsigned types.
-        debug_assert!(B >=
-            ((<usize as TryInto<BitCountType>>::try_into(C::BYTES).unwrap() * 8)
-             - (val.num_leading_zeros()))
+        debug_assert!(
+            B >= ((<usize as TryInto<BitCountType>>::try_into(C::BYTES).unwrap() * 8)
+                - (val.num_leading_zeros()))
         );
         // debug_assert!(B >= ((C::BYTES * 8) - (val.num_leading_zeros().try_into().unwrap())));
         // debug_assert!(B >= ((C::BYTES * 8) - (val.num_leading_zeros().try_into().unwrap())));
@@ -238,35 +239,46 @@ impl<const B: BitCountType, const S: usize> Wire<{B}, {S}> {
     // invoked). We found a workaround that we're using for now, but we'll still leave
     // this function in in-case this works once const generics become more stable.
     #[allow(unused)]
-    fn get_bytes</*T: core::marker::Sized, */const U: usize>(&self) -> [u8; U] {
+    fn get_bytes<const U: usize>(&self) -> [u8; U] {
         // let mut bytes = [0u8; core::mem::size_of::<T>()];
-        let mut bytes = ConstU8Arr::<{U}>::new();
+        let mut bytes = ConstU8Arr::<{ U }>::new();
 
-        bytes[(U-S)..U].copy_from_slice(&self.repr);
+        bytes[(U - S)..U].copy_from_slice(&self.repr);
         *bytes
     }
 
     // fn new_with_inference() -> Self
 }
 
-#[cfg(test)]
-mod tests {
-    use super::*;
+#[macro_export(crate)]
+macro_rules! new_wire {
+    ($bits:expr) => {
+        Wire::<{ $bits }, { num_bytes($bits) }>::new()
+    };
+}
 
-    macro_rules! new_wire {
-        ($bits:expr) => {Wire::<{ $bits }, { num_bytes($bits) }>::new()};
-    }
+// For some reason this fails to compile:
+// Update: it's because calls to `Wire::new_with_val` fail to compile (just like
+// `Wire::get_bytes`). The below works so we'll use it.
+// macro_rules! new_wire_with_val {
+//     ($bits:expr, $val:expr) => {Wire::<{ $bits }, { num_bytes($bits) }>::new_with_val($val)};
+// }
 
-    // For some reason this fails to compile:
-    // Update: it's because calls to `Wire::new_with_val` fail to compile (just like
-    // `Wire::get_bytes`). The below works so we'll use it.
-    // macro_rules! new_wire_with_val {
-    //     ($bits:expr, $val:expr) => {Wire::<{ $bits }, { num_bytes($bits) }>::new_with_val($val)};
-    // }
+#[macro_export(crate)]
+macro_rules! new_wire_with_val {
+    ($bits:expr, $val:expr) => {
+        {
+            let mut w = crate::new_wire!($bits);
+            w.set($val);
 
-    macro_rules! new_wire_with_val {
-        ($bits:expr, $val:expr) => {new_wire!($bits).set($val)};
-    }
+            w
+        }
+    };
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
 
     #[test]
     fn new() {
@@ -302,7 +314,7 @@ mod tests {
 
         macro_rules! max_test {
             ($type:ty, $max:expr) => {
-                new_wire_with_val!((8 * core::mem::size_of::<$type>()) as BitCountType, $max);
+                new_wire_with_val!((8 * size_of::<$type>()) as BitCountType, $max);
             };
         }
 
@@ -366,7 +378,7 @@ mod tests {
     #[test]
     fn fewer_bytes_but_enough_bits() {
         // A wire with 42 bits should have 6 bytes.
-        assert_eq!(size_of::<Wire::<{42}, {num_bytes(42)}>>(), 6);
+        assert_eq!(size_of::<Wire::<{ 42 }, { num_bytes(42) }>>(), 6);
 
         // This tests that we're actually checking the number of bits in our
         // bounds checks and not the number of bytes of the given type. A `u64`