Patchwork C++ PATCH for c++/53733 (DR 1402, deleting move ctor)

login
register
mail settings
Submitter Jason Merrill
Date Oct. 25, 2012, 3:56 p.m.
Message ID <5089611C.6050009@redhat.com>
Download mbox | patch
Permalink /patch/194246/
State New
Headers show

Comments

Jason Merrill - Oct. 25, 2012, 3:56 p.m.
In Portland we decided that we should allow defaulted move ctors/op= 
even if they could throw or might move a virtual base more than once, 
and that overload resolution should ignore an implicitly deleted move 
ctor/op=.  This patch implements that resolution.

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

Patch

commit 047c80285df6bbca5d2044efdd644a2930432b63
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Oct 18 00:22:06 2012 -0700

    	Core 1402
    cp/
    	* call.c (joust): An implicitly deleted move function is
    	worse than any non-deleted function.
    	* method.c (process_subob_fn): No special rules for move.
    	(synthesized_method_walk, implicitly_declare_fn): Likewise.
    	Warn about virtual base with non-trivial move assignment.
    	* cp-tree.h (struct lang_decl_fn): Remove suppress_implicit_decl.
    	(FNDECL_SUPPRESS_IMPLICIT_DECL): Remove.
    c-family/
    	* c.opt (Wvirtual-move-assign): New.

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 60cb726..7eb66c6 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -741,6 +741,10 @@  Wvolatile-register-var
 C ObjC C++ ObjC++ Var(warn_volatile_register_var) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn when a register variable is declared volatile
 
+Wvirtual-move-assign
+C++ ObjC++ Var(warn_virtual_move_assign) Warning Init(1)
+Warn if a virtual base has a non-trivial move assignment operator
+
 Wwrite-strings
 C ObjC C++ ObjC++ Var(warn_write_strings) Warning
 In C++, nonzero means warn about deprecated conversion from string literals to 'char *'.  In C, similar warning, except that the conversion is of course not deprecated by the ISO C standard.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index e21049b..b365240 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8176,6 +8176,22 @@  joust (struct z_candidate *cand1, struct z_candidate *cand2, bool warn,
       && (IS_TYPE_OR_DECL_P (cand1->fn)))
     return 1;
 
+  /* Prefer a non-deleted function over an implicitly deleted move
+     constructor or assignment operator.  This differs slightly from the
+     wording for issue 1402 (which says the move op is ignored by overload
+     resolution), but this way produces better error messages.  */
+  if (TREE_CODE (cand1->fn) == FUNCTION_DECL
+      && TREE_CODE (cand2->fn) == FUNCTION_DECL
+      && DECL_DELETED_FN (cand1->fn) != DECL_DELETED_FN (cand2->fn))
+    {
+      if (DECL_DELETED_FN (cand1->fn) && DECL_DEFAULTED_FN (cand1->fn)
+	  && move_fn_p (cand1->fn))
+	return -1;
+      if (DECL_DELETED_FN (cand2->fn) && DECL_DEFAULTED_FN (cand2->fn)
+	  && move_fn_p (cand2->fn))
+	return 1;
+    }
+
   /* a viable function F1
      is defined to be a better function than another viable function F2  if
      for  all arguments i, ICSi(F1) is not a worse conversion sequence than
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 7b4277b..77508a1 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -1954,7 +1954,7 @@  struct GTY(()) lang_decl_fn {
   unsigned thunk_p : 1;
   unsigned this_thunk_p : 1;
   unsigned hidden_friend_p : 1;
-  unsigned suppress_implicit_decl : 1;
+  /* 1 spare bit.  */
 
   /* For a non-thunk function decl, this is a tree list of
      friendly classes. For a thunk function decl, it is the
@@ -3144,12 +3144,6 @@  more_aggr_init_expr_args_p (const aggr_init_expr_arg_iterator *iter)
 #define DECL_HIDDEN_FRIEND_P(NODE) \
   (LANG_DECL_FN_CHECK (DECL_COMMON_CHECK (NODE))->hidden_friend_p)
 
-/* Nonzero if NODE is a FUNCTION_DECL generated by implicitly_declare_fn
-   that we shouldn't actually declare implicitly; it is only used for
-   comparing to an =default declaration.  */
-#define FNDECL_SUPPRESS_IMPLICIT_DECL(NODE) \
-  (LANG_DECL_FN_CHECK (DECL_COMMON_CHECK (NODE))->suppress_implicit_decl)
-
 /* Nonzero if DECL has been declared threadprivate by
    #pragma omp threadprivate.  */
 #define CP_DECL_THREADPRIVATE_P(DECL) \
diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index 4da5cc9..8a7d7cb 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -969,8 +969,8 @@  get_copy_assign (tree type)
    DELETED_P or give an error message MSG with argument ARG.  */
 
 static void
-process_subob_fn (tree fn, bool move_p, tree *spec_p, bool *trivial_p,
-		  bool *deleted_p, bool *constexpr_p, bool *no_implicit_p,
+process_subob_fn (tree fn, tree *spec_p, bool *trivial_p,
+		  bool *deleted_p, bool *constexpr_p,
 		  bool diag, tree arg)
 {
   if (!fn || fn == error_mark_node)
@@ -996,12 +996,6 @@  process_subob_fn (tree fn, bool move_p, tree *spec_p, bool *trivial_p,
 	}
     }
 
-  /* Core 1402: A non-trivial non-move ctor suppresses the implicit
-     declaration of the move ctor/op=.  */
-  if (no_implicit_p && move_p && !move_signature_fn_p (fn)
-      && !trivial_fn_p (fn))
-    *no_implicit_p = true;
-
   if (constexpr_p && !DECL_DECLARED_CONSTEXPR_P (fn))
     {
       *constexpr_p = false;
@@ -1027,7 +1021,7 @@  static void
 walk_field_subobs (tree fields, tree fnname, special_function_kind sfk,
 		   int quals, bool copy_arg_p, bool move_p,
 		   bool assign_p, tree *spec_p, bool *trivial_p,
-		   bool *deleted_p, bool *constexpr_p, bool *no_implicit_p,
+		   bool *deleted_p, bool *constexpr_p,
 		   bool diag, int flags, tsubst_flags_t complain)
 {
   tree field;
@@ -1126,7 +1120,7 @@  walk_field_subobs (tree fields, tree fnname, special_function_kind sfk,
 	{
 	  walk_field_subobs (TYPE_FIELDS (mem_type), fnname, sfk, quals,
 			     copy_arg_p, move_p, assign_p, spec_p, trivial_p,
-			     deleted_p, constexpr_p, no_implicit_p,
+			     deleted_p, constexpr_p,
 			     diag, flags, complain);
 	  continue;
 	}
@@ -1143,8 +1137,8 @@  walk_field_subobs (tree fields, tree fnname, special_function_kind sfk,
 
       rval = locate_fn_flags (mem_type, fnname, argtype, flags, complain);
 
-      process_subob_fn (rval, move_p, spec_p, trivial_p, deleted_p,
-			constexpr_p, no_implicit_p, diag, field);
+      process_subob_fn (rval, spec_p, trivial_p, deleted_p,
+			constexpr_p, diag, field);
     }
 }
 
@@ -1158,7 +1152,7 @@  walk_field_subobs (tree fields, tree fnname, special_function_kind sfk,
 static void
 synthesized_method_walk (tree ctype, special_function_kind sfk, bool const_p,
 			 tree *spec_p, bool *trivial_p, bool *deleted_p,
-			 bool *constexpr_p, bool *no_implicit_p, bool diag,
+			 bool *constexpr_p, bool diag,
 			 tree inherited_base, tree inherited_parms)
 {
   tree binfo, base_binfo, scope, fnname, rval, argtype;
@@ -1171,9 +1165,6 @@  synthesized_method_walk (tree ctype, special_function_kind sfk, bool const_p,
   if (spec_p)
     *spec_p = (cxx_dialect >= cxx0x ? noexcept_true_spec : empty_except_spec);
 
-  if (no_implicit_p)
-    *no_implicit_p = false;
-
   if (deleted_p)
     {
       /* "The closure type associated with a lambda-expression has a deleted
@@ -1314,8 +1305,8 @@  synthesized_method_walk (tree ctype, special_function_kind sfk, bool const_p,
       if (inherited_base)
 	argtype = NULL_TREE;
 
-      process_subob_fn (rval, move_p, spec_p, trivial_p, deleted_p,
-			constexpr_p, no_implicit_p, diag, basetype);
+      process_subob_fn (rval, spec_p, trivial_p, deleted_p,
+			constexpr_p, diag, basetype);
       if (ctor_p && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (basetype))
 	{
 	  /* In a constructor we also need to check the subobject
@@ -1327,8 +1318,8 @@  synthesized_method_walk (tree ctype, special_function_kind sfk, bool const_p,
 	     do they affect constexpr-ness (a constant expression doesn't
 	     throw) or exception-specification (a throw from one of the
 	     dtors would be a double-fault).  */
-	  process_subob_fn (rval, false, NULL, NULL,
-			    deleted_p, NULL, NULL, false,
+	  process_subob_fn (rval, NULL, NULL,
+			    deleted_p, NULL, false,
 			    basetype);
 	}
 
@@ -1342,31 +1333,20 @@  synthesized_method_walk (tree ctype, special_function_kind sfk, bool const_p,
 	    *deleted_p = true;
 	  check_vdtor = false;
 	}
+
+      if (diag && assign_p && move_p
+	  && BINFO_VIRTUAL_P (base_binfo)
+	  && rval && TREE_CODE (rval) == FUNCTION_DECL
+	  && move_fn_p (rval) && !trivial_fn_p (rval))
+	warning (OPT_Wvirtual_move_assign,
+		 "defaulted move assignment for %qT calls a non-trivial "
+		 "move assignment operator for virtual base %qT",
+		 ctype, basetype);
     }
 
   vbases = CLASSTYPE_VBASECLASSES (ctype);
   if (vbases == NULL)
     /* No virtual bases to worry about.  */;
-  else if (assign_p && move_p && no_implicit_p)
-    {
-      /* Don't implicitly declare a defaulted move assignment if a virtual
-	 base has non-trivial move assignment, since moving the same base
-	 more than once is dangerous.  */
-      /* Should the spec be changed to allow vbases that only occur once?  */
-      FOR_EACH_VEC_ELT (tree, vbases, i, base_binfo)
-	{
-	  tree basetype = BINFO_TYPE (base_binfo);
-	  if (copy_arg_p)
-	    argtype = build_stub_type (basetype, quals, move_p);
-	  rval = locate_fn_flags (base_binfo, fnname, argtype, flags, complain);
-	  if (rval && rval != error_mark_node
-	      && move_fn_p (rval) && !trivial_fn_p (rval))
-	    {
-	      *no_implicit_p = true;
-	      break;
-	    }
-	}
-    }
   else if (!assign_p)
     {
       if (constexpr_p)
@@ -1378,14 +1358,14 @@  synthesized_method_walk (tree ctype, special_function_kind sfk, bool const_p,
 	    argtype = build_stub_type (basetype, quals, move_p);
 	  rval = locate_fn_flags (base_binfo, fnname, argtype, flags, complain);
 
-	  process_subob_fn (rval, move_p, spec_p, trivial_p, deleted_p,
-			    constexpr_p, no_implicit_p, diag, basetype);
+	  process_subob_fn (rval, spec_p, trivial_p, deleted_p,
+			    constexpr_p, diag, basetype);
 	  if (ctor_p && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (basetype))
 	    {
 	      rval = locate_fn_flags (base_binfo, complete_dtor_identifier,
 				      NULL_TREE, flags, complain);
-	      process_subob_fn (rval, false, NULL, NULL,
-				deleted_p, NULL, NULL, false,
+	      process_subob_fn (rval, NULL, NULL,
+				deleted_p, NULL, false,
 				basetype);
 	    }
 	}
@@ -1394,14 +1374,14 @@  synthesized_method_walk (tree ctype, special_function_kind sfk, bool const_p,
   /* Now handle the non-static data members.  */
   walk_field_subobs (TYPE_FIELDS (ctype), fnname, sfk, quals,
 		     copy_arg_p, move_p, assign_p, spec_p, trivial_p,
-		     deleted_p, constexpr_p, no_implicit_p,
+		     deleted_p, constexpr_p,
 		     diag, flags, complain);
   if (ctor_p)
     walk_field_subobs (TYPE_FIELDS (ctype), complete_dtor_identifier,
 		       sfk_destructor, TYPE_UNQUALIFIED, false,
 		       false, false, NULL, NULL,
 		       deleted_p, NULL,
-		       NULL, false, flags, complain);
+		       false, flags, complain);
 
   pop_scope (scope);
 
@@ -1473,7 +1453,7 @@  maybe_explain_implicit_delete (tree decl)
 		 "definition would be ill-formed:", decl);
 	  pop_scope (scope);
 	  synthesized_method_walk (ctype, sfk, const_p,
-				   NULL, NULL, NULL, NULL, NULL, true,
+				   NULL, NULL, NULL, NULL, true,
 				   DECL_INHERITED_CTOR_BASE (decl), parms);
 	}
 
@@ -1494,7 +1474,7 @@  explain_implicit_non_constexpr (tree decl)
   bool dummy;
   synthesized_method_walk (DECL_CLASS_CONTEXT (decl),
 			   special_function_p (decl), const_p,
-			   NULL, NULL, NULL, &dummy, NULL, true,
+			   NULL, NULL, NULL, &dummy, true,
 			   NULL_TREE, NULL_TREE);
 }
 
@@ -1507,10 +1487,10 @@  deduce_inheriting_ctor (tree decl)
 {
   gcc_assert (DECL_INHERITED_CTOR_BASE (decl));
   tree spec;
-  bool trivial, constexpr_, deleted, no_implicit;
+  bool trivial, constexpr_, deleted;
   synthesized_method_walk (DECL_CONTEXT (decl), sfk_inheriting_constructor,
 			   false, &spec, &trivial, &deleted, &constexpr_,
-			   &no_implicit, /*diag*/false,
+			   /*diag*/false,
 			   DECL_INHERITED_CTOR_BASE (decl),
 			   FUNCTION_FIRST_USER_PARMTYPE (decl));
   DECL_DELETED_FN (decl) = deleted;
@@ -1540,7 +1520,6 @@  implicitly_declare_fn (special_function_kind kind, tree type,
   bool deleted_p;
   bool trivial_p;
   bool constexpr_p;
-  bool no_implicit_p;
 
   /* Because we create declarations for implicitly declared functions
      lazily, we may be creating the declaration for a member of TYPE
@@ -1627,11 +1606,10 @@  implicitly_declare_fn (special_function_kind kind, tree type,
       deleted_p = DECL_DELETED_FN (DECL_TEMPLATE_RESULT (inherited_ctor));
       constexpr_p
 	= DECL_DECLARED_CONSTEXPR_P (DECL_TEMPLATE_RESULT (inherited_ctor));
-      no_implicit_p = false;
     }
   else
     synthesized_method_walk (type, kind, const_p, &raises, &trivial_p,
-			     &deleted_p, &constexpr_p, &no_implicit_p, false,
+			     &deleted_p, &constexpr_p, false,
 			     inherited_base, inherited_parms);
   /* Don't bother marking a deleted constructor as constexpr.  */
   if (deleted_p)
@@ -1719,7 +1697,6 @@  implicitly_declare_fn (special_function_kind kind, tree type,
       DECL_DELETED_FN (fn) = deleted_p;
       DECL_DECLARED_CONSTEXPR_P (fn) = constexpr_p;
     }
-  FNDECL_SUPPRESS_IMPLICIT_DECL (fn) = no_implicit_p;
   DECL_EXTERNAL (fn) = true;
   DECL_NOT_REALLY_EXTERN (fn) = 1;
   DECL_DECLARED_INLINE_P (fn) = 1;
@@ -1731,6 +1708,18 @@  implicitly_declare_fn (special_function_kind kind, tree type,
   if (inherited_ctor && TREE_CODE (inherited_ctor) == TEMPLATE_DECL)
     fn = add_inherited_template_parms (fn, inherited_ctor);
 
+  /* Warn about calling a non-trivial move assignment in a virtual base.  */
+  if (kind == sfk_move_assignment && !deleted_p && !trivial_p
+      && CLASSTYPE_VBASECLASSES (type))
+    {
+      location_t loc = input_location;
+      input_location = DECL_SOURCE_LOCATION (fn);
+      synthesized_method_walk (type, kind, const_p,
+			       NULL, NULL, NULL, NULL, true,
+			       NULL_TREE, NULL_TREE);
+      input_location = loc;
+    }
+
   return fn;
 }
 
@@ -1909,17 +1898,6 @@  lazily_declare_fn (special_function_kind sfk, tree type)
 	  || type_has_user_declared_move_assign (type)))
     DECL_DELETED_FN (fn) = true;
 
-  /* For move variants, rather than declare them as deleted we just
-     don't declare them at all.  */
-  if (DECL_DELETED_FN (fn)
-      && (sfk == sfk_move_constructor
-	  || sfk == sfk_move_assignment))
-    return NULL_TREE;
-
-  /* We also suppress implicit move if it would call a non-trivial copy.  */
-  if (FNDECL_SUPPRESS_IMPLICIT_DECL (fn))
-    return NULL_TREE;
-
   /* A destructor may be virtual.  */
   if (sfk == sfk_destructor
       || sfk == sfk_move_assignment
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index a6baee7..afb9f21 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4722,6 +4722,16 @@  using scalars of wider type, which normally is more performance efficient;
 and @code{as a single scalar}, which means that vector fits into a
 scalar type.
 
+@item -Wno-virtual-move-assign
+@opindex Wvirtual-move-assign
+@opindex Wno-virtual-move-assign
+Suppress warnings about inheriting from a virtual base with a
+non-trivial C++11 move assignment operator.  This is dangerous because
+if the virtual base is reachable along more than one path, it will be
+moved multiple times, which can mean both objects end up in the
+moved-from state.  If the move assignment operator is written to avoid
+moving from a moved-from object, this warning can be disabled.
+
 @item -Wvla
 @opindex Wvla
 @opindex Wno-vla
diff --git a/gcc/testsuite/g++.dg/cpp0x/defaulted37.C b/gcc/testsuite/g++.dg/cpp0x/defaulted37.C
index 69105cc..1926f2e 100644
--- a/gcc/testsuite/g++.dg/cpp0x/defaulted37.C
+++ b/gcc/testsuite/g++.dg/cpp0x/defaulted37.C
@@ -3,13 +3,11 @@ 
 
 struct A
 {
-  int moved = 0;
-  A& operator=(A&&) { ++moved; }
-  ~A() { if (moved > 1) __builtin_abort(); }
+  A& operator=(A&&);
 };
 
-struct B: virtual A { B& operator=(B&&) = default; };
-struct C: virtual A { };	// { dg-error "operator=.const A&" }
+struct B: virtual A { B& operator=(B&&) = default; }; // { dg-warning "virtual base" }
+struct C: virtual A { };			      // { dg-warning "virtual base" }
 
 int main()
 {
@@ -17,5 +15,5 @@  int main()
   b2 = static_cast<B&&>(b1);
 
   C c1, c2;
-  c2 = static_cast<C&&>(c1);	// { dg-error "operator=.const C&" }
+  c2 = static_cast<C&&>(c1);
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/defaulted39.C b/gcc/testsuite/g++.dg/cpp0x/defaulted39.C
new file mode 100644
index 0000000..fe847d4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/defaulted39.C
@@ -0,0 +1,23 @@ 
+// DR 1402
+// { dg-options -std=c++11 }
+
+template <class T> T&& move(T& t);
+
+struct A
+{
+  A(const A&);
+};
+
+struct B
+{
+  B(B&&);
+};
+
+struct C
+{
+  A a;
+  B b;
+};
+
+extern C c1;
+C c2(move(c1));
diff --git a/gcc/testsuite/g++.dg/cpp0x/defaulted40.C b/gcc/testsuite/g++.dg/cpp0x/defaulted40.C
new file mode 100644
index 0000000..c160b39
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/defaulted40.C
@@ -0,0 +1,23 @@ 
+// DR 1402
+// { dg-options -std=c++11 }
+
+template <class T> T&& move(T& t);
+
+struct A
+{
+  A(const A&);
+};
+
+struct B
+{
+  B(B&&) = delete;		// { dg-prune-output "declared" }
+};
+
+struct C			// { dg-error "deleted" }
+{
+  A a;
+  B b;
+};
+
+extern C c1;
+C c2(move(c1));			// { dg-error "deleted" }