diff --git a/humility-core/src/reflect.rs b/humility-core/src/reflect.rs index 96350889a..44e592326 100644 --- a/humility-core/src/reflect.rs +++ b/humility-core/src/reflect.rs @@ -879,6 +879,16 @@ pub fn load_struct_or_tuple( ty: &HubrisStruct, addr: usize, ) -> Result { + // Skip transparent wrappers with single members for either structs or + // tuples. + if let [m] = ty.members.as_slice() + && is_transparent_wrapper(&ty.name) + { + let maddr = addr + m.offset; + let mty = hubris.lookup_type(m.goff)?; + return load_value(hubris, buf, mty, maddr); + } + if ty.probably_a_tuple() { // Start off the tuple filled with nonsense, so that we can set the // values out of order if required. Do tuple members ever appear out of @@ -964,25 +974,6 @@ fn is_transparent_wrapper(name: &str) -> bool { TRANSPARENT_WRAPPERS.contains(&stem) } -/// Strips any [`TRANSPARENT_WRAPPERS`] from around `v`, returning the payload. -fn peel_transparent(mut v: Value) -> Value { - loop { - v = match v { - Value::Struct(s) - if s.len() == 1 && is_transparent_wrapper(s.name()) => - { - s.members.into_values().next().unwrap() - } - Value::Tuple(t) - if t.len() == 1 && is_transparent_wrapper(t.name()) => - { - t.1.into_iter().next().unwrap() - } - v => return v, - }; - } -} - /// Loads a union from memory image `buf` at offset `addr`. /// /// We assume that the only union types in our image are using @@ -1010,14 +1001,14 @@ pub fn load_union( } for v in ty.variants.iter() { if v.name == "value" { - let value = peel_transparent(load_value( + let value = load_value( hubris, buf, hubris.lookup_type( v.goff.ok_or_else(|| anyhow!("Missing goff in union"))?, )?, addr, - )?); + )?; let mut members = IndexMap::new(); members.insert("value".to_string(), value); @@ -1447,75 +1438,3 @@ fn deserialize_base<'a>( }; Ok((v, buf)) } - -#[cfg(test)] -mod tests { - use super::*; - - fn tuple(name: &str, values: Vec) -> Value { - Value::Tuple(Tuple(name.to_string(), values)) - } - - fn strukt(name: &str, member: (&str, Value)) -> Value { - let mut members = IndexMap::new(); - members.insert(member.0.to_string(), member.1); - Value::Struct(Struct { name: name.to_string(), members }) - } - - fn payload() -> Value { - Value::Base(Base::U32(42)) - } - - #[test] - fn peels_nested_wrappers() { - // What newer toolchains produce for `MaybeUninit`. - let v = strukt( - "ManuallyDrop>", - ("value", tuple("MaybeDangling", vec![payload()])), - ); - assert!(matches!(peel_transparent(v), Value::Base(Base::U32(42)))); - } - - #[test] - fn peels_manually_drop_alone() { - // What older toolchains produce. - let v = strukt("ManuallyDrop", ("value", payload())); - assert!(matches!(peel_transparent(v), Value::Base(Base::U32(42)))); - } - - #[test] - fn leaves_ordinary_values_alone() { - assert!(matches!(peel_transparent(payload()), Value::Base(..))); - - // A 1-element tuple that isn't a compiler wrapper stays put. - let v = tuple("Foo", vec![payload()]); - let Value::Tuple(t) = peel_transparent(v) else { - panic!("newtype was peeled"); - }; - assert_eq!(t.name(), "Foo"); - } - - #[test] - fn leaves_lookalike_wrappers_alone() { - // A hubris type whose name merely starts with a wrapper's is not one. - let v = strukt("ManuallyDropGuard", ("value", payload())); - let Value::Struct(s) = peel_transparent(v) else { - panic!("lookalike type was peeled"); - }; - assert_eq!(s.name(), "ManuallyDropGuard"); - } - - #[test] - fn leaves_multi_field_wrappers_alone() { - // Were a wrapper to gain a second field, peeling it would be a guess: - // leave it for a human rather than pick a field. - let mut members = IndexMap::new(); - members.insert("value".to_string(), payload()); - members.insert("other".to_string(), payload()); - let v = Value::Struct(Struct { - name: "ManuallyDrop".to_string(), - members, - }); - assert!(matches!(peel_transparent(v), Value::Struct(..))); - } -}