This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: C++ PATCH for c++/66501 (wrong code with array move assignment)
- From: Jason Merrill <jason at redhat dot com>
- To: gcc-patches List <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 24 Jun 2015 11:39:27 -0400
- Subject: Re: C++ PATCH for c++/66501 (wrong code with array move assignment)
- Authentication-results: sourceware.org; auth=none
- References: <558967B9 dot 9050905 at redhat dot com>
On 06/23/2015 10:05 AM, Jason Merrill wrote:
build_vec_init was assuming that if a class has a trivial copy
assignment, then an array assignment is trivial. But overload
resolution might not choose the copy assignment operator. So this patch
changes build_vec_init to check for any non-trivial assignment operator.
On further consideration, it occurred to me that is_trivially_xible
gives the precise answer we want, so I'm changing build_vec_init to use it.
On 4.9 we don't have is_trivially_xible, so I'm doing a simpler check,
just adding TYPE_HAS_COMPLEX_MOVE_ASSIGN to the mix.
Tested x86_64-pc-linux-gnu, applying to trunk, 5 and 4.9.
commit d4d071b1f6552bfe57a1ed9e27de028580958afd
Author: Jason Merrill <jason@redhat.com>
Date: Tue Jun 23 22:02:30 2015 -0400
PR c++/66501
* init.c (vec_copy_assign_is_trivial): New.
(build_vec_init): Use it.
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 957a7a4..04c09d8 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -3367,6 +3367,18 @@ get_temp_regvar (tree type, tree init)
return decl;
}
+/* Subroutine of build_vec_init. Returns true if assigning to an array of
+ INNER_ELT_TYPE from INIT is trivial. */
+
+static bool
+vec_copy_assign_is_trivial (tree inner_elt_type, tree init)
+{
+ tree fromtype = inner_elt_type;
+ if (real_lvalue_p (init))
+ fromtype = cp_build_reference_type (fromtype, /*rval*/false);
+ return is_trivially_xible (MODIFY_EXPR, inner_elt_type, fromtype);
+}
+
/* `build_vec_init' returns tree structure that performs
initialization of a vector of aggregate types.
@@ -3443,8 +3455,7 @@ build_vec_init (tree base, tree maxindex, tree init,
&& TREE_CODE (atype) == ARRAY_TYPE
&& TREE_CONSTANT (maxindex)
&& (from_array == 2
- ? (!CLASS_TYPE_P (inner_elt_type)
- || !TYPE_HAS_COMPLEX_COPY_ASSIGN (inner_elt_type))
+ ? vec_copy_assign_is_trivial (inner_elt_type, init)
: !TYPE_NEEDS_CONSTRUCTING (type))
&& ((TREE_CODE (init) == CONSTRUCTOR
/* Don't do this if the CONSTRUCTOR might contain something
diff --git a/gcc/testsuite/g++.dg/cpp0x/rv-array1.C b/gcc/testsuite/g++.dg/cpp0x/rv-array1.C
new file mode 100644
index 0000000..9075764
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/rv-array1.C
@@ -0,0 +1,55 @@
+// PR c++/66501
+// { dg-do run { target c++11 } }
+
+int total_size;
+
+struct Object
+{
+ int size = 0;
+
+ Object () = default;
+
+ ~Object () {
+ total_size -= size;
+ }
+
+ Object (const Object &) = delete;
+ Object & operator= (const Object &) = delete;
+
+ Object (Object && b) {
+ size = b.size;
+ b.size = 0;
+ }
+
+ Object & operator= (Object && b) {
+ if (this != & b) {
+ total_size -= size;
+ size = b.size;
+ b.size = 0;
+ }
+ return * this;
+ }
+
+ void grow () {
+ size ++;
+ total_size ++;
+ }
+};
+
+struct Container {
+ Object objects[2];
+};
+
+int main (void)
+{
+ Container container;
+
+ // grow some objects in the container
+ for (auto & object : container.objects)
+ object.grow ();
+
+ // now empty it
+ container = Container ();
+
+ return total_size;
+}
commit 1c9edbe05d9e9437bcb7b3f621809461399aefe0
Author: Jason Merrill <jason@redhat.com>
Date: Tue Jun 23 22:02:30 2015 -0400
PR c++/66501
* init.c (vec_copy_assign_is_trivial): New.
(build_vec_init): Use it.
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 5cb7fc4..09a897f 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -3379,6 +3379,21 @@ get_temp_regvar (tree type, tree init)
return decl;
}
+/* Subroutine of build_vec_init. Returns true if assigning to an array of
+ INNER_ELT_TYPE from INIT is trivial. */
+
+static bool
+vec_copy_assign_is_trivial (tree inner_elt_type, tree init)
+{
+ if (!CLASS_TYPE_P (inner_elt_type))
+ return true;
+ if (cxx_dialect >= cxx11
+ && !real_lvalue_p (init)
+ && type_has_move_assign (inner_elt_type))
+ return !TYPE_HAS_COMPLEX_MOVE_ASSIGN (inner_elt_type);
+ return TYPE_HAS_TRIVIAL_COPY_ASSIGN (inner_elt_type);
+}
+
/* `build_vec_init' returns tree structure that performs
initialization of a vector of aggregate types.
@@ -3460,8 +3475,7 @@ build_vec_init (tree base, tree maxindex, tree init,
&& TREE_CODE (atype) == ARRAY_TYPE
&& TREE_CONSTANT (maxindex)
&& (from_array == 2
- ? (!CLASS_TYPE_P (inner_elt_type)
- || !TYPE_HAS_COMPLEX_COPY_ASSIGN (inner_elt_type))
+ ? vec_copy_assign_is_trivial (inner_elt_type, init)
: !TYPE_NEEDS_CONSTRUCTING (type))
&& ((TREE_CODE (init) == CONSTRUCTOR
/* Don't do this if the CONSTRUCTOR might contain something
diff --git a/gcc/testsuite/g++.dg/cpp0x/rv-array1.C b/gcc/testsuite/g++.dg/cpp0x/rv-array1.C
new file mode 100644
index 0000000..9075764
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/rv-array1.C
@@ -0,0 +1,55 @@
+// PR c++/66501
+// { dg-do run { target c++11 } }
+
+int total_size;
+
+struct Object
+{
+ int size = 0;
+
+ Object () = default;
+
+ ~Object () {
+ total_size -= size;
+ }
+
+ Object (const Object &) = delete;
+ Object & operator= (const Object &) = delete;
+
+ Object (Object && b) {
+ size = b.size;
+ b.size = 0;
+ }
+
+ Object & operator= (Object && b) {
+ if (this != & b) {
+ total_size -= size;
+ size = b.size;
+ b.size = 0;
+ }
+ return * this;
+ }
+
+ void grow () {
+ size ++;
+ total_size ++;
+ }
+};
+
+struct Container {
+ Object objects[2];
+};
+
+int main (void)
+{
+ Container container;
+
+ // grow some objects in the container
+ for (auto & object : container.objects)
+ object.grow ();
+
+ // now empty it
+ container = Container ();
+
+ return total_size;
+}