Select the smallest possible index type

[tcbrindle]

Previously, the index type of the variant was hard-coded to unsigned int. This patch instead selects the index type as the smallest unsigned integer type which can accommodate the number of alternatives, plus one for the valueless_by_exception state.

For example, in the common case of fewer than 255 alternatives, the index type is selected to be unsigned char; between 256 and 65535 alternatives, unsigned short is used; otherwise (in theory at least) unsigned int is used.

This results in a smaller variant (and better cache usage etc) whenever max(sizeof(Ts)...) < sizeof(unsigned).

Before:

sizeof(variant<monostate, bool, unsigned char, char, signed char>) == 8

After:

sizeof(variant<monostate, bool, unsigned char, char, signed char>) == 2

[SaadAttieh]

Ha tbh I thought the lib was already doing this. As a user, I would like to see this as I claim that the majority of variants will have fewer than 255 elements.

Regards, Saad Sent from my iPhone (excuse typos)

On 6 Feb 2019, at 4:37 pm, Tristan Brindle notifications@github.com wrote:

Previously, the index type of the variant was hard-coded to unsigned int. This patch instead selects the index type as the smallest unsigned integer type which can accommodate the number of alternatives, plus one for the valueless_by_exception state.

For example, in the common case of fewer than 255 alternatives, the index type is selected to be unsigned char; between 256 and 65535 alternatives, unsigned short is used; otherwise (in theory at least) unsigned int is used.

This results in a smaller variant (and better cache usage etc) whenever max(sizeof(Ts)...) < sizeof(unsigned).

Before:

sizeof(variant<monostate, bool, unsigned char, char, signed char>) == 8

After:

sizeof(variant<monostate, bool, unsigned char, char, signed char>) == 2

You can view, comment on, or merge this pull request online at:

https://github.com/mpark/variant/pull/58

Commit Summary

Select the smallest possible index type File Changes

M include/mpark/variant.hpp (21) Patch Links:

https://github.com/mpark/variant/pull/58.patch https://github.com/mpark/variant/pull/58.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[mpark]

Thanks for the patch @tcbrindle! I had helped review this change for libc++'s variant but forgot to implement it here 😅

[phlptp]

This change fails to compile on MSVC when some other windows headers are included. The max symbol is unfortunately defined as a macro, this also interferes with std::max. Short of globally defining -DNOMINMAX which can cause some other issues. The solution is to wrap the max call here in parenthesis

using index_t = typename std::conditional<
            sizeof...(Ts) < (std::numeric_limits<unsigned char>::max)(),
            unsigned char,
            typename std::conditional<
                sizeof...(Ts) < (std::numeric_limits<unsigned short>::max)(),
                unsigned short,
                unsigned int>::type
            >::type;

then this compiles fine on MSVC. I would submit a PR but It is a somewhat annoying process to get permission to do that at my organization.

[mpark]

Thanks. Fixed in 2744d0ad