libstdc++: std::variant with multiple identical types assignment fail to compile [PR91807]

Message ID CAFk2RUbYNFUtBa4e4-_FK1WNbpQpkq36mTZrMppxasL6Kn9HqA@mail.gmail.com
State New
Headers show
Series
  • libstdc++: std::variant with multiple identical types assignment fail to compile [PR91807]
Related show

Commit Message

Jakub Jelinek via Gcc-patches July 1, 2020, 5:46 p.m.
Looks like just a small thinko. We construct a temporary and move-construct
from it, but we should construct the temporary with the right index.

OK for trunk and gcc-10 if full tests pass?

2020-07-01  Ville Voutilainen  <ville.voutilainen@gmail.com>

    PR libstdc++/91807
    * include/std/variant
    (_Copy_assign_base::operator=(const _Copy_assign_base&):
    Do the move-construction from a temporary so that the temporary
    is constructed with an explicit index.
    * testsuite/20_util/variant/91807.cc: New.

Comments

Jakub Jelinek via Gcc-patches July 1, 2020, 6:09 p.m. | #1
On Wed, 1 Jul 2020 at 20:46, Ville Voutilainen
<ville.voutilainen@gmail.com> wrote:
>

> Looks like just a small thinko. We construct a temporary and move-construct

> from it, but we should construct the temporary with the right index.

>

> OK for trunk and gcc-10 if full tests pass?

>

> 2020-07-01  Ville Voutilainen  <ville.voutilainen@gmail.com>

>

>     PR libstdc++/91807

>     * include/std/variant

>     (_Copy_assign_base::operator=(const _Copy_assign_base&):

>     Do the move-construction from a temporary so that the temporary

>     is constructed with an explicit index.

>     * testsuite/20_util/variant/91807.cc: New.


And sure, s/move-construction/move-assignment/.
Jakub Jelinek via Gcc-patches July 1, 2020, 8:32 p.m. | #2
On Wed, 1 Jul 2020 at 21:09, Ville Voutilainen
<ville.voutilainen@gmail.com> wrote:
> And sure, s/move-construction/move-assignment/.


And with dg-options.

2020-07-01  Ville Voutilainen  <ville.voutilainen@gmail.com>

    PR libstdc++/91807
    * include/std/variant
    (_Copy_assign_base::operator=(const _Copy_assign_base&):
    Do the move-assignment from a temporary so that the temporary
    is constructed with an explicit index.
    * testsuite/20_util/variant/91807.cc: New.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index c9504914365..eb3d6779205 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -611,7 +611,8 @@ namespace __variant
 		      this->_M_destructive_copy(__rhs_index, __rhs_mem);
 		    else
 		      __variant_cast<_Types...>(*this)
-			= variant<_Types...>(__rhs_mem);
+			= variant<_Types...>(std::in_place_index<__rhs_index>,
+					     __rhs_mem);
 		  }
 	      }
 	    else
diff --git a/libstdc++-v3/testsuite/20_util/variant/91807.cc b/libstdc++-v3/testsuite/20_util/variant/91807.cc
new file mode 100644
index 00000000000..04bb5d7c807
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/variant/91807.cc
@@ -0,0 +1,35 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do compile { target c++17 } }
+
+// Copyright (C) 2020 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/>.
+
+#include <variant>
+
+struct me_data {
+  me_data() = default;
+
+  me_data(const me_data &) {};
+  me_data(me_data &&) noexcept {};
+  me_data& operator=(const me_data &) = default;
+};
+
+int main() {
+  std::variant<me_data, me_data> v1, v2;
+
+  v2 = v1;
+}
Jakub Jelinek via Gcc-patches July 1, 2020, 8:53 p.m. | #3
On 01/07/20 23:32 +0300, Ville Voutilainen via Libstdc++ wrote:
>On Wed, 1 Jul 2020 at 21:09, Ville Voutilainen

><ville.voutilainen@gmail.com> wrote:

>> And sure, s/move-construction/move-assignment/.

>

>And with dg-options.


OK for master and gcc-10, thanks.

Does it apply cleanly to gcc-9 too?
Jakub Jelinek via Gcc-patches July 1, 2020, 9 p.m. | #4
On Wed, 1 Jul 2020 at 23:53, Jonathan Wakely <jwakely@redhat.com> wrote:
>

> On 01/07/20 23:32 +0300, Ville Voutilainen via Libstdc++ wrote:

> >On Wed, 1 Jul 2020 at 21:09, Ville Voutilainen

> ><ville.voutilainen@gmail.com> wrote:

> >> And sure, s/move-construction/move-assignment/.

> >

> >And with dg-options.

>

> OK for master and gcc-10, thanks.

>

> Does it apply cleanly to gcc-9 too?


Yes. I managed to misremember where the new variant implementation
landed and misread the
bug title/version, so backporting there, too.

Patch

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index c9504914365..eb3d6779205 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -611,7 +611,8 @@  namespace __variant
 		      this->_M_destructive_copy(__rhs_index, __rhs_mem);
 		    else
 		      __variant_cast<_Types...>(*this)
-			= variant<_Types...>(__rhs_mem);
+			= variant<_Types...>(std::in_place_index<__rhs_index>,
+					     __rhs_mem);
 		  }
 	      }
 	    else
diff --git a/libstdc++-v3/testsuite/20_util/variant/91807.cc b/libstdc++-v3/testsuite/20_util/variant/91807.cc
new file mode 100644
index 00000000000..ddede7c9b32
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/variant/91807.cc
@@ -0,0 +1,34 @@ 
+// { dg-do compile { target c++17 } }
+
+// Copyright (C) 2020 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/>.
+
+#include <variant>
+
+struct me_data {
+  me_data() = default;
+  
+  me_data(const me_data &) {};
+  me_data(me_data &&) noexcept {};
+  me_data& operator=(const me_data &) = default;
+};
+
+int main() {
+  std::variant<me_data, me_data> v1, v2;
+  
+  v2 = v1;
+}