diff mbox series

[C++,2/4] Fix conversions for built-in operator overloading candidates.

Message ID 20190916043328.3739-2-jason@redhat.com
State New
Headers show
Series [C++,1/4] Handle location wrappers better in warn_logical_operator. | expand

Commit Message

Jason Merrill Sept. 16, 2019, 4:33 a.m. UTC 
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(-)

Comments

Andreas Schwab Sept. 17, 2019, 2:06 p.m. UTC | #1 
This breaks bootstrap on aarch64 (during stage2 build):

../../gcc/expmed.c: In function 'rtx_def* emit_store_flag_1(rtx, rtx_code, rtx, rtx, machine_mode, int, int, machine_mode)':
../../gcc/expmed.c:5602:19: error: 'int_mode' may be used uninitialized in this function [-Werror=maybe-uninitialized]
 5602 |   scalar_int_mode int_mode;
      |                   ^~~~~~~~

Andreas.
Iain Sandoe Sept. 17, 2019, 2:19 p.m. UTC | #2 
> On 17 Sep 2019, at 15:06, Andreas Schwab <schwab@suse.de> wrote:
> 
> This breaks bootstrap on aarch64 (during stage2 build):

for the record, also on x86_64-darwin1x (attempting some analysis).
Iain

> 
> ../../gcc/expmed.c: In function 'rtx_def* emit_store_flag_1(rtx, rtx_code, rtx, rtx, machine_mode, int, int, machine_mode)':
> ../../gcc/expmed.c:5602:19: error: 'int_mode' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 5602 |   scalar_int_mode int_mode;
>      |                   ^~~~~~~~
> 
> Andreas.
> 
> -- 
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
JiangNing OS Sept. 19, 2019, 12:19 a.m. UTC | #3 
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
Jason Merrill Sept. 19, 2019, 7:43 p.m. UTC | #4 
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
>
Jason Merrill Sept. 19, 2019, 8:11 p.m. UTC | #5 
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
> >
Marek Polacek Sept. 19, 2019, 8:28 p.m. UTC | #6 
On Thu, Sep 19, 2019 at 04:11:16PM -0400, Jason Merrill wrote:
> Do any of you have a reproducer for this?

I've attached it to 91825.

Marek
Jason Merrill Nov. 5, 2019, 11:53 p.m. UTC | #7 
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
> >
diff mbox series

Patch

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.