C++ PATCH for c++/66501 (wrong code with array move assignment)

C++ PATCH for c++/66501 (wrong code with array move assignment)

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 323 bytes --]

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.

Tested x86_64-pc-linux-gnu, applying to trunk.

[-- Attachment #2: 66501.patch --]
[-- Type: text/x-patch, Size: 3795 bytes --]

commit 2f1cd98c72127d70e198ed99c67cc5d031f052b6
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Jun 22 15:13:58 2015 -0400

    	PR c++/66501
    	* class.c (type_has_nontrivial_assignment): New.
    	* init.c (build_vec_init): Use it.
    	* cp-tree.h: Declare it.
    	* method.c (trivial_fn_p): Templates aren't trivial.

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 9da532e..88f1022 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -5136,6 +5136,24 @@ type_has_non_user_provided_default_constructor (tree t)
   return false;
 }
 
+/* Return true if TYPE has some non-trivial assignment operator.  */
+
+bool
+type_has_nontrivial_assignment (tree type)
+{
+  gcc_assert (TREE_CODE (type) != ARRAY_TYPE);
+  if (CLASS_TYPE_P (type))
+    for (tree fns
+	   = lookup_fnfields_slot_nolazy (type, ansi_assopname (NOP_EXPR));
+	 fns; fns = OVL_NEXT (fns))
+      {
+	tree fn = OVL_CURRENT (fns);
+	if (!trivial_fn_p (fn))
+	  return true;
+      }
+  return false;
+}
+
 /* TYPE is being used as a virtual base, and has a non-trivial move
    assignment.  Return true if this is due to there being a user-provided
    move assignment in TYPE or one of its subobjects; if there isn't, then
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index b53aa90..8eb7474 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5295,6 +5295,7 @@ extern tree in_class_defaulted_default_constructor (tree);
 extern bool user_provided_p			(tree);
 extern bool type_has_user_provided_constructor  (tree);
 extern bool type_has_non_user_provided_default_constructor (tree);
+extern bool type_has_nontrivial_assignment	(tree);
 extern bool vbase_has_user_provided_move_assign (tree);
 extern tree default_init_uninitialized_part (tree);
 extern bool trivial_default_constructor_is_constexpr (tree);
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index fc30fef..08c6c0e 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -3460,8 +3460,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))
+	  ? !type_has_nontrivial_assignment (inner_elt_type)
 	  : !TYPE_NEEDS_CONSTRUCTING (type))
       && ((TREE_CODE (init) == CONSTRUCTOR
 	   /* Don't do this if the CONSTRUCTOR might contain something
diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index 79e4bbc..da03c36 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -476,6 +476,8 @@ type_set_nontrivial_flag (tree ctype, special_function_kind sfk)
 bool
 trivial_fn_p (tree fn)
 {
+  if (TREE_CODE (fn) == TEMPLATE_DECL)
+    return false;
   if (!DECL_DEFAULTED_FN (fn))
     return false;
 
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;
+}

Re: C++ PATCH for c++/66501 (wrong code with array move assignment)

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 658 bytes --]

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.



[-- Attachment #2: 66501-5.patch --]
[-- Type: text/x-patch, Size: 2478 bytes --]

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;
+}

[-- Attachment #3: 66501-4.9.patch --]
[-- Type: text/x-patch, Size: 2560 bytes --]

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;
+}