libstdc++: Fix handling of const types in std::variant [PR102912]
authorJonathan Wakely <jwakely@redhat.com>
Wed, 3 Nov 2021 13:51:45 +0000 (13:51 +0000)
committerJonathan Wakely <jwakely@redhat.com>
Thu, 4 Nov 2021 09:36:09 +0000 (09:36 +0000)
commit7551a9957437f20254be41d396962b9ccc46cee6
tree05193da8c50149f1d6c44cb141a75e2c3c364532
parentfa62db42b99e9eb1c067d2171bc437b3394e4d5d
libstdc++: Fix handling of const types in std::variant [PR102912]

Prior to r12-4447 (implementing P2231R1 constexpr changes) we didn't
construct the correct member of the union in __variant_construct_single,
we just plopped an object in the memory occupied by the union:

  void* __storage = std::addressof(__lhs._M_u);
  using _Type = remove_reference_t<decltype(__rhs_mem)>;
  ::new (__storage) _Type(std::forward<decltype(__rhs_mem)>(__rhs_mem));

We didn't care whether we had variant<int, const int>, we would just
place an int (or const int) into the storage, and then set the _M_index
to say which one it was.

In the new constexpr-friendly code we use std::construct_at to construct
the union object, which constructs the active member of the right type.
But now we need to know exactly the right type. We have to distinguish
between alternatives of type int and const int, and we have to be able
to find a const int (or const std::string, as in the OP) among the
alternatives. So my change from remove_reference_t<decltype(__rhs_mem)>
to remove_cvref_t<_Up> was wrong. It strips the const from const int,
and then we can't find the index of the const int alternative.

But just using remove_reference_t doesn't work either. When the copy
assignment operator of std::variant<int> uses __variant_construct_single
it passes a const int& as __rhs_mem, but if we don't strip the const
then we try to find const int among the alternatives, and *that* fails.
Similarly for the copy constructor, which also uses a const int& as the
initializer for a non-const int alternative.

The root cause of the problem is that __variant_construct_single doesn't
know the index of the type it's supposed to construct, and the new
_Variant_storage::__index_of<_Type> helper doesn't work if __rhs_mem and
the alternative being constructed have different const-qualification. We
need to replace __variant_construct_single with something that knows the
index of the alternative being constructed. All uses of that function do
actually know the index, but that context is lost by the time we call
__variant_construct_single. This patch replaces that function and
__variant_construct, inlining their effects directly into the callers.

libstdc++-v3/ChangeLog:

PR libstdc++/102912
* include/std/variant (_Variant_storage::__index_of): Remove.
(__variant_construct_single): Remove.
(__variant_construct): Remove.
(_Copy_ctor_base::_Copy_ctor_base(const _Copy_ctor_base&)): Do
construction directly instead of using __variant_construct.
(_Move_ctor_base::_Move_ctor_base(_Move_ctor_base&&)): Likewise.
(_Move_ctor_base::_M_destructive_move()): Remove.
(_Move_ctor_base::_M_destructive_copy()): Remove.
(_Copy_assign_base::operator=(const _Copy_assign_base&)): Do
construction directly instead of using _M_destructive_copy.
(variant::swap): Do construction directly instead of using
_M_destructive_move.
* testsuite/20_util/variant/102912.cc: New test.
libstdc++-v3/include/std/variant
libstdc++-v3/testsuite/20_util/variant/102912.cc [new file with mode: 0644]