Update artiq/test/coredevice/test_portability.py

artiq/firmware/libproto_artiq/rpc_proto.rs

@@ -1,10 +1,14 @@
+// rpc_proto.rs
+
 use core::str;
 use core::slice;
 use cslice::{CSlice, CMutSlice};
 use byteorder::{NativeEndian, ByteOrder};
 use io::{ProtoRead, Read, Write, ProtoWrite, Error};
-use self::tag::{Tag, TagIterator, split_tag};
+use self::tag::{Tag, TagIterator, split_tag_extended, parse_param_names};
+use core::mem;
 
+/// Round up a value to the next multiple of `power_of_two`.
 #[inline]
 fn round_up(val: usize, power_of_two: usize) -> usize {
     assert!(power_of_two.is_power_of_two());
@@ -12,6 +16,7 @@ fn round_up(val: usize, power_of_two: usize) -> usize {
     (val + max_rem) & (!max_rem)
 }
 
+/// Round up a mutable pointer.
 #[inline]
 unsafe fn round_up_mut<T>(ptr: *mut T, power_of_two: usize) -> *mut T {
     round_up(ptr as usize, power_of_two) as *mut T
@@ -22,21 +27,20 @@ unsafe fn round_up_const<T>(ptr: *const T, power_of_two: usize) -> *const T {
     round_up(ptr as usize, power_of_two) as *const T
 }
 
+/// Align a const pointer to T.
 #[inline]
 unsafe fn align_ptr<T>(ptr: *const ()) -> *const T {
     round_up_const(ptr, core::mem::align_of::<T>()) as *const T
 }
 
+/// Align a mutable pointer to T.
 #[inline]
 unsafe fn align_ptr_mut<T>(ptr: *mut ()) -> *mut T {
     round_up_mut(ptr, core::mem::align_of::<T>()) as *mut T
 }
 
 /// Reads (deserializes) `length` array or list elements of type `tag` from `reader`,
-/// writing them into the buffer given by `storage`.
-///
-/// `alloc` is used for nested allocations (if elements themselves contain
-/// lists/arrays), see [recv_value].
+/// writing them into `storage`. For compound types, it calls `recv_value` repeatedly.
 unsafe fn recv_elements<R, E>(
     reader: &mut R,
     tag: Tag,
@@ -48,66 +52,69 @@ where
     R: Read + ?Sized,
     E: From<Error<R::ReadError>>,
 {
-    // List of simple types are special-cased in the protocol for performance.
     match tag {
         Tag::Bool => {
             let dest = slice::from_raw_parts_mut(storage as *mut u8, length);
             reader.read_exact(dest)?;
-        },
+        }
         Tag::Int32 => {
             let dest = slice::from_raw_parts_mut(storage as *mut u8, length * 4);
             reader.read_exact(dest)?;
             let dest = slice::from_raw_parts_mut(storage as *mut i32, length);
             NativeEndian::from_slice_i32(dest);
-        },
+        }
         Tag::Int64 | Tag::Float64 => {
             let dest = slice::from_raw_parts_mut(storage as *mut u8, length * 8);
             reader.read_exact(dest)?;
             let dest = slice::from_raw_parts_mut(storage as *mut i64, length);
             NativeEndian::from_slice_i64(dest);
-        },
+        }
         _ => {
             let mut data = storage;
             for _ in 0..length {
-                recv_value(reader, tag, &mut data, alloc)?
+                recv_value(reader, tag, &mut data, alloc)?;
             }
         }
     }
     Ok(())
 }
 
