From 192bdb3dac7ce8d13692b675d8d10e6bcfc9112b Mon Sep 17 00:00:00 2001 From: Mateusz Kowalczyk Date: Fri, 14 Feb 2025 11:24:53 +0900 Subject: [PATCH] prefer static_map! over explicit unsafe in serde deserialisation Instead of going over all the entries and checking if they are initialised and then going over them again and unwrapping unsafely, we can just go over it once and report the error on first failure. The semantics should be exactly the same but we no longer have an explicit `unsafe` in here: it's moved behind the `static_map!` macro instead which can be considered more battle-hardened. If we're accepting unsafe then it might make more sense to do something like `[MaybeUninit; L::LENGTH]` though something like the `Builder` is needed to make this ergonomic. Either way, I think the version in this commit at least hides the magic. PS: I checked if we can just replace `.unwrap_unchecked()` with `.unwrap()` but it seems that rustc is not able to notice that it will never fail so it generates panic code anyway. --- linearize/src/foreign/serde_1.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/linearize/src/foreign/serde_1.rs b/linearize/src/foreign/serde_1.rs index 59d2096..29345da 100644 --- a/linearize/src/foreign/serde_1.rs +++ b/linearize/src/foreign/serde_1.rs @@ -8,7 +8,7 @@ mod default { use { - crate::{Linearize, LinearizeExt, StaticCopyMap, StaticMap}, + crate::{Linearize, StaticCopyMap, StaticMap}, core::{ fmt::{Debug, Display, Formatter}, marker::PhantomData, @@ -74,15 +74,11 @@ mod default { while let Some((k, v)) = map.next_entry::()? { res[k] = Some(v); } - for (idx, v) in res.deref().iter().enumerate() { - if v.is_none() { - return Err(Error::custom(MissingKey(L::from_linear(idx).unwrap()))); - } - } - Ok(res.map_values(|v| unsafe { - // SAFETY: We just checked that v is Some. - v.unwrap_unchecked() - })) + + let mut res = res.into_values(); + Ok(crate::static_map! { k => { + res.next().flatten().ok_or(Error::custom(MissingKey(k)))? + }}) } }