Avoid creating transparent wrappers instead of peeling them off

humility-core/src/reflect.rs

@@ -879,6 +879,16 @@ pub fn load_struct_or_tuple(
     ty: &HubrisStruct,
     addr: usize,
 ) -> Result<Value> {
+    // 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 {
-        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<u32>`.
-        let v = strukt(
-            "ManuallyDrop<MaybeDangling<u32>>",
-            ("value", tuple("MaybeDangling<u32>", 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<u32>", ("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<u32>", ("value", payload()));
-        let Value::Struct(s) = peel_transparent(v) else {
-            panic!("lookalike type was peeled");
-        };
-        assert_eq!(s.name(), "ManuallyDropGuard<u32>");
-    }
-
-    #[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<u32>".to_string(),
-            members,
-        });
-        assert!(matches!(peel_transparent(v), Value::Struct(..)));
-    }
-}