Feature request: easier creation of etl::const_maps

[relativityspace-jsmith]

Maybe I'm just missing something, but while I love the idea and capabilities of etl::const_map, I really struggle to actually create them in code.

Like, consider the following toy example, where I want to map integers to std::chrono times. So far, the most painless way I've found to do this is:

using DelayTimeMapEntry = std::pair<const uint8_t, const std::chrono::nanoseconds>;
using namespace std::chrono_literals;
constexpr etl::const_map<uint8_t, const std::chrono::nanoseconds, 5> DELAY_TIMES{
    DelayTimeMapEntry{1, 100ns},
    DelayTimeMapEntry{2, 1us},
    DelayTimeMapEntry{3, 5us},
    DelayTimeMapEntry{4, 20us},
    DelayTimeMapEntry{5, 100us},
};

This works, but there are some pretty major rough edges:

• You have to manually construct each element in the map with the full pair type (I used a typedef here to make this less painless, but still)
• You have to repeat the element types between the pair and the map declaration
• You have to manually count the number of elements and put it as the third template argument to the const_map
• If you forget const on either of the template arguments to the pair, compilation fails with a very unhelpful error:

  etl/include/etl/const_map.h:511:81: error: static assertion failed: All elements must be value_type
  511 |       static_assert((etl::are_all_same<value_type, etl::decay_t<TElements>...>::value), "All elements must be value_type");
         |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
  etl/include/etl/const_map.h:511:81: note: 'std::integral_constant<bool, false>::value' evaluates to false
  

  Maybe this is just me but it feels unintuitive that the pair elements themselves need to be const. Of course they will be const when I get them out because they are going into a const container, but why should the constructor care if the input is const?

This means that anytime I want to create a const_map I have to pull up my existing code and very carefully base my new map on it, staying clear of all these gotchas. It's especially bad since this is the only possible way to create a const_map -- it's not like you can create them element-by-element!

Personally I'd really love if we could get a helper function to create them more easily, maybe something like etl::make_const_map. So that way you could just do:

constexpr auto DELAY_TIMES = etl::make_const_map<uint8_t, std::chrono::nanoseconds>(
    {1, 100ns},
    {2, 1us},
    {3, 5us},
    {4, 20us},
    {5, 100us},
);

[relativityspace-jsmith]

Actually I just noticed an even more serious usability problem: const_map works by doing a binary search of the passed elements, and it doesn't re-sort the elements when constructed. So if you happen to construct one with elements that are not already in sorted order, find() will fail to find a key that exists in the map for no obvious reason. As far as I can tell this doesn't seem to be documented anywhere. Any way we could get a static_assert to guard against this?

@jwellbelove ^

[jwellbelove]

Any way we could get a static_assert to guard against this?

I'm not sure, I'd have to experiment.
I have managed to modify the message router class to internally sort the message types to allow a binary sort, but those are template parameter types.
Maybe there is some way to do this for constructor arguments in a constexpr class.

It would be better to raise this discussion as an issue.

[relativityspace-jsmith]

Created #1341

[mike919192]

Hi @relativityspace-jsmith . Thanks, I wasn't aware of the sorting issue so that is good to keep in mind for the future.

I have been using const_map in a way that has a little bit less repetition than your example. See this example:

struct acq_value {
    u8 div1{ 128 };
    u8 div2{ 128 };
    u16 clock{ 1 };
    u8 clock_div{ 1 };
    bool phase_reverse{ false };
};

using acq_map_value_t = etl::const_map<int, acq_value, 0>::value_type;

static constexpr auto acq_table = etl::const_map(
    acq_map_value_t{ 25'000, acq_value{ .div1 = 64, .div2 = 64 } }, 
    acq_map_value_t{ 75'000, acq_value{ .clock = 2 } }, 
    acq_map_value_t{ 225'000, acq_value{ .clock = 6, .phase_reverse = true } },
    acq_map_value_t{ 375'000, acq_value{ .clock = 10 } }, 
    acq_map_value_t{ 675'000, acq_value{ .clock = 18 } }, 
    acq_map_value_t{ 975'000, acq_value{ .clock = 26 } }, 
    acq_map_value_t{ 1'275'000, acq_value{ .clock = 34 } }, 
    acq_map_value_t{ 1'575'000, acq_value{ .clock = 42 } }, 
    acq_map_value_t{ 2'500'000, acq_value{ .clock = 66 } }, 
    acq_map_value_t{ 3'675'000, acq_value{ .clock = 98, .clock_div = 2 } }, 
    acq_map_value_t{ 6'000'000, acq_value{ .clock = 162, .clock_div = 2 } }
);

One small nit is that I use Size parameter = 0 to get the value_type, but the actual map deduces the actual size.

[relativityspace-jsmith]

Nice, that's a cool way to do it!