[Bug libstdc++/102912] New: Not full support of const arguments in std::variant

[Bug libstdc++/102912] New: Not full support of const arguments in std::variant

[fchelnokov at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102912

            Bug ID: 102912
           Summary: Not full support of const arguments in std::variant
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: fchelnokov at gmail dot com
  Target Milestone: ---

This program is valid:
```
#include <string>
#include <variant>

using S = std::variant<const int, const std::string>;

int main() {
    S s(1);
    S u = s;

    S v("abc");
    S w = v;
}
```
but its compilation in GCC results in a long error:
```
In file included from <source>:2:
/opt/compiler-explorer/gcc-trunk-20211024/include/c++/12.0.0/variant: In
instantiation of 'constexpr std::__detail::__variant::_Variadic_union<_First,
_Rest ...>::_Variadic_union(std::in_place_index_t<_Np>, _Args&& ...) [with long
unsigned int _Np = 1; _Args = {const int&}; _First = const
std::__cxx11::basic_string<char>; _Rest = {}]':
/opt/compiler-explorer/gcc-trunk-20211024/include/c++/12.0.0/variant:409:4:  
required from 'constexpr std::__detail::__variant::_Variadic_union<_First,
_Rest ...>::_Variadic_union(std::in_place_index_t<_Np>, _Args&& ...) [with long
unsigned int _Np = 2; _Args = {const int&}; _First = const int; _Rest = {const
std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>
>}]'
/opt/compiler-explorer/gcc-trunk-20211024/include/c++/12.0.0/bits/stl_construct.h:119:7:
  required from 'constexpr void std::_Construct(_Tp*, _Args&& ...) [with _Tp =
std::__detail::__variant::_Variadic_union<const int, const
std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >
>; _Args = {const std::in_place_index_t<2>&, const int&}]'
...
Demo: https://gcc.godbolt.org/z/ocajj3aao

Related discussion: https://stackoverflow.com/q/69696170/7325599
```

[Bug libstdc++/102912] Not full support of const arguments in std::variant

[hewillk at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102912

康桓瑋 <hewillk at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hewillk at gmail dot com

--- Comment #1 from 康桓瑋 <hewillk at gmail dot com> ---
*** Bug 102913 has been marked as a duplicate of this bug. ***

[Bug libstdc++/102912] Not full support of const arguments in std::variant

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102912

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-10-25
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |rejects-valid

[Bug libstdc++/102912] [12 Regression] Not full support of const arguments in std::variant

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102912

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |11.2.1
            Summary|Not full support of const   |[12 Regression] Not full
                   |arguments in std::variant   |support of const arguments
                   |                            |in std::variant
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
   Target Milestone|---                         |12.0
             Status|NEW                         |ASSIGNED

[Bug libstdc++/102912] [12 Regression] Not full support of const arguments in std::variant

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102912

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
This is a bit of a mess. Previously 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));

It didn't matter if 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. That's why 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.

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 __index_of<_Type>
helper doesn't work if __rhs_mem and the alternative we're trying to construct
have different const-qualification. We need to replace
__variant_construct_single with something that has more type information
available.

[Bug libstdc++/102912] [12 Regression] Not full support of const arguments in std::variant

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102912

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:7551a9957437f20254be41d396962b9ccc46cee6

commit r12-4888-g7551a9957437f20254be41d396962b9ccc46cee6
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Nov 3 13:51:45 2021 +0000

    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.

[Bug libstdc++/102912] [12 Regression] Not full support of const arguments in std::variant

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102912

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed