Skip to content

Conversation

@Fuuzetsu
Copy link
Contributor

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<T>; 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.

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<T>;
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.
@mahkoh
Copy link
Owner

mahkoh commented Oct 2, 2025

I think a problem is that this code implicitly assumes that into_values produces values in the same order in which they are consumed by the static_map macro. I don't know if I'm making this assumption anywhere else or if there is a test verifying this.

On the other hand, with map_values this question does not arise.

@Fuuzetsu
Copy link
Contributor Author

Fuuzetsu commented Oct 3, 2025

I think it would be extremely surprising if this assumption wasn't true but OK.

The better solution is probably to implement try_map_values that allows us to do it in one pass: just like there's map and try_map for arrays in stdlib (though latter is unstable).

@mahkoh
Copy link
Owner

mahkoh commented Oct 3, 2025

try_map_values sounds like a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants