Re: C++ PATCH for c++/86094, wrong calling convention for move-only class
On Wed, Jun 13, 2018 at 5:05 PM, Jason Merrill wrote:
> On Wed, Jun 13, 2018 at 4:12 PM, Jason Merrill wrote:
>> On Mon, Jun 11, 2018 at 2:38 PM, Jason Merrill wrote:
>>> The fix for 80178 was broken, because I forgot that copy_fn_p is false
>>> for move constructors. As a result, the calling convention for a
>>> class with a trivial move constructor and deleted copy constructor
>>> changed inappropriately.
>>
>> This patch restores the broken behavior to -fabi-version=12 and adds
>> -fabi-version=13 for the fix; people can use -Wabi=12 with GCC 8.2 to
>> check for compatibility issues against 8.1, or -Wabi=11 to check
>> compatibility with GCC 7.
>>
>> Tested x86_64-pc-linux-gnu, applying to trunk and 8. Do we want to
>> accelerate 8.2 because of this issue?
>
> And one more patch, to suggest -Wabi=11 rather than useless plain -Wabi.
...and another improvement to the warning.
Tested x86_64-pc-linux-gnu, applying to trunk and 8.
commit 3825732dc47c121f0c2d08dfa1993d0cf008ce89
Author: Jason Merrill
Date: Thu Jun 14 14:36:20 2018 -0400
* tree.c (maybe_warn_parm_abi): Inform the location of the class.
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 48a0ff37372..a88481db1e0 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -4074,25 +4074,23 @@ maybe_warn_parm_abi (tree t, location_t loc)
if ((flag_abi_version == 12 || warn_abi_version == 12)
&& classtype_has_non_deleted_move_ctor (t))
{
+ bool w;
if (flag_abi_version > 12)
- warning_at (loc, OPT_Wabi, "-fabi-version=13 (GCC 8.2) fixes the "
- "calling convention for %qT, which was accidentally "
- "changed in 8.1", t);
+ w = warning_at (loc, OPT_Wabi, "-fabi-version=13 (GCC 8.2) fixes the "
+ "calling convention for %qT, which was accidentally "
+ "changed in 8.1", t);
else
- warning_at (loc, OPT_Wabi, "-fabi-version=12 (GCC 8.1) accidentally "
- "changes the calling convention for %qT", t);
+ w = warning_at (loc, OPT_Wabi, "-fabi-version=12 (GCC 8.1) accident"
+ "ally changes the calling convention for %qT", t);
+ if (w)
+ inform (location_of (t), " declared here");
return;
}
- warning_at (loc, OPT_Wabi, "the calling convention for %qT changes in "
- "-fabi-version=13 (GCC 8.2)", t);
- static bool explained = false;
- if (!explained)
-{
- inform (loc, " because all of its copy and move constructors "
- "are deleted");
- explained = true;
-}
+ if (warning_at (loc, OPT_Wabi, "the calling convention for %qT changes in "
+ "-fabi-version=13 (GCC 8.2)", t))
+inform (location_of (t), " because all of its copy and move "
+ "constructors are deleted");
}
/* Returns true iff copying an object of type T (including via move
diff --git a/gcc/testsuite/g++.dg/abi/invisiref2a.C b/gcc/testsuite/g++.dg/abi/invisiref2a.C
index 05330553287..127ee0ae763 100644
--- a/gcc/testsuite/g++.dg/abi/invisiref2a.C
+++ b/gcc/testsuite/g++.dg/abi/invisiref2a.C
@@ -3,7 +3,7 @@
// { dg-additional-options "-fabi-version=12 -Wabi -fdump-tree-gimple" }
// { dg-final { scan-tree-dump "struct S &" "gimple" } }
-struct S {
+struct S { // { dg-message "" }
S(S&&) = default;
int i;
};
Re: C++ PATCH for c++/86094, wrong calling convention for move-only class
On Wed, Jun 13, 2018 at 5:08 PM, Jakub Jelinek wrote: > > On Wed, Jun 13, 2018 at 04:12:25PM -0400, Jason Merrill wrote: > > On Mon, Jun 11, 2018 at 2:38 PM, Jason Merrill wrote: > > > The fix for 80178 was broken, because I forgot that copy_fn_p is false > > > for move constructors. As a result, the calling convention for a > > > class with a trivial move constructor and deleted copy constructor > > > changed inappropriately. > > > > This patch restores the broken behavior to -fabi-version=12 and adds > > -fabi-version=13 for the fix; people can use -Wabi=12 with GCC 8.2 to > > check for compatibility issues against 8.1, or -Wabi=11 to check > > compatibility with GCC 7. > > > > Tested x86_64-pc-linux-gnu, applying to trunk and 8. Do we want to > > accelerate 8.2 because of this issue? > > I'd like to see the powerpc64le-linux __ieee128 long double support in 8.2 > too, accelerating 8.2 because of this would make that impossible. I'd think we could put that support in an August release regardless of what version number is on that release. Is it important that it be numbered 8.2? Jason
Re: C++ PATCH for c++/86094, wrong calling convention for move-only class
On Wed, Jun 13, 2018 at 04:12:25PM -0400, Jason Merrill wrote: > On Mon, Jun 11, 2018 at 2:38 PM, Jason Merrill wrote: > > The fix for 80178 was broken, because I forgot that copy_fn_p is false > > for move constructors. As a result, the calling convention for a > > class with a trivial move constructor and deleted copy constructor > > changed inappropriately. > > This patch restores the broken behavior to -fabi-version=12 and adds > -fabi-version=13 for the fix; people can use -Wabi=12 with GCC 8.2 to > check for compatibility issues against 8.1, or -Wabi=11 to check > compatibility with GCC 7. > > Tested x86_64-pc-linux-gnu, applying to trunk and 8. Do we want to > accelerate 8.2 because of this issue? I'd like to see the powerpc64le-linux __ieee128 long double support in 8.2 too, accelerating 8.2 because of this would make that impossible. Jakub
Re: C++ PATCH for c++/86094, wrong calling convention for move-only class
On Wed, Jun 13, 2018 at 4:12 PM, Jason Merrill wrote:
> On Mon, Jun 11, 2018 at 2:38 PM, Jason Merrill wrote:
>> The fix for 80178 was broken, because I forgot that copy_fn_p is false
>> for move constructors. As a result, the calling convention for a
>> class with a trivial move constructor and deleted copy constructor
>> changed inappropriately.
>
> This patch restores the broken behavior to -fabi-version=12 and adds
> -fabi-version=13 for the fix; people can use -Wabi=12 with GCC 8.2 to
> check for compatibility issues against 8.1, or -Wabi=11 to check
> compatibility with GCC 7.
>
> Tested x86_64-pc-linux-gnu, applying to trunk and 8. Do we want to
> accelerate 8.2 because of this issue?
And one more patch, to suggest -Wabi=11 rather than useless plain -Wabi.
Tested x86_64-pc-linux-gnu, applying to trunk and 8.
commit bf7773e6fa607769b619535e1b22272a36726868
Author: Jason Merrill
Date: Wed Jun 13 16:09:46 2018 -0400
* c-opts.c (c_common_post_options): Warn about useless -Wabi.
(c_common_handle_option) [OPT_Wabi_]: Remove flag_abi_compat_version
handling.
diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index 107359ec20d..bbcb1bb1a9c 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -417,8 +417,6 @@ c_common_handle_option (size_t scode, const char *arg, int value,
value = 2;
}
warn_abi_version = value;
- if (flag_abi_compat_version == -1)
- flag_abi_compat_version = value;
break;
case OPT_fcanonical_system_headers:
@@ -942,7 +940,17 @@ c_common_post_options (const char **pfilename)
{
warn_abi_version = latest_abi_version;
if (flag_abi_version == latest_abi_version)
- flag_abi_compat_version = abi_compat_default;
+ {
+ if (warning (OPT_Wabi, "-Wabi won't warn about anything"))
+ {
+ inform (input_location, "-Wabi warns about differences "
+ "from the most up-to-date ABI, which is also used "
+ "by default");
+ inform (input_location, "use e.g. -Wabi=11 to warn about "
+ "changes from GCC 7");
+ }
+ flag_abi_compat_version = abi_compat_default;
+ }
else
flag_abi_compat_version = latest_abi_version;
}
diff --git a/gcc/testsuite/g++.dg/abi/pr83489.C b/gcc/testsuite/g++.dg/abi/pr83489.C
index aee04eca9d8..b6c82ef16e6 100644
--- a/gcc/testsuite/g++.dg/abi/pr83489.C
+++ b/gcc/testsuite/g++.dg/abi/pr83489.C
@@ -1,5 +1,5 @@
// PR c++/83489
-// { dg-options "-Wabi" }
+// { dg-options "-Wabi=11" }
struct A
{
diff --git a/gcc/testsuite/obj-c++.dg/bitfield-1.mm b/gcc/testsuite/obj-c++.dg/bitfield-1.mm
index a63761904b5..084d2cb0916 100644
--- a/gcc/testsuite/obj-c++.dg/bitfield-1.mm
+++ b/gcc/testsuite/obj-c++.dg/bitfield-1.mm
@@ -5,7 +5,7 @@
superclasses should be removed). */
/* Contributed by Ziemowit Laski . */
/* { dg-do run } */
-/* { dg-options "-Wpadded -Wabi" } */
+/* { dg-options "-Wpadded -Wabi=8" } */
/* Leave blank lines here to keep warnings on the same lines. */
diff --git a/gcc/testsuite/obj-c++.dg/layout-1.mm b/gcc/testsuite/obj-c++.dg/layout-1.mm
index 33879ad17a4..89921236ec3 100644
--- a/gcc/testsuite/obj-c++.dg/layout-1.mm
+++ b/gcc/testsuite/obj-c++.dg/layout-1.mm
@@ -1,7 +1,7 @@
/* Ensure that we do not get bizarre warnings referring to
__attribute__((packed)) or some such. */
/* { dg-do compile } */
-/* { dg-options "-Wpadded -Wpacked -Wabi" } */
+/* { dg-options "-Wpadded -Wpacked -Wabi=8" } */
#include "../objc-obj-c++-shared/TestsuiteObject.h"
Re: C++ PATCH for c++/86094, wrong calling convention for move-only class
On Mon, Jun 11, 2018 at 2:38 PM, Jason Merrill wrote:
> The fix for 80178 was broken, because I forgot that copy_fn_p is false
> for move constructors. As a result, the calling convention for a
> class with a trivial move constructor and deleted copy constructor
> changed inappropriately.
This patch restores the broken behavior to -fabi-version=12 and adds
-fabi-version=13 for the fix; people can use -Wabi=12 with GCC 8.2 to
check for compatibility issues against 8.1, or -Wabi=11 to check
compatibility with GCC 7.
Tested x86_64-pc-linux-gnu, applying to trunk and 8. Do we want to
accelerate 8.2 because of this issue?
commit f118d95dd7802a80387982135686be27b3e0dbb7
Author: Jason Merrill
Date: Wed Jun 13 13:35:29 2018 -0400
PR c++/86094 - wrong code with defaulted move ctor.
gcc/c-family/
* c-opts.c (c_common_post_options): Bump the current ABI version to
13. Set warn_abi_version and flag_abi_compat_version to the current
version rather than 0. Fix defaulting flag_abi_compat_version from
warn_abi_version.
gcc/cp/
* class.c (classtype_has_non_deleted_move_ctor): New.
* tree.c (maybe_warn_parm_abi, type_has_nontrivial_copy_init):
Handle v12 breakage.
diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index ddaaef37e1d..107359ec20d 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -915,31 +915,38 @@ c_common_post_options (const char **pfilename)
if (flag_declone_ctor_dtor == -1)
flag_declone_ctor_dtor = optimize_size;
- if (warn_abi_version == -1)
-{
- if (flag_abi_compat_version != -1)
- warn_abi_version = flag_abi_compat_version;
- else
- warn_abi_version = 0;
-}
-
if (flag_abi_compat_version == 1)
{
warning (0, "%<-fabi-compat-version=1%> is not supported, using =2");
flag_abi_compat_version = 2;
}
- else if (flag_abi_compat_version == -1)
+
+ /* Change flag_abi_version to be the actual current ABI level, for the
+ benefit of c_cpp_builtins, and to make comparison simpler. */
+ const int latest_abi_version = 13;
+ /* Generate compatibility aliases for ABI v11 (7.1) by default. */
+ const int abi_compat_default = 11;
+
+#define clamp(X) if (X == 0 || X > latest_abi_version) X = latest_abi_version
+ clamp (flag_abi_version);
+ clamp (warn_abi_version);
+ clamp (flag_abi_compat_version);
+#undef clamp
+
+ /* Default -Wabi= or -fabi-compat-version= from each other. */
+ if (warn_abi_version == -1 && flag_abi_compat_version != -1)
+warn_abi_version = flag_abi_compat_version;
+ else if (flag_abi_compat_version == -1 && warn_abi_version != -1)
+flag_abi_compat_version = warn_abi_version;
+ else if (warn_abi_version == -1 && flag_abi_compat_version == -1)
{
- /* Generate compatibility aliases for ABI v11 (7.1) by default. */
- flag_abi_compat_version
- = (flag_abi_version == 0 ? 11 : 0);
+ warn_abi_version = latest_abi_version;
+ if (flag_abi_version == latest_abi_version)
+ flag_abi_compat_version = abi_compat_default;
+ else
+ flag_abi_compat_version = latest_abi_version;
}
- /* 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 = 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
artificial symbols are generated. */
diff --git a/gcc/common.opt b/gcc/common.opt
index 4aebcaf0656..efbeba33819 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -938,7 +938,11 @@ Driver Undocumented
;
; 12: Corrects the calling convention for classes with only deleted copy/move
; constructors and changes passing/returning of empty records.
-; Default in G++ 8.
+; Default in G++ 8.1.
+;
+; 13: Fixes the accidental change in 12 to the calling convention for classes
+; with deleted copy constructor and trivial move constructor.
+; Default in G++ 8.2.
;
; Additional positive integers will be assigned as new versions of
; the ABI become the default version of the ABI.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index b6e78c6377d..9a397e3856c 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -5175,6 +5175,19 @@ classtype_has_move_assign_or_move_ctor_p (tree t, bool user_p)
return false;
}
+/* True iff T has a move constructor that is not deleted. */
+
+bool
+classtype_has_non_deleted_move_ctor (tree t)
+{
+ if (CLASSTYPE_LAZY_MOVE_CTOR (t))
+lazily_declare_fn (sfk_move_constructor, t);
+ for (ovl_iterator iter (CLASSTYPE_CONSTRUCTORS (t)); iter; ++iter)
+if (move_fn_p (*iter) && !DECL_DELETED_FN (*iter))
+ return true;
+ return false;
+}
+
/* If T, a class, has a user-provided copy constructor, copy assignment
operator, or destructor, returns that function. Otherwise, null. */
diff --git
C++ PATCH for c++/86094, wrong calling convention for move-only class
The fix for 80178 was broken, because I forgot that copy_fn_p is false
for move constructors. As a result, the calling convention for a
class with a trivial move constructor and deleted copy constructor
changed inappropriately.
Tested x86_64-pc-linux-gnu, applying to trunk and 8.
commit d6cc7705f626778cb7e19fccf90117e7ed47d794
Author: Jason Merrill
Date: Mon Jun 11 12:14:27 2018 -0400
PR c++/86094 - wrong code with defaulted move ctor.
* tree.c (type_has_nontrivial_copy_init): Fix move ctor handling.
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index bbbda7e98b6..156d1e469c6 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -4135,7 +4135,7 @@ type_has_nontrivial_copy_init (const_tree type)
for (ovl_iterator iter (CLASSTYPE_CONSTRUCTORS (t)); iter; ++iter)
{
tree fn = *iter;
- if (copy_fn_p (fn))
+ if (copy_fn_p (fn) || move_fn_p (fn))
{
saw_copy = true;
if (!DECL_DELETED_FN (fn))
diff --git a/gcc/testsuite/g++.dg/abi/invisiref2.C b/gcc/testsuite/g++.dg/abi/invisiref2.C
new file mode 100644
index 000..592d212ab11
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/invisiref2.C
@@ -0,0 +1,14 @@
+// PR c++/86094
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-Wabi=11 -fdump-tree-gimple" }
+// { dg-final { scan-tree-dump-not "struct S &" "gimple" } }
+
+struct S {
+ S(S&&) = default;
+ int i;
+};
+
+S foo(S s)
+{
+ return s;
+}
