[Bug libstdc++/112942] New: swap(variant&, variant&) is incorrectly marked as deleted

[Bug libstdc++/112942] New: swap(variant&, variant&) is incorrectly marked as deleted

[kotonartem at protonmail dot com]

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

            Bug ID: 112942
           Summary: swap(variant&, variant&) is incorrectly marked as
                    deleted
           Product: gcc
           Version: 13.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: kotonartem at protonmail dot com
  Target Milestone: ---

GCC rejects the following code (when using libstdc++):

```
#include <variant>

struct A
{};

void swap(A&, A&) = delete;

void foo()
{
    using V = std::variant<A>;
    V a, b;

    using std::swap; // this is unnecessary due to ADL
    swap(a, b);
}
```

with the following message:

```
<source>: In function 'void foo()':
<source>:14:9: error: use of deleted function
'std::enable_if_t<(!((is_move_constructible_v<_Types> && ...) &&
(is_swappable_v<_Types> && ...)))> std::swap(variant<_Types ...>&,
variant<_Types ...>&) [with _Types = {A};
enable_if_t<(!((is_move_constructible_v<_Types> && ...) &&
(is_swappable_v<_Types> && ...)))> = void]'
   14 |     swap(a, b);
      |     ~~~~^~~~~~
In file included from <source>:1:
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/variant:1303:5: note:
declared here
 1303 |     swap(variant<_Types...>&, variant<_Types...>&) = delete;
      |     ^~~~
```

The compiler explorer link: https://godbolt.org/z/5od4s5cf3.

There is also a similar test case that expects the current behavior:
https://github.com/gcc-mirror/gcc/blob/c250ff9/libstdc%2B%2B-v3/testsuite/20_util/variant/compile.cc#L294-L300.
> ```
> // Not swappable, and variant<C> not swappable via the generic std::swap.
> struct C { };
> void swap(C&, C&) = delete;
> 
> static_assert( !std::is_swappable_v<variant<C>> );
> static_assert( !std::is_swappable_v<variant<int, C>> );
> static_assert( !std::is_swappable_v<variant<C, int>> );
> ```

I believe that this behavior is incorrect and the variant should indeed be
swappable even if `swap(A&, A&)` is deleted.

My reasoning is that in [variant.specalg] the `std::is_swappable_v<T_i>`
requirement is mentioned under "Constraints" section, which, according to
[structure.specifications], describes "the conditions for the function's
participation in overload resolution". That is in contrast to "Mandates"
section, which would render the program ill-formed.

And since the `swap(variant<A>&, variant<A>&)` overload is not considered, the
generic `template<class T> void swap(T& a, T& b)` should be chosen instead.
Unlike the former overload, it only requires `T` (aka `variant<A>`) to be both
move constructible and move assignable, which it is.

It should be noted that libc++ handles this case correctly, i.e.
`std::is_swappable_v<std::variant<A>>` is `true`.

[Bug libstdc++/112942] swap(variant&, variant&) is incorrectly marked as deleted

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
https://wg21.cmeerw.net/lwg/issue2749

[Bug libstdc++/112942] swap(variant&, variant&) is incorrectly marked as deleted

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
https://gcc.gnu.org/pipermail/libstdc++/2016-November/045176.html

[Bug libstdc++/112942] swap(variant&, variant&) is incorrectly marked as deleted

[kotonartem at protonmail dot com]

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

--- Comment #3 from Artem Koton <kotonartem at protonmail dot com> ---
(In reply to Andrew Pinski from comment #1)
> https://wg21.cmeerw.net/lwg/issue2749

Could you elaborate? I understand that this issue discusses the constraints in
question but a failure to meet them should still result in the function just
being excluded from overload resolution. The program should not be ill-formed.

[Bug libstdc++/112942] swap(variant&, variant&) is incorrectly marked as deleted

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Artem Koton from comment #3)
> (In reply to Andrew Pinski from comment #1)
> > https://wg21.cmeerw.net/lwg/issue2749
> 
> Could you elaborate? I understand that this issue discusses the constraints
> in question but a failure to meet them should still result in the function
> just being excluded from overload resolution. The program should not be
> ill-formed.

Just giving background information of the wording changes to C++17 on
std::variant and std::swap here and about the testcase you pointed to and about
when it was added specifcially while changing for that defect report.

[Bug libstdc++/112942] [LWG2766] swap(variant&, variant&) is incorrectly marked as deleted

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
            Summary|swap(variant&, variant&) is |[LWG2766] swap(variant&,
                   |incorrectly marked as       |variant&) is incorrectly
                   |deleted                     |marked as deleted
   Last reconfirmed|                            |2023-12-10
             Status|UNCONFIRMED                 |SUSPENDED

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #1)
> https://wg21.cmeerw.net/lwg/issue2749

LWG 2749 added the current constraint to [variant.specalg]. The libstdc++
behaviour is because we implement the proposed resolution of LWG 2766:
https://cplusplus.github.io/LWG/issue2766

That issue points out that swapping variant<A> when is_swappable_v<A> is false
makes no sense. The author of A explicitly disabled swapping of A. Falling back
to the generic std::swap that uses moves seems wrong. If variant's own swap
overload is disabled, then swapping should be disabled.

But LWG 2766 seems to have languished for years. I'll poke LWG about it.

[Bug libstdc++/112942] [LWG2766] swap(variant&, variant&) is incorrectly marked as deleted

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #4)
> Just giving background information of the wording changes to C++17 on
> std::variant and std::swap here and about the testcase you pointed to and
> about when it was added specifcially while changing for that defect report.

The test case verifies the LWG 2766 behaviour, *not* the LWG 2749 behaviour.