On Fri, Sep 26, 2014 at 1:12 AM, Abramo Bagnara <[email protected]> wrote:
> Il 26/09/2014 00:28, Richard Smith ha scritto: > > + if ((LHSIsNull && !RHSIsNull) || > > + (LCanPointeeTy->isIncompleteOrObjectType() && > > + RCanPointeeTy->isVoidType())) > > > > I don't think this is right: isIncompleteOrObjectType returns true for > > 'void', but the C standard says we do no conversions if both types are > > some flavour of void*. It also returns false for pointer-to-function > > The condition in patch mimics exactly what I read in C99. I've just > noted that in C11, incomplete types have been excluded. We should have > different behavior in the two standards? > Oh wow, you can't leave those C folks alone for a minute. Look at http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1439.pdf They made 'void' an object type. *sigh* As C99 leads to unspecified cast for cases like (void*) ptr1 == (const > void*) ptr2, I'd prefer to use the C11 superior specification. 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). > types, which we support here as an extension. Also, you should handle > > Do you mean that we want to interpret the condition "is a pointer to > object type" as "is not a pointer to void", right? Yes, that sounds good to me. > the case where one operand is (void*)0 and the other operand is T* and > > non-null. Per the C standard, first we convert the null pointer to the > > other type, and *then* we convert towards the void* type (if it's still > > present), so in this case we convert to T* not to void*. > > If I've understood what you mean you're saying that with current condition: > > (T*) ptr == (void*)0 is modified to (void*) (T*) ptr == (void*) 0 > > while > > (void*) 0 == (T*) ptr is modified to (void*) 0 == (void*) (T*) ptr > > So, to gain some simmetry, we should interpret the two conditions in > standard in a short circuit way (i.e. if X then do A else if Y then do B). > > Right? Yes. > Also, I think you still need an implicit conversion here if the address > > space is different, even if nothing else is (but I don't know which > > direction we should convert in that case). > > It does not seems to me a good idea: it seems to me rather insane to > accept comparison of pointers with different address spaces... 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...) > Please also provide some CodeGen tests showing that we do the right > > thing here, in particular if the address spaces differ. > > The changes should never change generated code, I'm missing something? > See above. I've attached a revised patch to explain how I've interpreted the issues > you've noted. Looks good apart from the address space cast issue.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
