C++ PATCH for c++/80178 (ABI of non-copyable class)
Jason Merrill
jason@redhat.com
Mon May 8 19:16:00 GMT 2017
Bug 80178 points out that we are doing bitwise copy when passing a
class with only deleted copy/move constructors; we should use
invisible reference instead.
The second patch updates the trunk to use -fabi-version=12 by default for GCC 8.
Tested x86_64-pc-linux-gnu, applying to trunk.
-------------- next part --------------
commit f34d5751de4b94419ead04c5ab9be0dde4d21953
Author: Jason Merrill <jason@redhat.com>
Date: Mon May 8 14:24:03 2017 -0400
PR c++/80178 - parameter passing for uncopyable classes
* tree.c (type_has_nontrivial_copy_init): True for classes with only
deleted copy/move ctors.
(remember_deleted_copy, maybe_warn_parm_abi): New.
* decl.c (require_complete_types_for_parms, check_function_type):
Call maybe_warn_parm_abi.
* call.c (convert_for_arg_passing, build_cxx_call): Likewise.
diff --git a/gcc/common.opt b/gcc/common.opt
index a5c3aea..1330555 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -910,6 +910,10 @@ Driver Undocumented
; and introduces new inheriting constructor handling.
; Default in G++ 7.
;
+; 12: Corrects the calling convention for classes with only deleted copy/move
+; constructors.
+; Default in G++ 8.
+;
; Additional positive integers will be assigned as new versions of
; the ABI become the default version of the ABI.
fabi-version=
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index f1e431c..d9accd1 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7352,17 +7352,21 @@ convert_for_arg_passing (tree type, tree val, tsubst_flags_t complain)
&& COMPLETE_TYPE_P (type)
&& tree_int_cst_lt (TYPE_SIZE (type), TYPE_SIZE (integer_type_node)))
val = cp_perform_integral_promotions (val, complain);
- if ((complain & tf_warning)
- && warn_suggest_attribute_format)
+ if (complain & tf_warning)
{
- tree rhstype = TREE_TYPE (val);
- const enum tree_code coder = TREE_CODE (rhstype);
- const enum tree_code codel = TREE_CODE (type);
- if ((codel == POINTER_TYPE || codel == REFERENCE_TYPE)
- && coder == codel
- && check_missing_format_attribute (type, rhstype))
- warning (OPT_Wsuggest_attribute_format,
- "argument of function call might be a candidate for a format attribute");
+ if (warn_suggest_attribute_format)
+ {
+ tree rhstype = TREE_TYPE (val);
+ const enum tree_code coder = TREE_CODE (rhstype);
+ const enum tree_code codel = TREE_CODE (type);
+ if ((codel == POINTER_TYPE || codel == REFERENCE_TYPE)
+ && coder == codel
+ && check_missing_format_attribute (type, rhstype))
+ warning (OPT_Wsuggest_attribute_format,
+ "argument of function call might be a candidate "
+ "for a format attribute");
+ }
+ maybe_warn_parm_abi (type, EXPR_LOC_OR_LOC (val, input_location));
}
return val;
}
@@ -8234,7 +8238,10 @@ build_cxx_call (tree fn, int nargs, tree *argarray,
return error_mark_node;
if (MAYBE_CLASS_TYPE_P (TREE_TYPE (fn)))
- fn = build_cplus_new (TREE_TYPE (fn), fn, complain);
+ {
+ fn = build_cplus_new (TREE_TYPE (fn), fn, complain);
+ maybe_warn_parm_abi (TREE_TYPE (fn), loc);
+ }
}
return convert_from_reference (fn);
}
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index d2d48e7..b64fa6d 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6614,6 +6614,7 @@ extern bool type_has_unique_obj_representations (const_tree);
extern bool scalarish_type_p (const_tree);
extern bool type_has_nontrivial_default_init (const_tree);
extern bool type_has_nontrivial_copy_init (const_tree);
+extern void maybe_warn_parm_abi (tree, location_t);
extern bool class_tmpl_impl_spec_p (const_tree);
extern int zero_init_p (const_tree);
extern bool check_abi_tag_redeclaration (const_tree, const_tree, const_tree);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 2b20bf9..f21058d 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -12394,6 +12394,9 @@ require_complete_types_for_parms (tree parms)
{
relayout_decl (parms);
DECL_ARG_TYPE (parms) = type_passed_as (TREE_TYPE (parms));
+
+ maybe_warn_parm_abi (TREE_TYPE (parms),
+ DECL_SOURCE_LOCATION (parms));
}
else
/* grokparms or complete_type_or_else will have already issued
@@ -14646,7 +14649,11 @@ check_function_type (tree decl, tree current_function_parms)
TREE_TYPE (decl) = fntype;
}
else
- abstract_virtuals_error (decl, TREE_TYPE (fntype));
+ {
+ abstract_virtuals_error (decl, TREE_TYPE (fntype));
+ maybe_warn_parm_abi (TREE_TYPE (fntype),
+ DECL_SOURCE_LOCATION (decl));
+ }
}
/* True iff FN is an implicitly-defined default constructor. */
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index ba1cb33..858d0d4 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3. If not see
#include "convert.h"
#include "gimplify.h"
#include "attribs.h"
+#include "flags.h"
static tree bot_manip (tree *, int *, void *);
static tree bot_replace (tree *, int *, void *);
@@ -3596,21 +3597,112 @@ type_has_nontrivial_default_init (const_tree t)
return 0;
}
+/* Track classes with only deleted copy/move constructors so that we can warn
+ if they are used in call/return by value. */
+
+static GTY(()) hash_set<tree>* deleted_copy_types;
+static void
+remember_deleted_copy (const_tree t)
+{
+ if (!deleted_copy_types)
+ deleted_copy_types = hash_set<tree>::create_ggc(37);
+ deleted_copy_types->add (CONST_CAST_TREE (t));
+}
+void
+maybe_warn_parm_abi (tree t, location_t loc)
+{
+ if (!deleted_copy_types
+ || !deleted_copy_types->contains (t))
+ return;
+
+ warning_at (loc, OPT_Wabi, "the calling convention for %qT changes in "
+ "-fabi-version=12 (GCC 8)", t);
+ static bool explained = false;
+ if (!explained)
+ {
+ inform (loc, " because all of its copy and move constructors "
+ "are deleted");
+ explained = true;
+ }
+}
+
/* Returns true iff copying an object of type T (including via move
constructor) is non-trivial. That is, T has no non-trivial copy
- constructors and no non-trivial move constructors. */
+ constructors and no non-trivial move constructors, and not all copy/move
+ constructors are deleted. This function implements the ABI notion of
+ non-trivial copy, which has diverged from the one in the standard. */
bool
-type_has_nontrivial_copy_init (const_tree t)
+type_has_nontrivial_copy_init (const_tree type)
{
- t = strip_array_types (CONST_CAST_TREE (t));
+ tree t = strip_array_types (CONST_CAST_TREE (type));
if (CLASS_TYPE_P (t))
{
gcc_assert (COMPLETE_TYPE_P (t));
- return ((TYPE_HAS_COPY_CTOR (t)
- && TYPE_HAS_COMPLEX_COPY_CTOR (t))
- || TYPE_HAS_COMPLEX_MOVE_CTOR (t));
+
+ if (TYPE_HAS_COMPLEX_COPY_CTOR (t)
+ || TYPE_HAS_COMPLEX_MOVE_CTOR (t))
+ /* Nontrivial. */
+ return true;
+
+ if (cxx_dialect < cxx11)
+ /* No deleted functions before C++11. */
+ return false;
+
+ /* Before ABI v12 we did a bitwise copy of types with only deleted
+ copy/move constructors. */
+ if (!abi_version_at_least (12)
+ && !(warn_abi && abi_version_crosses (12)))
+ return false;
+
+ bool saw_copy = false;
+ bool saw_non_deleted = false;
+
+ if (CLASSTYPE_LAZY_MOVE_CTOR (t))
+ saw_copy = saw_non_deleted = true;
+ else if (CLASSTYPE_LAZY_COPY_CTOR (t))
+ {
+ saw_copy = true;
+ if (type_has_user_declared_move_constructor (t)
+ || type_has_user_declared_move_assign (t))
+ /* [class.copy]/8 If the class definition declares a move
+ constructor or move assignment operator, the implicitly declared
+ copy constructor is defined as deleted.... */;
+ else
+ /* Any other reason the implicitly-declared function would be
+ deleted would also cause TYPE_HAS_COMPLEX_COPY_CTOR to be
+ set. */
+ saw_non_deleted = true;
+ }
+
+ if (!saw_non_deleted && CLASSTYPE_METHOD_VEC (t))
+ for (tree fns = CLASSTYPE_CONSTRUCTORS (t); fns; fns = OVL_NEXT (fns))
+ {
+ tree fn = OVL_CURRENT (fns);
+ if (copy_fn_p (fn))
+ {
+ saw_copy = true;
+ if (!DECL_DELETED_FN (fn))
+ {
+ /* Not deleted, therefore trivial. */
+ saw_non_deleted = true;
+ break;
+ }
+ }
+ }
+
+ gcc_assert (saw_copy);
+
+ if (saw_copy && !saw_non_deleted)
+ {
+ if (warn_abi && abi_version_crosses (12))
+ remember_deleted_copy (t);
+ if (abi_version_at_least (12))
+ return true;
+ }
+
+ return false;
}
else
return 0;
diff --git a/gcc/testsuite/g++.dg/abi/invisiref1.C b/gcc/testsuite/g++.dg/abi/invisiref1.C
new file mode 100644
index 0000000..5534b89
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/invisiref1.C
@@ -0,0 +1,24 @@
+// PR c++/80178
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wabi=10 -fdump-tree-gimple" }
+// { dg-final { scan-tree-dump "foo .&D" "gimple" } }
+
+struct A {
+ A();
+ A &operator=(A &&o);
+ void *p;
+};
+void notdefined(A);
+
+void foo(A) { } // { dg-warning "calling convention" }
+
+A baz() // { dg-warning "calling convention" }
+{
+ return {};
+}
+
+void bar() {
+ foo({}); // { dg-warning "calling convention" }
+ notdefined({}); // { dg-warning "calling convention" }
+ baz(); // { dg-warning "calling convention" }
+}
diff --git a/gcc/testsuite/g++.dg/abi/invisiref1a.C b/gcc/testsuite/g++.dg/abi/invisiref1a.C
new file mode 100644
index 0000000..aff7b1a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/invisiref1a.C
@@ -0,0 +1,24 @@
+// PR c++/80178
+// { dg-do compile { target c++11 } }
+// { dg-options "-fabi-version=11 -Wabi -fdump-tree-gimple" }
+// { dg-final { scan-tree-dump "foo .D" "gimple" } }
+
+struct A {
+ A();
+ A &operator=(A &&o);
+ void *p;
+};
+void notdefined(A);
+
+void foo(A) { } // { dg-warning "calling convention" }
+
+A baz() // { dg-warning "calling convention" }
+{
+ return {};
+}
+
+void bar() {
+ foo({}); // { dg-warning "calling convention" }
+ notdefined({}); // { dg-warning "calling convention" }
+ baz(); // { dg-warning "calling convention" }
+}
-------------- next part --------------
commit 827fdcd1aac10e5a9bba14338cc07ace10ea1a7a
Author: Jason Merrill <jason@redhat.com>
Date: Mon May 8 14:23:13 2017 -0400
Bump C++ ABI version.
* c-opts.c (c_common_post_options): Update defaults for
flag_abi_version and flag_abi_compat_version.
diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index ea0e01b..ed1b0377 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -909,15 +909,15 @@ c_common_post_options (const char **pfilename)
}
else if (flag_abi_compat_version == -1)
{
- /* Generate compatibility aliases for ABI v10 (6.1) by default. */
+ /* Generate compatibility aliases for ABI v11 (7.1) by default. */
flag_abi_compat_version
- = (flag_abi_version == 0 ? 10 : 0);
+ = (flag_abi_version == 0 ? 11 : 0);
}
/* Change flag_abi_version to be the actual current ABI level for the
benefit of c_cpp_builtins. */
if (flag_abi_version == 0)
- flag_abi_version = 11;
+ flag_abi_version = 12;
/* By default, enable the new inheriting constructor semantics along with ABI
11. New and old should coexist fine, but it is a change in what
diff --git a/gcc/testsuite/g++.dg/abi/macro0.C b/gcc/testsuite/g++.dg/abi/macro0.C
index 5bc2277..30eb029 100644
--- a/gcc/testsuite/g++.dg/abi/macro0.C
+++ b/gcc/testsuite/g++.dg/abi/macro0.C
@@ -1,6 +1,6 @@
// This testcase will need to be kept in sync with c_common_post_options.
// { dg-options "-fabi-version=0" }
-#if __GXX_ABI_VERSION != 1011
+#if __GXX_ABI_VERSION != 1012
#error "Incorrect value of __GXX_ABI_VERSION"
#endif
More information about the Gcc-patches
mailing list