Our sorted_vector_map implementation ignores two important parameters: Allocator which details the type of pair to allocate and Container which details the container to use.
Ignoring the type of Allocator means we always assume std::pair<Key, Value> equivalent packing for each element in the map. This is provably wrong as there are examples of using a packed structure for this - these are probably the only two uses though unless you wanted to do something weird with cache alignment. This is a relatively simple fix as we can use
std::allocator_traits<Allocator>::value_type to get the appropriate pair and otherwise keep our logic the same.
The underlying Container is more complicated to fix. It should be guaranteed by codegen that the handler for the underlying Container is available. In other cases we hand off the work to this other container's handler by adding an extra layer of indirection. This is not possible here as we need to change the output format of that container from a list to a map, which is especially important when considering key capture.
It's possible that we could cheat here and specialise list containers for ThisList<std::pair<T1, T2>> to have key capture. We'd end up with an extra layer of indirection in the sorted_vector_map but that's probably okay - it's explicitly a wrapping structure.
JakeHillion
commented
6 months ago
Our sorted_vector_map implementation ignores two important parameters:
Allocatorwhich details the type of pair to allocate andContainerwhich details the container to use.Ignoring the type of
Allocatormeans we always assumestd::pair<Key, Value>equivalent packing for each element in the map. This is provably wrong as there are examples of using a packed structure for this - these are probably the only two uses though unless you wanted to do something weird with cache alignment. This is a relatively simple fix as we can usestd::allocator_traits<Allocator>::value_typeto get the appropriate pair and otherwise keep our logic the same.The underlying
Containeris more complicated to fix. It should be guaranteed by codegen that the handler for the underlying Container is available. In other cases we hand off the work to this other container's handler by adding an extra layer of indirection. This is not possible here as we need to change the output format of that container from a list to a map, which is especially important when considering key capture.It's possible that we could cheat here and specialise list containers for
ThisList<std::pair<T1, T2>>to have key capture. We'd end up with an extra layer of indirection in the sorted_vector_map but that's probably okay - it's explicitly a wrapping structure.