Proposal: Change iterator implementations to return borrowed keys

[Themayu]

Or: "Do the iterators really need to copy the keys by default?"

I'm having a bit of trouble creating an implementation for bevy_reflect::Map on a wrapper type around your fixed_map::Map type. The problem stems from the requirement that bevy_reflect::Map::get_at and bevy_reflect::Map::get_at_mut return the key-value pairs as borrowed dyn Reflect trait objects. While this is easy for the values (since they are borrowed anyway), there comes a problem with the keys. Because the keys are given from your iter() and iter_mut() iterators as owned values, this makes it impossible to meet the requirement for the trait.

Due to how trivial it is to acquire an owned key if necessary for a user's use-case (since the keys must implement Copy, simply doing *key in the iterator adaptor is good enough), I propose that the iterator implementations instead return borrows of the keys and leave it to the user to copy them if necessary.

[udoprog]

I don't mind. I can't remember why they're copy, so if you want to give it a stab, please document in case you come up against any problems.

[Themayu]

Looking at the way fixed_map::Key gets expanded in both the simple (unit variants) and complex (composite keys) cases, there's a problem that needs to be resolved in that currently, keys aren't stored at all in the MapStorage. This presents a problem because it means there's nowhere for the keys to be borrowed from.

In the simple case, we could store the list of keys somewhere in the storage to get around this, with the trade-off of making __MapStorage larger as a result. In the complex case, I'm not sure what we would do.

EDIT: Oops. Pressed send slightly too early.

[udoprog]

Ah yeah. That's clearly also an issue for SetStorage, since for simple enums it's implemented as a boolean array. Or as a bitset if #[key(bitset)] is specified.

Tricky! I'm not sure how to construct compound keys like for OptionStorage without resorting to more macros :thinking:

[Themayu]

For compound keys I was thinking of delegating the key storage to the innermost layer that each variant combination can reach. Though with long chains, that can risk massively bloating up the storage space of the innermost layer.