-/// Reads (deserializes) a value of type `tag` from `reader`, writing the results to
-/// the kernel-side buffer `data` (the passed pointer to which is incremented to point
-/// past the just-received data). For nested allocations (lists/arrays), `alloc` is
-/// invoked any number of times with the size of the required allocation as a parameter
-/// (which is assumed to be correctly aligned for all payload types).
-unsafe fn recv_value<R, E>(reader: &mut R, tag: Tag, data: &mut *mut (),
-                           alloc: &dyn Fn(usize) -> Result<*mut (), E>)
-                          -> Result<(), E>
-    where R: Read + ?Sized,
-          E: From<Error<R::ReadError>>
+/// Reads (deserializes) a single value of type `tag` from `reader` into the buffer pointed
+/// to by `data`. For compound types it recurses.
+unsafe fn recv_value<R, E>(
+    reader: &mut R,
+    tag: Tag,
+    data: &mut *mut (),
+    alloc: &dyn Fn(usize) -> Result<*mut (), E>,
+) -> Result<(), E>
+where
+    R: Read + ?Sized,
+    E: From<Error<R::ReadError>>,
 {
     macro_rules! consume_value {
-        ($ty:ty, |$ptr:ident| $map:expr) => ({
+        ($ty:ty, |$ptr:ident| $map:expr) => {{
             let $ptr = align_ptr_mut::<$ty>(*data) as *mut $ty;
             *data = $ptr.offset(1) as *mut ();
             $map
-        })
+        }};
     }
 
     match tag {
         Tag::None => Ok(()),
         Tag::Bool =>
             consume_value!(u8, |ptr| {
-                *ptr = reader.read_u8()?; Ok(())
+                *ptr = reader.read_u8()?;
+                Ok(())
             }),
         Tag::Int32 =>
             consume_value!(u32, |ptr| {
-                *ptr = reader.read_u32()?; Ok(())
+                *ptr = reader.read_u32()?;
+                Ok(())
             }),
         Tag::Int64 | Tag::Float64 =>
             consume_value!(u64, |ptr| {
-                *ptr = reader.read_u64()?; Ok(())
+                *ptr = reader.read_u64()?;
+                Ok(())
             }),
         Tag::String | Tag::Bytes | Tag::ByteArray => {
             consume_value!(CMutSlice<u8>, |ptr| {
@@ -126,129 +133,141 @@ unsafe fn recv_value<R, E>(reader: &mut R, tag: Tag, data: &mut *mut (),
             *data = round_up_mut(*data, alignment);
             let mut it = it.clone();
             for _ in 0..arity {
-                let tag = it.next().expect("truncated tag");
-                recv_value(reader, tag, data, alloc)?
+                let subtag = it.next().expect("truncated tuple tag");
+                recv_value(reader, subtag, data, alloc)?;
             }
-            // Take into account any tail padding (if element(s) with largest alignment
-            // are not at the end).
             *data = round_up_mut(*data, alignment);
             Ok(())
         }
         Tag::List(it) => {
             #[repr(C)]
-            struct List { elements: *mut (), length: usize }
+            struct List {
+                elements: *mut (),
+                length: usize,
+            }
             consume_value!(*mut List, |ptr_to_list| {
-                let tag = it.clone().next().expect("truncated tag");
+                let elt_tag = it.clone().next().expect("truncated list tag");
                 let length = reader.read_u32()? as usize;
-
-                // To avoid multiple kernel CPU roundtrips, use a single allocation for
-                // both the pointer/length List (slice) and the backing storage for the
-                // elements. We can assume that alloc() is aligned suitably, so just
-                // need to take into account any extra padding required.
-                // (Note: On RISC-V, there will never actually be any types with
-                // alignment larger than 8 bytes, so storage_offset == 0 always.)
-                let list_size = 4 + 4;
-                let storage_offset = round_up(list_size, tag.alignment());
-                let storage_size = tag.size() * length;
-
+                let list_size = 4 + 4; // space for pointer + length
+                let storage_offset = round_up(list_size, elt_tag.alignment());
+                let storage_size = elt_tag.size() * length;
                 let allocation = alloc(storage_offset + storage_size)? as *mut u8;
                 *ptr_to_list = allocation as *mut List;
-                let storage = allocation.offset(storage_offset as isize) as *mut ();
-
+                let storage = allocation.add(storage_offset) as *mut ();
                 (**ptr_to_list).length = length;
                 (**ptr_to_list).elements = storage;
-                recv_elements(reader, tag, length, storage, alloc)
+                recv_elements(reader, elt_tag, length, storage, alloc)
             })
         }
         Tag::Array(it, num_dims) => {
             consume_value!(*mut (), |buffer| {
-                // Deserialize length along each dimension and compute total number of
-                // elements.
                 let mut total_len: usize = 1;
                 for _ in 0..num_dims {
                     let len = reader.read_u32()? as usize;
                     total_len *= len;
-                    consume_value!(usize, |ptr| *ptr = len )
+                    consume_value!(usize, |ptr| *ptr = len);
                 }
-
-                // Allocate backing storage for elements; deserialize them.
-                let elt_tag = it.clone().next().expect("truncated tag");
+                let elt_tag = it.clone().next().expect("truncated array tag");
                 *buffer = alloc(elt_tag.size() * total_len)?;
                 recv_elements(reader, elt_tag, total_len, *buffer, alloc)
             })
         }
         Tag::Range(it) => {
             *data = round_up_mut(*data, tag.alignment());
-            let tag = it.clone().next().expect("truncated tag");
-            recv_value(reader, tag, data, alloc)?;
-            recv_value(reader, tag, data, alloc)?;
-            recv_value(reader, tag, data, alloc)?;
+            let subtag = it.clone().next().expect("truncated range tag");
+            recv_value(reader, subtag, data, alloc)?;
+            recv_value(reader, subtag, data, alloc)?;
+            recv_value(reader, subtag, data, alloc)?;
             Ok(())
         }
-        Tag::Keyword(_) => unreachable!(),
-        Tag::Object => unreachable!()
+        Tag::Keyword(_) => unreachable!("Keyword tag should not appear in legacy composite types"),
+        Tag::Object => unreachable!("Object tag is not expected in legacy RPC"),
     }
 }
 
-pub fn recv_return<'a, R, E>(reader: &mut R, tag_bytes: &'a [u8], data: *mut (),
-                         alloc: &dyn Fn(usize) -> Result<*mut (), E>)
-                        -> Result<&'a [u8], E>
-    where R: Read + ?Sized,
-          E: From<Error<R::ReadError>>
+/// Receives a return value. Reads a tag from `tag_bytes`, then uses that tag to
+/// read data from `reader`. Returns any leftover bytes from the tag (if any).
+pub fn recv_return<'a, R, E>(
+    reader: &mut R,
+    tag_bytes: &'a [u8],
+    data: *mut (),
+    alloc: &dyn Fn(usize) -> Result<*mut (), E>,
+) -> Result<&'a [u8], E>
+where
+    R: Read + ?Sized,
+    E: From<Error<R::ReadError>>,
 {
-    let mut it = TagIterator::new(tag_bytes);
-    #[cfg(feature = "log")]
-    debug!("recv ...->{}", it);
+    // --- Extended: split the tag into three parts: argument types, parameter names, return type.
+    let (param_type_slice, param_name_slice, return_type_slice) = split_tag_extended(tag_bytes);
+    
+    // Parse parameter names from the parameter names slice.
+    let param_names = parse_param_names(param_name_slice);
+    
+    // Print the parsed keyword parameter names for debugging.
+    println!("[recv_return] Parameter names: {:?}", param_names);
 
-    let tag = it.next().expect("truncated tag");
-    let mut data = data;
-    unsafe { recv_value(reader, tag, &mut data, alloc)? };
-
-    Ok(it.data)
+    // Use the argument type slice to create the TagIterator for the return value.
+    let mut it = TagIterator::new(param_type_slice);
+    if let Some(ret_tag) = it.next() {
+        let mut data = data;
+        unsafe { recv_value(reader, ret_tag, &mut data, alloc)? };
+    }
+    Ok(return_type_slice)
 }
 
-unsafe fn send_elements<W>(writer: &mut W, elt_tag: Tag, length: usize, data: *const (), write_tags: bool)
-                          -> Result<(), Error<W::WriteError>>
-    where W: Write + ?Sized
+/// Writes (serializes) `length` elements of type `elt_tag` from `data` into `writer`.
+unsafe fn send_elements<W>(
+    writer: &mut W,
+    elt_tag: Tag,
+    length: usize,
+    data: *const (),
+    write_tags: bool,
+) -> Result<(), Error<W::WriteError>>
+where
+    W: Write + ?Sized,
 {
     if write_tags {
         writer.write_u8(elt_tag.as_u8())?;
     }
     match elt_tag {
-        // we cannot use NativeEndian::from_slice_i32 as the data is not mutable,
-        // and that is not needed as the data is already in native endian
         Tag::Bool => {
             let slice = slice::from_raw_parts(data as *const u8, length);
             writer.write_all(slice)?;
-        },
+        }
         Tag::Int32 => {
             let slice = slice::from_raw_parts(data as *const u8, length * 4);
             writer.write_all(slice)?;
-        },
+        }
         Tag::Int64 | Tag::Float64 => {
             let slice = slice::from_raw_parts(data as *const u8, length * 8);
             writer.write_all(slice)?;
-        },
+        }
         _ => {
-            let mut data = data;
+            let mut data_ptr = data;
             for _ in 0..length {
-                send_value(writer, elt_tag, &mut data, write_tags)?;
+                send_value(writer, elt_tag, &mut data_ptr, write_tags)?;
             }
         }
     }
     Ok(())
 }
 
