<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Sep 26, 2014 at 1:12 AM, Abramo Bagnara <span dir="ltr"><<a href="mailto:abramo.bagnara@bugseng.com" target="_blank">abramo.bagnara@bugseng.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Il 26/09/2014 00:28, Richard Smith ha scritto:<br>
<span class="">> +      if ((LHSIsNull && !RHSIsNull) ||<br>
> +          (LCanPointeeTy->isIncompleteOrObjectType() &&<br>
> +           RCanPointeeTy->isVoidType()))<br>
><br>
> I don't think this is right: isIncompleteOrObjectType returns true for<br>
> 'void', but the C standard says we do no conversions if both types are<br>
> some flavour of void*. It also returns false for pointer-to-function<br>
<br>
</span>The condition in patch mimics exactly what I read in C99. I've just<br>
noted that in C11, incomplete types have been excluded. We should have<br>
different behavior in the two standards?<br></blockquote><div><br></div><div>Oh wow, you can't leave those C folks alone for a minute. Look at</div><div><br></div><div>  <a href="http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1439.pdf">http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1439.pdf</a><br></div><div><br></div><div>They made 'void' an object type. *sigh*</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
As C99 leads to unspecified cast for cases like (void*) ptr1 == (const<br>
void*) ptr2, I'd prefer to use the C11 superior specification.</blockquote><div><br></div><div>Me too, if only the C11 spec said what we thought it did. I think we should carry on as if the C spec said the sane thing (no conversion if both operands are of void pointer types).</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">
> types, which we support here as an extension. Also, you should handle<br>
<br>
</span>Do you mean that we want to interpret the condition "is a pointer to<br>
object type" as "is not a pointer to void", right?</blockquote><div><br></div><div>Yes, that sounds good to me.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">
> the case where one operand is (void*)0 and the other operand is T* and<br>
> non-null. Per the C standard, first we convert the null pointer to the<br>
> other type, and *then* we convert towards the void* type (if it's still<br>
> present), so in this case we convert to T* not to void*.<br>
<br>
</span>If I've understood what you mean you're saying that with current condition:<br>
<br>
(T*) ptr == (void*)0 is modified to (void*) (T*) ptr == (void*) 0<br>
<br>
while<br>
<br>
(void*) 0 == (T*) ptr is modified to (void*) 0 == (void*) (T*) ptr<br>
<br>
So, to gain some simmetry, we should interpret the two conditions in<br>
standard in a short circuit way (i.e. if X then do A else if Y then do B).<br>
<br>
Right?</blockquote><div><br></div><div>Yes.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">
> Also, I think you still need an implicit conversion here if the address<br>
> space is different, even if nothing else is (but I don't know which<br>
> direction we should convert in that case).<br>
<br>
</span>It does not seems to me a good idea: it seems to me rather insane to<br>
accept comparison of pointers with different address spaces...</blockquote><div><br></div><div>I think you are introducing a behavioral change here: we used to always cast the two pointers to be pointers into the same address space. We no longer do so. This needs test coverage, and you may need to add a diagnostic for the address-space-mismatch case. (I'm also not sure this change is correct, but equally I'm not sure it's wrong...)</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">
> Please also provide some CodeGen tests showing that we do the right<br>
> thing here, in particular if the address spaces differ.<br>
<br>
</span>The changes should never change generated code, I'm missing something?<br></blockquote><div><br></div><div>See above.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
I've attached a revised patch to explain how I've interpreted the issues<br>
you've noted.</blockquote><div><br></div><div>Looks good apart from the address space cast issue. </div></div></div></div>