[v3 PATCH] PR libstdc++/87619

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[v3 PATCH] PR libstdc++/87619

Ville Voutilainen
Simple, short, and sweet, just a thinko.

2018-10-16  Ville Voutilainen  <[hidden email]>

    PR libstdc++/87619
    * include/std/variant (__select_index): Fix an off-by-one.
    * testsuite/20_util/variant/87619.cc: New.

pr87619.diff (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [v3 PATCH] PR libstdc++/87619

Jonathan Wakely-3
On 17/10/18 15:06 +0300, Ville Voutilainen wrote:

>On Wed, 17 Oct 2018 at 12:20, Ville Voutilainen
><[hidden email]> wrote:
>>
>> On Wed, 17 Oct 2018 at 12:07, Jonathan Wakely <[hidden email]> wrote:
>> > It might be worth also calling f<256>() and doing:
>> >
>> > template <std::size_t... Is>
>> > void f_impl(std::index_sequence<Is...> is)
>> > {
>> >   using V = std::variant<S<Is>...>;
>> >
>> >   // For a variant of 255 alternatives the valid indices are [0,254]
>> >   // and index 255 means valueless-by-exception, so fits in one byte.
>> >   if constexpr (std::variant_size_v<V> <= 255)
>> >     static_assert(sizeof(V) == 2)
>> >   else
>> >     static_assert(sizeof(V) > 2)
>> > }
>> >
>> > Just to check we don't introduce an off-by-one error in the *other*
>> > direction in future. What do you think?
>>
>> Agreed. I'll patch that in.
>>
>> > To be really portable we would use numeric_limits<unsigned char>::max()
>> > but we don't need to worry about non-8-bit char in our implementation.
>>
>> I can change that as well so that we don't need to re-think it every time.
>
>Here.

Great, thanks for the changes - OK for trunk.