public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jason Merrill <jason@redhat.com>
To: JiangNing OS <jiangning@os.amperecomputing.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
	Feng Xue OS <fxue@os.amperecomputing.com>
Subject: Re: [C++ PATCH 2/4] Fix conversions for built-in operator overloading candidates.
Date: Thu, 19 Sep 2019 20:11:00 -0000	[thread overview]
Message-ID: <CADzB+2mzUucnovY8e2AON5W7+5Avy9ZYznptj43uChsHJhW-Dw@mail.gmail.com> (raw)
In-Reply-To: <CADzB+2=C4Y0gvr-9HCnYz=etzkFSKpUFUg-fuh=h4=f8aEAGag@mail.gmail.com>

Do any of you have a reproducer for this?

On Thu, Sep 19, 2019 at 3: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
> >

  reply	other threads:[~2019-09-19 20:11 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-16  4:33 [C++ PATCH 1/4] Handle location wrappers better in warn_logical_operator Jason Merrill
2019-09-16  4:33 ` [C++ PATCH 4/4] PR c++/30277 - int-width bit-field promotion Jason Merrill
2019-09-21  6:23   ` Jakub Jelinek
2019-09-21 21:51     ` Jason Merrill
2019-09-16  4:33 ` [C++ PATCH 3/4] PR c++/82165 - enum bitfields and operator overloading Jason Merrill
2019-09-16  4:33 ` [C++ PATCH 2/4] Fix conversions for built-in operator overloading candidates Jason Merrill
2019-09-17 14:07   ` Andreas Schwab
2019-09-17 14:19     ` Iain Sandoe
2019-09-19  0:19   ` JiangNing OS
2019-09-19 19:43     ` Jason Merrill
2019-09-19 20:11       ` Jason Merrill [this message]
2019-09-19 20:28         ` Marek Polacek
2019-11-05 23:53       ` Jason Merrill

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CADzB+2mzUucnovY8e2AON5W7+5Avy9ZYznptj43uChsHJhW-Dw@mail.gmail.com \
    --to=jason@redhat.com \
    --cc=fxue@os.amperecomputing.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jiangning@os.amperecomputing.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).