On Fri, May 4, 2018 at 2:06 PM, Paolo Carlini <paolo.carl...@oracle.com> wrote:
> Hi,
>
>
> On 04/05/2018 19:45, Jason Merrill wrote:
>>
>> On Sun, Apr 29, 2018 at 3:23 AM, Paolo Carlini<paolo.carl...@oracle.com>
>> wrote:
>>>
>>> Hi,
>>>
>>> On 28/04/2018 18:41, Jason Merrill wrote:
>>>>
>>>> On Fri, Apr 27, 2018 at 7:26 PM, Paolo Carlini<paolo.carl...@oracle.com>
>>>> wrote:
>>>>>
>>>>> Hi again,
>>>>>
>>>>> I'm now pretty sure that we have a latent issue in ocp_convert. The bug
>>>>> fixed by Jakub shows that we used to not have issues with
>>>>> integer_zero_node.
>>>>> That's easy to explain: at the beginning of ocp_convert there is code
>>>>> which
>>>>> handles first some special / simple cases when
>>>>> same_type_ignoring_top_level_qualifiers_p is true. That code isn't of
>>>>> course
>>>>> used for integer_zero_node as source expression, which therefore is
>>>>> handled
>>>>> by:
>>>>>
>>>>> if (NULLPTR_TYPE_P (type) && e && null_ptr_cst_p (e))
>>>>> {
>>>>> if (complain & tf_warning)
>>>>> maybe_warn_zero_as_null_pointer_constant (e, loc);
>>>>> return nullptr_node;
>>>>> }
>>>>
>>>> Maybe we should move this code up, then.
>>>
>>> You are totally right. Yesterday I realized that and tested on
>>> x86_64-linux
>>> the below, both with and without Jakub's fix.
>>
>> + if (!TREE_SIDE_EFFECTS (e))
>> + return nullptr_node;
>>
>> So what happens if e has side-effects?
>
> In that case nothing should change wrt the status quo, that is, the "fast
> path" wrapping the thing in a NOP_EXPR. That only for NULLPTR_TYPE_P nodes,
> I don't think that can happen for integer_zero_nodes. I must say, if I take
> out the check there are no regressions, but using it seems consistent with
> decay_conversion, were not having the check caused a real wrong code bug.
> What do you think? Maybe an alternative would be returning immediately e
> as-is?!?
The patch is OK as is.
Jason
