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.
commit 047c80285df6bbca5d2044efdd644a2930432b63
Author: Jason Merrill <ja...@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" }