-unsafe fn send_value<W>(writer: &mut W, tag: Tag, data: &mut *const (), write_tags: bool)
-                       -> Result<(), Error<W::WriteError>>
-    where W: Write + ?Sized
+/// Writes (serializes) a single value of type `tag` from `data` into `writer`.
+unsafe fn send_value<W>(
+    writer: &mut W,
+    tag: Tag,
+    data: &mut *const (),
+    write_tags: bool,
+) -> Result<(), Error<W::WriteError>>
+where
+    W: Write + ?Sized,
 {
     macro_rules! consume_value {
-        ($ty:ty, |$ptr:ident| $map:expr) => ({
+        ($ty:ty, |$ptr:ident| $map:expr) => {{
             let $ptr = align_ptr::<$ty>(*data);
-            *data = $ptr.offset(1) as *const ();
+            *data = $ptr.add(1) as *const ();
             $map
-        })
+        }};
     }
     if write_tags {
         writer.write_u8(tag.as_u8())?;
@@ -256,14 +275,11 @@ unsafe fn send_value<W>(writer: &mut W, tag: Tag, data: &mut *const (), write_ta
     match tag {
         Tag::None => Ok(()),
         Tag::Bool =>
-            consume_value!(u8, |ptr|
-                writer.write_u8(*ptr)),
+            consume_value!(u8, |ptr| writer.write_u8(*ptr)),
         Tag::Int32 =>
-            consume_value!(u32, |ptr|
-                writer.write_u32(*ptr)),
+            consume_value!(u32, |ptr| writer.write_u32(*ptr)),
         Tag::Int64 | Tag::Float64 =>
-            consume_value!(u64, |ptr|
-                writer.write_u64(*ptr)),
+            consume_value!(u64, |ptr| writer.write_u64(*ptr)),
         Tag::String =>
             consume_value!(CSlice<u8>, |ptr|
                 writer.write_string(str::from_utf8((*ptr).as_ref()).unwrap())),
@@ -277,9 +293,9 @@ unsafe fn send_value<W>(writer: &mut W, tag: Tag, data: &mut *const (), write_ta
             }
             let mut max_alignment = 0;
             for _ in 0..arity {
-                let tag = it.next().expect("truncated tag");
-                max_alignment = core::cmp::max(max_alignment, tag.alignment());
-                send_value(writer, tag, data, write_tags)?
+                let subtag = it.next().expect("truncated tuple tag");
+                max_alignment = core::cmp::max(max_alignment, subtag.alignment());
+                send_value(writer, subtag, data, write_tags)?;
             }
             *data = round_up_const(*data, max_alignment);
             Ok(())
@@ -290,8 +306,8 @@ unsafe fn send_value<W>(writer: &mut W, tag: Tag, data: &mut *const (), write_ta
             consume_value!(&List, |ptr| {
                 let length = (**ptr).length as usize;
                 writer.write_u32((**ptr).length)?;
-                let tag = it.clone().next().expect("truncated tag");
-                send_elements(writer, tag, length, (**ptr).elements, write_tags)
+                let subtag = it.clone().next().expect("truncated list tag");
+                send_elements(writer, subtag, length, (**ptr).elements, write_tags)
             })
         }
         Tag::Array(it, num_dims) => {
@@ -299,8 +315,7 @@ unsafe fn send_value<W>(writer: &mut W, tag: Tag, data: &mut *const (), write_ta
                 writer.write_u8(num_dims)?;
             }
             consume_value!(*const(), |buffer| {
-                let elt_tag = it.clone().next().expect("truncated tag");
-
+                let elt_tag = it.clone().next().expect("truncated array tag");
                 let mut total_len = 1;
                 for _ in 0..num_dims {
                     consume_value!(u32, |len| {
@@ -313,71 +328,115 @@ unsafe fn send_value<W>(writer: &mut W, tag: Tag, data: &mut *const (), write_ta
             })
         }
         Tag::Range(it) => {
-            let tag = it.clone().next().expect("truncated tag");
-            send_value(writer, tag, data, write_tags)?;
-            send_value(writer, tag, data, write_tags)?;
-            send_value(writer, tag, data, write_tags)?;
+            let subtag = it.clone().next().expect("truncated range tag");
+            send_value(writer, subtag, data, write_tags)?;
+            send_value(writer, subtag, data, write_tags)?;
+            send_value(writer, subtag, data, write_tags)?;
             Ok(())
         }
         Tag::Keyword(it) => {
+            // In legacy composite types, 'k' tags should never appear.
+            // However, if they do, we now provide a simple pass-through: write out
+            // the keyword name (a CSlice<u8>) and then the following value.
             #[repr(C)]
             struct Keyword<'a> { name: CSlice<'a, u8> }
             consume_value!(Keyword, |ptr| {
                 writer.write_string(str::from_utf8((*ptr).name.as_ref()).unwrap())?;
-                let tag = it.clone().next().expect("truncated tag");
-                let mut data = ptr.offset(1) as *const ();
-                send_value(writer, tag, &mut data, write_tags)
+                let subtag = it.clone().next().expect("truncated keyword tag");
+                let mut data2 = ptr.add(1) as *const ();
+                send_value(writer, subtag, &mut data2, write_tags)
             })
-            // Tag::Keyword never appears in composite types, so we don't have
-            // to accurately advance data.
         }
         Tag::Object => {
             #[repr(C)]
             struct Object { id: u32 }
-            consume_value!(*const Object, |ptr|
+            consume_value!( *const Object, |ptr|
                 writer.write_u32((**ptr).id))
         }
     }
 }
 
-pub fn send_args<W>(writer: &mut W, service: u32, tag_bytes: &[u8], data: *const *const (), write_tags: bool)
-                   -> Result<(), Error<W::WriteError>>
-    where W: Write + ?Sized
+pub fn send_args<W>(
+    writer: &mut W,
+    service: u32,
+    tag_bytes: &[u8],
+    data: *const *const (),
+    write_tags: bool,
+) -> Result<(), Error<W::WriteError>>
+where
+    W: Write + ?Sized,
 {
-    let (arg_tags_bytes, return_tag_bytes) = split_tag(tag_bytes);
+    // Use the original split_tag function.
+    let (arg_tags_bytes, return_tag_bytes) = {
+        // If extended functionality is desired, you could use split_tag_extended
+        // and log parameter names. For legacy compatibility, we keep the original split.
+        let sep = tag_bytes.iter().position(|&b| b == b':')
+            .expect("tag missing ':' separator");
+        let (arg_tags_bytes, rest) = tag_bytes.split_at(sep);
+        let return_tag_bytes = &rest[1..];
+        (arg_tags_bytes, return_tag_bytes)
+    };
 
     let mut args_it = TagIterator::new(arg_tags_bytes);
-    #[cfg(feature = "log")]
-    {
-        let return_it = TagIterator::new(return_tag_bytes);
-        debug!("send<{}>({})->{}", service, args_it, return_it);
-    }
-
     writer.write_u32(service)?;
     for index in 0.. {
         if let Some(arg_tag) = args_it.next() {
-            let mut data = unsafe { *data.offset(index) };
-            unsafe { send_value(writer, arg_tag, &mut data, write_tags)? };
+            let mut data_ptr = unsafe { *data.add(index) };
+            unsafe { send_value(writer, arg_tag, &mut data_ptr, write_tags)?; }
         } else {
-            break
+            break;
         }
     }
     writer.write_u8(0)?;
     writer.write_bytes(return_tag_bytes)?;
-
     Ok(())
 }
 
+//
+// The tag module with extended splitting and parameter name parsing.
+//
 mod tag {
     use core::fmt;
     use super::round_up;
 
+    /// Splits tag_bytes into three parts: argument type slice, parameter name slice, and return type slice.
+    pub fn split_tag_extended(tag_bytes: &[u8]) -> (&[u8], &[u8], &[u8]) {
+        let pipe_pos = tag_bytes.iter().position(|&b| b == b'|')
+            .expect("Missing '|' delimiter in extended tag format");
+        let colon_pos = tag_bytes.iter().position(|&b| b == b':')
+            .expect("Missing ':' delimiter in extended tag format");
+        assert!(colon_pos > pipe_pos, "Invalid tag format: ':' appears before '|'");
+        let param_type_slice = &tag_bytes[..pipe_pos];
+        let param_name_slice = &tag_bytes[pipe_pos + 1 .. colon_pos];
+        let return_type_slice = &tag_bytes[colon_pos + 1 ..];
+        (param_type_slice, param_name_slice, return_type_slice)
+    }
+
+    /// Parses parameter names from the parameter name slice.
+    /// Each parameter name is stored as [ length_byte, name_bytes... ].
+    pub fn parse_param_names(param_name_slice: &[u8]) -> Vec<String> {
+        let mut idx = 0;
+        let mut names = Vec::new();
+        while idx < param_name_slice.len() {
+            let len_byte = param_name_slice[idx];
+            idx += 1;
+            let length = len_byte as usize;
+            assert!(idx + length <= param_name_slice.len(), "Parameter names truncated");
+            let name_bytes = &param_name_slice[idx .. idx + length];
+            idx += length;
+            let name_str = core::str::from_utf8(name_bytes)
+                .expect("Invalid UTF-8 in parameter name")
+                .to_string();
+            names.push(name_str);
+        }
+        names
+    }
+
+    /// The original split_tag for backward compatibility (splits only at ':').
     pub fn split_tag(tag_bytes: &[u8]) -> (&[u8], &[u8]) {
-        let tag_separator =
-            tag_bytes.iter()
-                     .position(|&b| b == b':')
-                     .expect("tag without a return separator");
-        let (arg_tags_bytes, rest) = tag_bytes.split_at(tag_separator);
+        let pos = tag_bytes.iter().position(|&b| b == b':')
+            .expect("Tag missing ':' separator");
+        let (arg_tags_bytes, rest) = tag_bytes.split_at(pos);
         let return_tag_bytes = &rest[1..];
         (arg_tags_bytes, return_tag_bytes)
     }
@@ -397,30 +456,31 @@ mod tag {
         Array(TagIterator<'a>, u8),
         Range(TagIterator<'a>),
         Keyword(TagIterator<'a>),
-        Object
+        Object,
     }
 
     impl<'a> Tag<'a> {
         pub fn as_u8(self) -> u8 {
             match self {
-                Tag::None => b'n',
-                Tag::Bool => b'b',
-                Tag::Int32 => b'i',
-                Tag::Int64 => b'I',
-                Tag::Float64 => b'f',
-                Tag::String => b's',
-                Tag::Bytes => b'B',
+                Tag::None      => b'n',
+                Tag::Bool      => b'b',
+                Tag::Int32     => b'i',
+                Tag::Int64     => b'I',
+                Tag::Float64   => b'f',
+                Tag::String    => b's',
+                Tag::Bytes     => b'B',
                 Tag::ByteArray => b'A',
-                Tag::Tuple(_, _) => b't',
-                Tag::List(_) => b'l',
-                Tag::Array(_, _) => b'a',
-                Tag::Range(_) => b'r',
+                Tag::Tuple(_,_)=> b't',
+                Tag::List(_)   => b'l',
+                Tag::Array(_,_)=> b'a',
+                Tag::Range(_)  => b'r',
                 Tag::Keyword(_) => b'k',
-                Tag::Object => b'O',
+                Tag::Object    => b'O',
             }
         }
 
         pub fn alignment(self) -> usize {
+            // Using a CSlice as a placeholder type for compound types.
             use cslice::CSlice;
             match self {
                 Tag::None => 1,
@@ -428,27 +488,33 @@ mod tag {
                 Tag::Int32 => core::mem::align_of::<i32>(),
                 Tag::Int64 => core::mem::align_of::<i64>(),
                 Tag::Float64 => core::mem::align_of::<f64>(),
-                // struct type: align to largest element
-                Tag::Tuple(it, arity) => {
-                    let it = it.clone();
-                    it.take(arity.into()).map(|t| t.alignment()).max().unwrap()
-                },
-                Tag::Range(it) => {
-                    let it = it.clone();
-                    it.take(3).map(|t| t.alignment()).max().unwrap()
-                }
-                // the ptr/length(s) pair is basically CSlice
-                Tag::Bytes | Tag::String | Tag::ByteArray | Tag::List(_) | Tag::Array(_, _) =>
+                Tag::String | Tag::Bytes | Tag::ByteArray | Tag::List(_) | Tag::Array(_, _) =>
                     core::mem::align_of::<CSlice<()>>(),
-                Tag::Keyword(_) => unreachable!("Tag::Keyword should not appear in composite types"),
+                Tag::Tuple(it, arity) => {
+                    let mut max = 1;
+                    let mut itc = it.clone();
+                    for _ in 0..arity {
+                        if let Some(sub) = itc.next() {
+                            max = max.max(sub.alignment());
+                        }
+                    }
+                    max
+                }
+                Tag::Range(it) => {
+                    let mut max = 1;
+                    let mut itc = it.clone();
+                    for _ in 0..3 {
+                        if let Some(sub) = itc.next() {
+                            max = max.max(sub.alignment());
+                        }
+                    }
+                    max
+                }
+                Tag::Keyword(_) => unreachable!("Keyword tag should not appear in composite types"),
                 Tag::Object => core::mem::align_of::<u32>(),
             }
         }
 
-        /// Returns the "alignment size" of a value with the type described by the tag
-        /// (in bytes), i.e. the stride between successive elements in a list/array of
-        /// the given type, or the offset from a struct element of this type to the
-        /// next field.
         pub fn size(self) -> usize {
             match self {
                 Tag::None => 0,
@@ -456,55 +522,48 @@ mod tag {
                 Tag::Int32 => 4,
                 Tag::Int64 => 8,
                 Tag::Float64 => 8,
-                Tag::String => 8,
-                Tag::Bytes => 8,
-                Tag::ByteArray => 8,
-                Tag::Tuple(it, arity) => {
-                    let mut size = 0;
-                    let mut max_alignment = 0;
-                    let mut it = it.clone();
-                    for _ in 0..arity {
-                        let tag = it.next().expect("truncated tag");
-                        let alignment = tag.alignment();
-                        max_alignment = core::cmp::max(max_alignment, alignment);
-                        size = round_up(size, alignment);
-                        size += tag.size();
-                    }
-                    // Take into account any tail padding (if element(s) with largest
-                    // alignment are not at the end).
-                    size = round_up(size, max_alignment);
-                    size
-                }
+                Tag::String | Tag::Bytes | Tag::ByteArray => 8,
                 Tag::List(_) => 8,
                 Tag::Array(_, num_dims) => 4 * (1 + num_dims as usize),
-                Tag::Range(it) => {
-                    let tag = it.clone().next().expect("truncated tag");
-                    tag.size() * 3
+                Tag::Tuple(it, arity) => {
+                    let mut size = 0;
+                    let mut max_align = 1;
+                    let mut itc = it.clone();
+                    for _ in 0..arity {
+                        let sub = itc.next().expect("Tuple tag truncated");
+                        max_align = max_align.max(sub.alignment());
+                        size = round_up(size, sub.alignment());
+                        size += sub.size();
+                    }
+                    round_up(size, max_align)
                 }
-                Tag::Keyword(_) => unreachable!(),
-                Tag::Object => unreachable!(),
+                Tag::Range(it) => {
+                    let sub = it.clone().next().expect("Range tag truncated");
+                    sub.size() * 3
+                }
+                Tag::Keyword(_) => unreachable!("Keyword tag should not appear in composite types"),
+                Tag::Object => unreachable!("Object tag not supported"),
             }
         }
     }
 
     #[derive(Debug, Clone, Copy)]
     pub struct TagIterator<'a> {
-        pub data: &'a [u8]
+        pub data: &'a [u8],
     }
 
     impl<'a> TagIterator<'a> {
-        pub fn new(data: &'a [u8]) -> TagIterator<'a> {
-            TagIterator { data: data }
+        pub fn new(data: &'a [u8]) -> Self {
+            TagIterator { data }
         }
 
         pub fn next(&mut self) -> Option<Tag<'a>> {
-            if self.data.len() == 0 {
-                return None
+            if self.data.is_empty() {
+                return None;
             }
-
-            let tag_byte = self.data[0];
+            let b = self.data[0];
             self.data = &self.data[1..];
-            Some(match tag_byte {
+            Some(match b {
                 b'n' => Tag::None,
                 b'b' => Tag::Bool,
                 b'i' => Tag::Int32,
@@ -527,85 +586,57 @@ mod tag {
                 b'r' => Tag::Range(self.sub(1)),
                 b'k' => Tag::Keyword(self.sub(1)),
                 b'O' => Tag::Object,
-                _    => unreachable!()
+                _    => unreachable!("Unknown tag byte"),
             })
         }
 
         fn sub(&mut self, count: u8) -> TagIterator<'a> {
-            let data = self.data;
+            let start = self.data;
             for _ in 0..count {
-                self.next().expect("truncated tag");
+                self.next().expect("Truncated sub-tag block");
             }
-            TagIterator { data: &data[..(data.len() - self.data.len())] }
+            let used = start.len() - self.data.len();
+            TagIterator { data: &start[..used] }
         }
     }
 
     impl<'a> Iterator for TagIterator<'a> {
         type Item = Tag<'a>;
         fn next(&mut self) -> Option<Self::Item> {
-            (self as &mut TagIterator<'a>).next()
+            self.next()
         }
     }
 
     impl<'a> fmt::Display for TagIterator<'a> {
         fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-            let mut it = self.clone();
+            let mut it = *self;
             let mut first = true;
             while let Some(tag) = it.next() {
-                if first {
-                    first = false
+                if !first {
+                    write!(f, ", ")?;
                 } else {
-                    write!(f, ", ")?
+                    first = false;
                 }
-
                 match tag {
-                    Tag::None =>
-                        write!(f, "None")?,
-                    Tag::Bool =>
-                        write!(f, "Bool")?,
-                    Tag::Int32 =>
-                        write!(f, "Int32")?,
-                    Tag::Int64 =>
-                        write!(f, "Int64")?,
-                    Tag::Float64 =>
-                        write!(f, "Float64")?,
-                    Tag::String =>
-                        write!(f, "String")?,
-                    Tag::Bytes =>
-                        write!(f, "Bytes")?,
-                    Tag::ByteArray =>
-                        write!(f, "ByteArray")?,
-                    Tag::Tuple(it, _) => {
-                        write!(f, "Tuple(")?;
-                        it.fmt(f)?;
-                        write!(f, ")")?;
-                    }
-                    Tag::List(it) => {
-                        write!(f, "List(")?;
-                        it.fmt(f)?;
-                        write!(f, ")")?;
-                    }
-                    Tag::Array(it, num_dims) => {
-                        write!(f, "Array(")?;
-                        it.fmt(f)?;
-                        write!(f, ", {})", num_dims)?;
-                    }
-                    Tag::Range(it) => {
-                        write!(f, "Range(")?;
-                        it.fmt(f)?;
-                        write!(f, ")")?;
-                    }
-                    Tag::Keyword(it) => {
-                        write!(f, "Keyword(")?;
-                        it.fmt(f)?;
-                        write!(f, ")")?;
-                    }
-                    Tag::Object =>
-                        write!(f, "Object")?,
+                    Tag::None => write!(f, "None")?,
+                    Tag::Bool => write!(f, "Bool")?,
+                    Tag::Int32 => write!(f, "Int32")?,
+                    Tag::Int64 => write!(f, "Int64")?,
+                    Tag::Float64 => write!(f, "Float64")?,
+                    Tag::String => write!(f, "String")?,
+                    Tag::Bytes => write!(f, "Bytes")?,
+                    Tag::ByteArray => write!(f, "ByteArray")?,
+                    Tag::Tuple(_, cnt) => write!(f, "Tuple({})", cnt)?,
+                    Tag::List(_) => write!(f, "List")?,
+                    Tag::Array(_, n) => write!(f, "Array({})", n)?,
+                    Tag::Range(_) => write!(f, "Range")?,
+                    Tag::Keyword(_) => write!(f, "Keyword")?,
+                    Tag::Object => write!(f, "Object")?,
                 }
             }
-
             Ok(())
         }
     }
 }
+
+pub use self::tag::{Tag, TagIterator, split_tag_extended, parse_param_names};

artiq/test/coredevice/test_portability.py

@@ -9,9 +9,7 @@ from artiq.test.hardware_testbench import ExperimentCase
 def _run_on_host(k_class, *args, **kwargs):
     device_mgr = dict()
     device_mgr["core"] = sim_devices.Core(device_mgr)
-
-    k_inst = k_class((device_mgr, None, None, {}),
-                     *args, **kwargs)
+    k_inst = k_class((device_mgr, None, None, {}), *args, **kwargs)
     k_inst.run()
     return k_inst
 
@@ -30,7 +28,7 @@ class _Primes(EnvExperiment):
         for x in range(1, self.maximum):
             d = 2
             prime = True
-            while d*d <= x:
+            while d * d <= x:
                 if x % d == 0:
                     prime = False
                     break
@@ -58,7 +56,7 @@ class _Misc(EnvExperiment):
 
         self.input = 84
         self.al = [1, 2, 3, 4, 5]
-        self.list_copy_in = [2*Hz, 10*MHz]
+        self.list_copy_in = [2 * Hz, 10 * MHz]
 
         self.half_input = 0
         self.acc = 0
@@ -66,7 +64,7 @@ class _Misc(EnvExperiment):
 
     @kernel
     def run(self):
-        self.half_input = self.input//2
+        self.half_input = self.input // 2
         self.acc = 0
         for i in range(len(self.al)):
             self.acc += self.al[i]
@@ -83,7 +81,7 @@ class _PulseLogger(EnvExperiment):
         if not hasattr(self.parent_test, "first_timestamp"):
             self.parent_test.first_timestamp = t
         origin = self.parent_test.first_timestamp
-        t_usec = round(self.core.mu_to_seconds(t-origin)*1000000)
+        t_usec = round(self.core.mu_to_seconds(t - origin) * 1000000)
         self.parent_test.output_list.append((self.name, t_usec, l, f))
 
     def on(self, t, f):
@@ -105,9 +103,7 @@ class _Pulses(EnvExperiment):
         self.output_list = output_list
 
         for name in "a", "b", "c", "d":
-            pl = _PulseLogger(self,
-                              parent_test=self,
-                              name=name)
+            pl = _PulseLogger(self, parent_test=self, name=name)
             setattr(self, name, pl)
 
     @kernel
@@ -115,16 +111,17 @@ class _Pulses(EnvExperiment):
         for i in range(3):
             with parallel:
                 with sequential:
-                    self.a.pulse(100+i, 20*us)
-                    self.b.pulse(200+i, 20*us)
+                    self.a.pulse(100 + i, 20 * us)
+                    self.b.pulse(200 + i, 20 * us)
                 with sequential:
-                    self.c.pulse(300+i, 10*us)
-                    self.d.pulse(400+i, 20*us)
+                    self.c.pulse(300 + i, 10 * us)
+                    self.d.pulse(400 + i, 20 * us)
 
 
 class _MyException(Exception):
     pass
 
+
 class _NestedFinally(EnvExperiment):
     def build(self, trace):
         self.setattr_device("core")
@@ -148,6 +145,7 @@ class _NestedFinally(EnvExperiment):
         finally:
             self._trace(2)
 
+
 class _NestedExceptions(EnvExperiment):
     def build(self, trace):
         self.setattr_device("core")
@@ -177,6 +175,7 @@ class _NestedExceptions(EnvExperiment):
         finally:
             self._trace(4)
 
+
 class _Exceptions(EnvExperiment):
     def build(self, trace):
         self.setattr_device("core")
@@ -264,6 +263,27 @@ class _Keywords(EnvExperiment):
 
     @kernel
     def run(self):
+        # This RPC call uses a keyword argument.
+        self.rpc(kw=self.value)
+
+
+# New experiment that demonstrates the issue with keyword RPC calls.
+# The legacy firmware RPC code encodes keyword arguments with a 'k' tag
+# but does not implement a read path for them. Therefore, if a keyword argument
+# is passed, the firmware will panic.
+class _KeywordsFailure(EnvExperiment):
+    def build(self, value, output):
+        self.setattr_device("core")
+        self.value = value
+        self.output = output
+
+    def rpc(self, kw):
+        self.output.append(kw)
+
+    @kernel
+    def run(self):
+        # This call should trigger the 'k' tag in the legacy RPC code,
+        # which is not handled on the firmware side.
         self.rpc(kw=self.value)
 
 
@@ -336,4 +356,4 @@ class HostVsDeviceCase(ExperimentCase):
             self.assertEqual(output, [False])
             output = []
             f(_Keywords, value=True, output=output)
-            self.assertEqual(output, [True])
+            self.assertEqual(output, [True])