Improper implicit pointer cast in AST

[Richard Smith]

+      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 types,
which we support here as an extension. Also, you should handle 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*.

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).

Please also provide some CodeGen tests showing that we do the right thing
here, in particular if the address spaces differ.

On Mon, Sep 15, 2014 at 7:24 AM, Abramo Bagnara <abramo.bagnara at bugseng.com>
wrote:

> Ping.
>
> Il 07/09/2014 14:44, Abramo Bagnara ha scritto:
> > Il 07/09/2014 13:05, Abramo Bagnara ha scritto:
> >> Il 07/09/2014 05:25, Richard Smith ha scritto:
> >>> On Sat, Sep 6, 2014 at 3:27 AM, Abramo Bagnara
> >>> <abramo.bagnara at bugseng.com <mailto:abramo.bagnara at bugseng.com>>
> wrote:
> >>>
> >>>     Ping
> >>>
> >>>     Il 04/09/2014 15:13, Abramo Bagnara ha scritto:
> >>>     >
> >>>     > For the following source
> >>>     >
> >>>     > void f() {
> >>>     >   char *p;
> >>>     >   const char *cp;
> >>>     >   p == cp;
> >>>     >   p != cp;
> >>>     >   p < cp;
> >>>     >   p <= cp;
> >>>     >   p > cp;
> >>>     >   p >= cp;
> >>>     >   p - cp;
> >>>     > }
> >>>     >
> >>>     > clang (unexpectedly for me) emits an implicit cast from const
> char
> >>>     * to
> >>>     > char * (i.e. it remove a qualifier) for all the relational and
> >>>     equality
> >>>     > operator, but it (expectedly) does not emit one for the
> subtraction.
> >>>     >
> >>>     > AFAIK the C standard does not require any conversion for
> >>>     relational and
> >>>     > equality operator (just like for subtraction).
> >>>     >
> >>>     > Do we have a reason to add the implicit casts or it is a bug?
> >>>
> >>>
> >>> It seems like a minor convenience to have an AST invariant that both
> >>> operands are of the same type here, but I expect we could remove these
> >>> implicit casts without any significant repercussions; I'd be mildly in
> >>> favor of doing that since it improves AST fidelity.
> >>
> >> I've attached a patch doing that.
> >>
> >> Ok to commit?
> >
> > I've attached a patch that follows better what standard requires and
> > that previously was not done. I've also weakened an assertion to avoid
> > to have an exceedingly complex condition.
> >
> >
>
>
> --
> Abramo Bagnara
>
> BUGSENG srl - http://bugseng.com
> mailto:abramo.bagnara at bugseng.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140925/fbe8527d/attachment.html>