Improper implicit pointer cast in AST

[Abramo Bagnara]

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?

-- 
Abramo Bagnara

BUGSENG srl - http://bugseng.com
mailto:abramo.bagnara at bugseng.com
-------------- next part --------------
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp	(revisione 217331)
+++ lib/Sema/SemaChecking.cpp	(copia locale)
@@ -5688,7 +5688,14 @@
 static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
   // The type the comparison is being performed in.
   QualType T = E->getLHS()->getType();
-  assert(S.Context.hasSameUnqualifiedType(T, E->getRHS()->getType())
+  assert((S.Context.hasSameUnqualifiedType(T, E->getRHS()->getType()) ||
+          (!S.getLangOpts().CPlusPlus &&
+           T->isPointerType() &&
+           E->getRHS()->getType()->isPointerType() &&
+           !E->getLHS()->isNullPointerConstant(S.Context,
+                                               Expr::NPC_ValueDependentIsNull) &&
+           !E->getRHS()->isNullPointerConstant(S.Context,
+                                               Expr::NPC_ValueDependentIsNull)))
          && "comparison with mismatched types");
   if (E->isValueDependent())
     return AnalyzeImpConvsInComparison(S, E);
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp	(revisione 217331)
+++ lib/Sema/SemaExpr.cpp	(copia locale)
@@ -8045,9 +8045,11 @@
       unsigned AddrSpaceR = RCanPointeeTy.getAddressSpace();
       CastKind Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion
                                                : CK_BitCast;
-      if (LHSIsNull && !RHSIsNull)
-        LHS = ImpCastExprToType(LHS.get(), RHSType, Kind);
-      else
+      if (LHSIsNull) {
+        if (!RHSIsNull)
+          LHS = ImpCastExprToType(LHS.get(), RHSType, Kind);
+      }
+      else if (RHSIsNull)
         RHS = ImpCastExprToType(RHS.get(), LHSType, Kind);
     }
     return ResultTy;