[v3 PATCH] PR libstdc++/87619

[Jonathan Wakely]

On 16/10/18 22:59 +0300, Ville Voutilainen wrote:
>Simple, short, and sweet, just a thinko.
>
>2018-10-16  Ville Voutilainen  <ville.voutilainen@gmail.com>
>
>    PR libstdc++/87619
>    * include/std/variant (__select_index): Fix an off-by-one.
>    * testsuite/20_util/variant/87619.cc: New.

>diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
>index 2d86a70..55b6440 100644
>--- a/libstdc++-v3/include/std/variant
>+++ b/libstdc++-v3/include/std/variant
>@@ -362,7 +362,7 @@ namespace __variant
> 
>   template <typename... _Types>
>   using __select_index =
>-    typename __select_int::_Select_int_base<sizeof...(_Types) + 1,
>+    typename __select_int::_Select_int_base<sizeof...(_Types),
> 					    unsigned char,
> 					    unsigned short>::type::value_type;
> 
>diff --git a/libstdc++-v3/testsuite/20_util/variant/87619.cc b/libstdc++-v3/testsuite/20_util/variant/87619.cc
>new file mode 100644
>index 0000000..a8db6b8
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/20_util/variant/87619.cc
>@@ -0,0 +1,46 @@
>+// Copyright (C) 2018 Free Software Foundation, Inc.
>+//
>+// This file is part of the GNU ISO C++ Library.  This library is free
>+// software; you can redistribute it and/or modify it under the
>+// terms of the GNU General Public License as published by the
>+// Free Software Foundation; either version 3, or (at your option)
>+// any later version.
>+
>+// This library is distributed in the hope that it will be useful,
>+// but WITHOUT ANY WARRANTY; without even the implied warranty of
>+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+// GNU General Public License for more details.
>+
>+// You should have received a copy of the GNU General Public License along
>+// with this library; see the file COPYING3.  If not see
>+// <http://www.gnu.org/licenses/>.
>+
>+// { dg-options "-std=gnu++17" }
>+// { dg-do compile { target c++17 } }
>+
>+#include <variant>
>+#include <utility>
>+
>+
>+template<std::size_t I>
>+struct S {
>+};
>+
>+template <std::size_t... Is>
>+void f_impl(std::index_sequence<Is...>)
>+{
>+	using V = std::variant<S<Is>...>;
>+	static_assert(sizeof(V) == 2);

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?

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.

>+}
>+
>+template <std::size_t I>
>+void f()
>+{
>+	f_impl(std::make_index_sequence<I>{});
>+}
>+
>+int main()
>+{
>+	f<254>();
>+	f<255>();
>+}