This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [C++ PATCH 2/4] Fix conversions for built-in operator overloading candidates.
- From: Jason Merrill <jason at redhat dot com>
- To: JiangNing OS <jiangning at os dot amperecomputing dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Feng Xue OS <fxue at os dot amperecomputing dot com>
- Date: Tue, 5 Nov 2019 23:53:40 +0000
- Subject: Re: [C++ PATCH 2/4] Fix conversions for built-in operator overloading candidates.
- References: <20190916043328.3739-1-jason@redhat.com> <20190916043328.3739-2-jason@redhat.com> <CY4PR01MB2709226D39F774CBE45705699C890@CY4PR01MB2709.prod.exchangelabs.com> <CADzB+2=C4Y0gvr-9HCnYz=etzkFSKpUFUg-fuh=h4=f8aEAGag@mail.gmail.com>
I'm applying this patch again now, this time with a tweak to expmed.c
to prevent the false positive warning from breaking bootstrap. I
don't see any problem in the gimple output, so the bug must be in the
middle-end somewhere.
On Thu, Sep 19, 2019 at 8:43 PM Jason Merrill <jason@redhat.com> wrote:
>
> I've reverted this patch for the moment.
>
> On Wed, Sep 18, 2019 at 8:19 PM JiangNing OS
> <jiangning@os.amperecomputing.com> wrote:
> >
> > Hi Jason,
> >
> > This commit caused boot-strap failure on aarch64. Is it a bug? Can this be fixed ASAP?
> >
> > ../../gcc/gcc/expmed.c:5602:19: error: ���int_mode��� may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > 5602 | scalar_int_mode int_mode;
> > | ^~~~~~~~
> >
> > Thanks,
> > -Jiangning
> >
> > > -----Original Message-----
> > > From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org>
> > > On Behalf Of Jason Merrill
> > > Sent: Monday, September 16, 2019 12:33 PM
> > > To: gcc-patches@gcc.gnu.org
> > > Subject: [C++ PATCH 2/4] Fix conversions for built-in operator overloading
> > > candidates.
> > >
> > > While working on C++20 operator<=>, I noticed that build_new_op_1 was
> > > doing too much conversion when a built-in candidate was selected; the
> > > standard says it should only perform user-defined conversions, and then
> > > leave the normal operator semantics to handle any standard conversions.
> > > This is important for operator<=> because a comparison of two different
> > > unscoped enums is ill-formed; if we promote the enums to int here,
> > > cp_build_binary_op never gets to see the original operand types, so we can't
> > > give the error.
> > >
> > > Tested x86_64-pc-linux-gnu, applying to trunk.
> > >
> > > * call.c (build_new_op_1): Don't apply any standard conversions to
> > > the operands of a built-in operator. Don't suppress conversions in
> > > cp_build_unary_op.
> > > * typeck.c (cp_build_unary_op): Do integral promotions for enums.
> > > ---
> > > gcc/cp/call.c | 51 ++++++++++++++++++++++++------------------------
> > > gcc/cp/typeck.c | 4 ++--
> > > gcc/cp/ChangeLog | 7 +++++++
> > > 3 files changed, 34 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/gcc/cp/call.c b/gcc/cp/call.c index c3045d948c5..457fa6605c2
> > > 100644
> > > --- a/gcc/cp/call.c
> > > +++ b/gcc/cp/call.c
> > > @@ -6139,41 +6139,40 @@ build_new_op_1 (const op_location_t &loc,
> > > enum tree_code code, int flags,
> > > break;
> > > }
> > >
> > > - /* We need to strip any leading REF_BIND so that bitfields
> > > - don't cause errors. This should not remove any important
> > > - conversions, because builtins don't apply to class
> > > - objects directly. */
> > > + /* "If a built-in candidate is selected by overload resolution, the
> > > + operands of class type are converted to the types of the
> > > + corresponding parameters of the selected operation function,
> > > + except that the second standard conversion sequence of a
> > > + user-defined conversion sequence (12.3.3.1.2) is not applied."
> > > +*/
> > > conv = cand->convs[0];
> > > - if (conv->kind == ck_ref_bind)
> > > - conv = next_conversion (conv);
> > > - arg1 = convert_like (conv, arg1, complain);
> > > + if (conv->user_conv_p)
> > > + {
> > > + while (conv->kind != ck_user)
> > > + conv = next_conversion (conv);
> > > + arg1 = convert_like (conv, arg1, complain);
> > > + }
> > >
> > > if (arg2)
> > > {
> > > conv = cand->convs[1];
> > > - if (conv->kind == ck_ref_bind)
> > > - conv = next_conversion (conv);
> > > - else
> > > - arg2 = decay_conversion (arg2, complain);
> > > -
> > > - /* We need to call warn_logical_operator before
> > > - converting arg2 to a boolean_type, but after
> > > - decaying an enumerator to its value. */
> > > - if (complain & tf_warning)
> > > - warn_logical_operator (loc, code, boolean_type_node,
> > > - code_orig_arg1, arg1,
> > > - code_orig_arg2, arg2);
> > > -
> > > - arg2 = convert_like (conv, arg2, complain);
> > > + if (conv->user_conv_p)
> > > + {
> > > + while (conv->kind != ck_user)
> > > + conv = next_conversion (conv);
> > > + arg2 = convert_like (conv, arg2, complain);
> > > + }
> > > }
> > > +
> > > if (arg3)
> > > {
> > > conv = cand->convs[2];
> > > - if (conv->kind == ck_ref_bind)
> > > - conv = next_conversion (conv);
> > > - convert_like (conv, arg3, complain);
> > > + if (conv->user_conv_p)
> > > + {
> > > + while (conv->kind != ck_user)
> > > + conv = next_conversion (conv);
> > > + arg3 = convert_like (conv, arg3, complain);
> > > + }
> > > }
> > > -
> > > }
> > > }
> > >
> > > @@ -6241,7 +6240,7 @@ build_new_op_1 (const op_location_t &loc, enum
> > > tree_code code, int flags,
> > > case REALPART_EXPR:
> > > case IMAGPART_EXPR:
> > > case ABS_EXPR:
> > > - return cp_build_unary_op (code, arg1, candidates != 0, complain);
> > > + return cp_build_unary_op (code, arg1, false, complain);
> > >
> > > case ARRAY_REF:
> > > return cp_build_array_ref (input_location, arg1, arg2, complain); diff --git
> > > a/gcc/cp/typeck.c b/gcc/cp/typeck.c index 70094d1b426..620f2c9afdf 100644
> > > --- a/gcc/cp/typeck.c
> > > +++ b/gcc/cp/typeck.c
> > > @@ -6242,7 +6242,7 @@ cp_build_unary_op (enum tree_code code, tree
> > > xarg, bool noconvert,
> > > : _("wrong type argument to unary plus"));
> > > else
> > > {
> > > - if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
> > > + if (!noconvert && INTEGRAL_OR_ENUMERATION_TYPE_P
> > > (TREE_TYPE
> > > +(arg)))
> > > arg = cp_perform_integral_promotions (arg, complain);
> > >
> > > /* Make sure the result is not an lvalue: a unary plus or minus @@
> > > -6267,7 +6267,7 @@ cp_build_unary_op (enum tree_code code, tree xarg,
> > > bool noconvert,
> > > |
> > > WANT_VECTOR_OR_COMPLEX,
> > > arg, true)))
> > > errstring = _("wrong type argument to bit-complement");
> > > - else if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
> > > + else if (!noconvert && INTEGRAL_OR_ENUMERATION_TYPE_P
> > > (TREE_TYPE
> > > + (arg)))
> > > {
> > > /* Warn if the expression has boolean value. */
> > > if (TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE diff --git
> > > a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index e92d49f1b76..a03a428109b
> > > 100644
> > > --- a/gcc/cp/ChangeLog
> > > +++ b/gcc/cp/ChangeLog
> > > @@ -1,3 +1,10 @@
> > > +2019-09-15 Jason Merrill <jason@redhat.com>
> > > +
> > > + * call.c (build_new_op_1): Don't apply any standard conversions to
> > > + the operands of a built-in operator. Don't suppress conversions in
> > > + cp_build_unary_op.
> > > + * typeck.c (cp_build_unary_op): Do integral promotions for enums.
> > > +
> > > 2019-09-15 Marek Polacek <polacek@redhat.com>
> > >
> > > PR c++/91740 - ICE with constexpr call and ?: in ARRAY_REF.
> > > --
> > > 2.21.0
> >
commit b7924099ce49704c98e19f7385fa8e7daec3beb2
Author: jason <jason@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Mon Sep 16 04:34:18 2019 +0000
Fix conversions for built-in operator overloading candidates.
While working on C++20 operator<=>, I noticed that build_new_op_1 was doing
too much conversion when a built-in candidate was selected; the standard
says it should only perform user-defined conversions, and then leave the
normal operator semantics to handle any standard conversions. This is
important for operator<=> because a comparison of two different unscoped
enums is ill-formed; if we promote the enums to int here, cp_build_binary_op
never gets to see the original operand types, so we can't give the error.
I'm also disabling -Wmaybe-uninitialized for expmed.c to avoid the bootstrap
failure from the last time I applied this patch.
* call.c (build_new_op_1): Don't apply any standard conversions to
the operands of a built-in operator. Don't suppress conversions in
cp_build_unary_op.
* typeck.c (cp_build_unary_op): Do integral promotions for enums.
PR tree-optimization/91825
* expmed.c: Reduce -Wmaybe-uninitialized to warning.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 2a1da3cc4e1..390a4c581e2 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6156,41 +6156,40 @@ build_new_op_1 (const op_location_t &loc, enum tree_code code, int flags,
break;
}
- /* We need to strip any leading REF_BIND so that bitfields
- don't cause errors. This should not remove any important
- conversions, because builtins don't apply to class
- objects directly. */
+ /* "If a built-in candidate is selected by overload resolution, the
+ operands of class type are converted to the types of the
+ corresponding parameters of the selected operation function,
+ except that the second standard conversion sequence of a
+ user-defined conversion sequence (12.3.3.1.2) is not applied." */
conv = cand->convs[0];
- if (conv->kind == ck_ref_bind)
- conv = next_conversion (conv);
- arg1 = convert_like (conv, arg1, complain);
+ if (conv->user_conv_p)
+ {
+ while (conv->kind != ck_user)
+ conv = next_conversion (conv);
+ arg1 = convert_like (conv, arg1, complain);
+ }
if (arg2)
{
conv = cand->convs[1];
- if (conv->kind == ck_ref_bind)
- conv = next_conversion (conv);
- else
- arg2 = decay_conversion (arg2, complain);
-
- /* We need to call warn_logical_operator before
- converting arg2 to a boolean_type, but after
- decaying an enumerator to its value. */
- if (complain & tf_warning)
- warn_logical_operator (loc, code, boolean_type_node,
- code_orig_arg1, arg1,
- code_orig_arg2, arg2);
-
- arg2 = convert_like (conv, arg2, complain);
+ if (conv->user_conv_p)
+ {
+ while (conv->kind != ck_user)
+ conv = next_conversion (conv);
+ arg2 = convert_like (conv, arg2, complain);
+ }
}
+
if (arg3)
{
conv = cand->convs[2];
- if (conv->kind == ck_ref_bind)
- conv = next_conversion (conv);
- convert_like (conv, arg3, complain);
+ if (conv->user_conv_p)
+ {
+ while (conv->kind != ck_user)
+ conv = next_conversion (conv);
+ arg3 = convert_like (conv, arg3, complain);
+ }
}
-
}
}
@@ -6258,7 +6257,7 @@ build_new_op_1 (const op_location_t &loc, enum tree_code code, int flags,
case REALPART_EXPR:
case IMAGPART_EXPR:
case ABS_EXPR:
- return cp_build_unary_op (code, arg1, candidates != 0, complain);
+ return cp_build_unary_op (code, arg1, false, complain);
case ARRAY_REF:
return cp_build_array_ref (input_location, arg1, arg2, complain);
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index bf2502a3686..50240537938 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -6298,7 +6298,7 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool noconvert,
: _("wrong type argument to unary plus"));
else
{
- if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
+ if (!noconvert && INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (arg)))
arg = cp_perform_integral_promotions (arg, complain);
/* Make sure the result is not an lvalue: a unary plus or minus
@@ -6323,7 +6323,7 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool noconvert,
| WANT_VECTOR_OR_COMPLEX,
arg, true)))
errstring = _("wrong type argument to bit-complement");
- else if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
+ else if (!noconvert && INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (arg)))
{
/* Warn if the expression has boolean value. */
if (TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
diff --git a/gcc/expmed.c b/gcc/expmed.c
index f1975fe33fe..ff8554b1562 100644
--- a/gcc/expmed.c
+++ b/gcc/expmed.c
@@ -18,6 +18,8 @@ You should have received a copy of the GNU General Public License
along with GCC; see the file COPYING3. If not see
<http://www.gnu.org/licenses/>. */
+/* Work around tree-optimization/91825. */
+#pragma GCC diagnostic warning "-Wmaybe-uninitialized"
#include "config.h"
#include "system.h"
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 4312d125061..8ee23dd5986 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@
+2019-10-26 Jason Merrill <jason@redhat.com>
+
+ PR tree-optimization/91825
+ * expmed.c: Reduce -Wmaybe-uninitialized to warning.
+
2019-11-05 Jim Wilson <jimw@sifive.com>
PR middle-end/92263
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 2085319bbc9..560896a1dd8 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,10 @@
+2019-09-15 Jason Merrill <jason@redhat.com>
+
+ * call.c (build_new_op_1): Don't apply any standard conversions to
+ the operands of a built-in operator. Don't suppress conversions in
+ cp_build_unary_op.
+ * typeck.c (cp_build_unary_op): Do integral promotions for enums.
+
2019-11-04 Jason Merrill <jason@redhat.com>
Use vec instead of raw array for built-in candidates.