Improper implicit pointer cast in AST

[Abramo Bagnara]

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 --------------
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp	(revisione 217331)
+++ lib/Sema/SemaChecking.cpp	(copia locale)
@@ -5688,7 +5688,8 @@
 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)
          && "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,14 @@
       unsigned AddrSpaceR = RCanPointeeTy.getAddressSpace();
       CastKind Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion
                                                : CK_BitCast;
-      if (LHSIsNull && !RHSIsNull)
+      // C99 6.5.9p5
+      if ((LHSIsNull && !RHSIsNull) ||
+          (LCanPointeeTy->isIncompleteOrObjectType() &&
+           RCanPointeeTy->isVoidType()))
         LHS = ImpCastExprToType(LHS.get(), RHSType, Kind);
-      else
+      else if ((!LHSIsNull && RHSIsNull) ||
+               (LCanPointeeTy->isVoidType() &&
+                RCanPointeeTy->isIncompleteOrObjectType()))
         RHS = ImpCastExprToType(RHS.get(), LHSType, Kind);
     }
     return ResultTy;