[PATCH v2] c++: P2865R5, Remove Deprecated Array Comparisons from C++26 [PR117788]

[Jason Merrill]

On 12/4/24 12:27 PM, Marek Polacek wrote:
> On Tue, Dec 03, 2024 at 04:27:22PM -0500, Jason Merrill wrote:
>> On 12/3/24 2:46 PM, Marek Polacek wrote:
>>> On Thu, Nov 28, 2024 at 12:04:56PM -0500, Jason Merrill wrote:
>>>> On 11/27/24 9:06 PM, Marek Polacek wrote:
>>>>> Not a bugfix, but this should only affect C++26.
>>>>>
>>>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>>>
>>>>> -- >8--
>>>>> This patch implements P2865R5 by promoting the warning to error in C++26
>>>>> only.  -Wno-array-compare shouldn't disable the error, so adjust the call
>>>>> sites as well.
>>>>
>>>> I think it's fine for -Wno-array-compare to suppress the error (and
>>>> -Wno-error=array-compare to reduce it to a warning), so how about
>>>> DK_PERMERROR rather than DK_ERROR?
>>>
>>> Sounds good.
>>>> We also need SFINAE for this when !tf_warning_or_error.
>>>
>>> I've added Warray-compare-1.C, which has:
>>>
>>>     template<int I>
>>>     void f (int(*)[arr1 == arr2 ? I : I]);
>>>
>>> but when we call cp_build_binary_op from the parser, complain is
>>> tf_warning_or_error, so we warn (as does clang++).  I suspect
>>> that goes against [temp.deduct.general]/8.
>>
>> No, that's fine; in C++26 that template is IFNDR because no well-formed
>> instantiation exists, it's OK for us to give a diagnostic and then continue
>> just like in a non-template.
> 
> Ah yes.
> 
>> I'm not sure there is a SFINAE situation where this would come up, but I'd
>> still like to adjust this:
>>
>>> @@ -6125,11 +6124,10 @@ cp_build_binary_op (const op_location_t &location,
>>>    			"comparison with string literal results "
>>>    			"in unspecified behavior");
>>>    	}
>>> -      else if (warn_array_compare
>>> -	       && TREE_CODE (TREE_TYPE (orig_op0)) == ARRAY_TYPE
>>> +      else if (TREE_CODE (TREE_TYPE (orig_op0)) == ARRAY_TYPE
>>>    	       && TREE_CODE (TREE_TYPE (orig_op1)) == ARRAY_TYPE
>>>    	       && code != SPACESHIP_EXPR
>>> -	       && (complain & tf_warning))
>>> +	       && (complain & tf_warning_or_error))
>>>    	do_warn_array_compare (location, code,
>>>    			       tree_strip_any_location_wrapper (orig_op0),
>>>    			       tree_strip_any_location_wrapper (orig_op1));
>>
>> If we happen to get here when not complaining, we'll silently accept it.
>> Either we should handle that case by returning error_mark_node in C++26 and
>> above, or we should assert that it can't happen.
> 
> We actually can get there.  But returning error_mark_node in C++26
> causes problems: we hit:
> 
>          /* If we ran into a problem, make sure we complained.  */
>          gcc_assert (seen_error ());
> 
> because a permerror doesn't count as an error.  Either we'd have to go
> back to DK_ERROR, or leave the patch as-is.

Hmm, I guess cp_seen_error should also consider werrorcount.

Jason