Re: [PATCH] c++, v2: Implement C++26 P2864R2 - Remove Deprecated Arithmetic Conversion on Enumerations From C++26

[Jason Merrill <jason@redhat.com>]

On 11/14/23 12:38, Jakub Jelinek wrote:
> On Mon, Nov 13, 2023 at 10:59:52PM -0500, Jason Merrill wrote:
>> On 11/13/23 06:50, Jakub Jelinek wrote:
>>> The following patch implements C++26 P2864R2 by emitting pedwarn enabled by
>>> the same options as the C++20 and later warnings (i.e. -Wenum-compare,
>>> -Wdeprecated-enum-enum-conversion and -Wdeprecated-enum-float-conversion
>>> which are all enabled by default).  I think we still want to allow users
>>> some option workaround, so am not using directly error, but if that is
>>> what you want instead, I can change it.
>>
>> I agree, but we also need to return error_mark_node for these cases when
>> SFINAE, i.e. !(complain & tf_warning_or_error)
> 
> So like this then?
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux.
> 
> 2023-11-14  Jakub Jelinek  <jakub@redhat.com>
> 
> gcc/cp/
> 	* typeck.cc: Implement C++26 P2864R2 - Remove Deprecated Arithmetic
> 	Conversion on Enumerations From C++26.
> 	(do_warn_enum_conversions): Return bool rather than void, add COMPLAIN
> 	argument.  Use pedwarn rather than warning_at for C++26 and remove
> 	" is deprecated" part of the diagnostics in that case.  For SFINAE
> 	in C++26 return true on newly erroneous cases.
> 	(cp_build_binary_op): For C++26 call do_warn_enum_conversions
> 	unconditionally, pass complain argument to it and if it returns true,
> 	return error_mark_node.
> 	* call.cc (build_conditional_expr): Use pedwarn rather than warning_at
> 	for C++26 and remove " is deprecated" part of the diagnostics in that
> 	case and check for complain & tf_warning_or_error.  Use emit_diagnostic
> 	with cxx_dialect >= cxx26 ? DK_PEDWARN : DK_WARNING.  For SFINAE in
> 	C++26 return error_mark_node on newly erroneous cases.
> 	(build_new_op): Use emit_diagnostic with cxx_dialect >= cxx26
> 	? DK_PEDWARN : DK_WARNING and complain & tf_warning_or_error check
> 	for C++26.  For SFINAE in C++26 return error_mark_node on newly
> 	erroneous cases.
> gcc/testsuite/
> 	* g++.dg/cpp26/enum-conv1.C: New test.
> 	* g++.dg/cpp2a/enum-conv1.C: Adjust expected diagnostics in C++26.
> 	* g++.dg/diagnostic/enum3.C: Likewise.
> 	* g++.dg/parse/attr3.C: Likewise.
> 	* g++.dg/cpp0x/linkage2.C: Likewise.
> 
> --- gcc/cp/typeck.cc.jj	2023-11-13 13:02:21.573785549 +0100
> +++ gcc/cp/typeck.cc	2023-11-14 09:49:31.026997048 +0100
> @@ -4940,16 +4940,25 @@ warn_for_null_address (location_t locati
>      type, this behavior is deprecated ([depr.arith.conv.enum]).  CODE is the
>      code of the binary operation, TYPE0 and TYPE1 are the types of the operands,
>      and LOC is the location for the whole binary expression.
> +   For C++26 this is ill-formed rather than deprecated.
> +   Return true for SFINAE errors.
>      TODO: Consider combining this with -Wenum-compare in build_new_op_1.  */
>   
> -static void
> +static bool
>   do_warn_enum_conversions (location_t loc, enum tree_code code, tree type0,
> -			  tree type1)
> +			  tree type1, tsubst_flags_t complain)
>   {
>     if (TREE_CODE (type0) == ENUMERAL_TYPE
>         && TREE_CODE (type1) == ENUMERAL_TYPE
>         && TYPE_MAIN_VARIANT (type0) != TYPE_MAIN_VARIANT (type1))
>       {
> +      if (cxx_dialect >= cxx26)
> +	{
> +	  if ((complain & tf_warning_or_error) == 0)
> +	    return true;
> +	}
> +      else if ((complain & tf_warning) == 0)
> +	return false;
>         /* In C++20, -Wdeprecated-enum-enum-conversion is on by default.
>   	 Otherwise, warn if -Wenum-conversion is on.  */
>         enum opt_code opt;
> @@ -4958,7 +4967,7 @@ do_warn_enum_conversions (location_t loc
>         else if (warn_enum_conversion)
>   	opt = OPT_Wenum_conversion;
>         else
> -	return;
> +	return false;
>   
>         switch (code)
>   	{
> @@ -4969,21 +4978,29 @@ do_warn_enum_conversions (location_t loc
>   	case EQ_EXPR:
>   	case NE_EXPR:
>   	  /* Comparisons are handled by -Wenum-compare.  */
> -	  return;
> +	  return false;
>   	case SPACESHIP_EXPR:
>   	  /* This is invalid, don't warn.  */
> -	  return;
> +	  return false;
>   	case BIT_AND_EXPR:
>   	case BIT_IOR_EXPR:
>   	case BIT_XOR_EXPR:
> -	  warning_at (loc, opt, "bitwise operation between different "
> -		      "enumeration types %qT and %qT is deprecated",
> -		      type0, type1);
> -	  return;
> +	  if (cxx_dialect >= cxx26)
> +	    pedwarn (loc, opt, "bitwise operation between different "
> +		     "enumeration types %qT and %qT", type0, type1);
> +	  else
> +	    warning_at (loc, opt, "bitwise operation between different "
> +			"enumeration types %qT and %qT is deprecated",
> +			type0, type1);
> +	  return false;
>   	default:
> -	  warning_at (loc, opt, "arithmetic between different enumeration "
> -		      "types %qT and %qT is deprecated", type0, type1);
> -	  return;
> +	  if (cxx_dialect >= cxx26)
> +	    pedwarn (loc, opt, "arithmetic between different enumeration "
> +		     "types %qT and %qT", type0, type1);
> +	  else
> +	    warning_at (loc, opt, "arithmetic between different enumeration "
> +			"types %qT and %qT is deprecated", type0, type1);
> +	  return false;
>   	}
>       }
>     else if ((TREE_CODE (type0) == ENUMERAL_TYPE
> @@ -4991,6 +5008,13 @@ do_warn_enum_conversions (location_t loc
>   	   || (SCALAR_FLOAT_TYPE_P (type0)
>   	       && TREE_CODE (type1) == ENUMERAL_TYPE))
>       {
> +      if (cxx_dialect >= cxx26)
> +	{
> +	  if ((complain & tf_warning_or_error) == 0)
> +	    return true;
> +	}
> +      else if ((complain & tf_warning) == 0)
> +	return false;
>         const bool enum_first_p = TREE_CODE (type0) == ENUMERAL_TYPE;
>         /* In C++20, -Wdeprecated-enum-float-conversion is on by default.
>   	 Otherwise, warn if -Wenum-conversion is on.  */
> @@ -5000,7 +5024,7 @@ do_warn_enum_conversions (location_t loc
>         else if (warn_enum_conversion)
>   	opt = OPT_Wenum_conversion;
>         else
> -	return;
> +	return false;
>   
>         switch (code)
>   	{
> @@ -5010,7 +5034,13 @@ do_warn_enum_conversions (location_t loc
>   	case LE_EXPR:
>   	case EQ_EXPR:
>   	case NE_EXPR:
> -	  if (enum_first_p)
> +	  if (enum_first_p && cxx_dialect >= cxx26)
> +	    pedwarn (loc, opt, "comparison of enumeration type %qT with "
> +		     "floating-point type %qT", type0, type1);
> +	  else if (cxx_dialect >= cxx26)
> +	    pedwarn (loc, opt, "comparison of floating-point type %qT "
> +		      "with enumeration type %qT", type0, type1);
> +	  else if (enum_first_p)
>   	    warning_at (loc, opt, "comparison of enumeration type %qT with "
>   			"floating-point type %qT is deprecated",
>   			type0, type1);
> @@ -5018,12 +5048,18 @@ do_warn_enum_conversions (location_t loc
>   	    warning_at (loc, opt, "comparison of floating-point type %qT "
>   			"with enumeration type %qT is deprecated",
>   			type0, type1);
> -	  return;
> +	  return false;
>   	case SPACESHIP_EXPR:
>   	  /* This is invalid, don't warn.  */
> -	  return;
> +	  return false;
>   	default:
> -	  if (enum_first_p)
> +	  if (enum_first_p && cxx_dialect >= cxx26)
> +	    pedwarn (loc, opt, "arithmetic between enumeration type %qT "
> +		     "and floating-point type %qT", type0, type1);
> +	  else if (cxx_dialect >= cxx26)
> +	    pedwarn (loc, opt, "arithmetic between floating-point type %qT "
> +		     "and enumeration type %qT", type0, type1);
> +	  else if (enum_first_p)
>   	    warning_at (loc, opt, "arithmetic between enumeration type %qT "
>   			"and floating-point type %qT is deprecated",
>   			type0, type1);
> @@ -5031,9 +5067,10 @@ do_warn_enum_conversions (location_t loc
>   	    warning_at (loc, opt, "arithmetic between floating-point type %qT "
>   			"and enumeration type %qT is deprecated",
>   			type0, type1);
> -	  return;
> +	  return true;

Shouldn't this be return false?

OK with that change.

Jason