[ARC][ObjC++] Use ObjC semantic rules for comparisons between a pointer and objective-c object pointer

[arphaman]

Summary

When ARC is enabled in Objective-C++, comparisons between a pointer and Objective-C object pointer typically result in errors like this: invalid operands to a binary expression. This error message can be quite confusing as it doesn't provide a solution to the problem, unlike the non-C++ diagnostic: implicit conversion of Objective-C pointer type 'id' to C pointer type 'void *' requires a bridged cast (it also provides fix-its). This patch forces comparisons between pointers and objective-c object pointers in ARC to use the Objective-C semantic rules to ensure that a better diagnostic is reported.

[ahatanak]

lib/Sema/SemaExpr.cpp

9431

It wasn't clear to me why the code has to be added to the right hand side of the >= operator.

Is it equivalent to adding the following code, which I think is easier to understand? I'm assuming you are trying to avoid executing the statement when ObjCAutoRefCount is true and either LHSType or RHSType is an objc pointer type.

&& (!LangOpts.ObjCAutoRefCount || (!LHSType->isObjCObjectPointerType() && !RHSType->isObjCObjectPointerType()))

[arphaman]

lib/Sema/SemaExpr.cpp

9431

Yes, that's correct. I think a more clear condition would be better here, I agree.

[arphaman]

The condition in the if is now more clear.

[ahatanak]

Thanks, LGTM.

lib/Sema/SemaExpr.cpp

if ((LHSType->isIntegerType() && !LHSIsNull) ||
 
     // C++ [expr.eq]p2:
     //   If at least one operand is a pointer [...] bring them to their
     //   composite pointer type.
     // C++ [expr.rel]p2:
     //   If both operands are pointers, [...] bring them to their composite
     //   pointer type.
     if ((int)LHSType->isPointerType() + (int)RHSType->isPointerType() >=
-        (IsRelational ? 2 : 1)) {
+        (IsRelational || (LangOpts.ObjCAutoRefCount &&
+                          (LHSType->isObjCObjectPointerType() ||
+                           RHSType->isObjCObjectPointerType()))
+             ? 2
+             : 1)) {
       if (convertPointersToCompositeType(*this, Loc, LHS, RHS))
         return QualType();
       else
         return ResultTy;
     }
   } else if (LHSType->isPointerType() &&
              RHSType->isPointerType()) { // C99 6.5.8p2
     // All of the following pointer-related warnings are GCC extensions, except

test/SemaObjCXX/arc-ptr-comparison.mm

+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin11 -fsyntax-only -fobjc-arc -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin11 -fsyntax-only -verify -DNOARC %s
+#ifdef NOARC
+// expected-no-diagnostics
+#endif
+
+int testObjCComparisonRules(void *v, id x, id y) {
+  return v == x;
+#ifndef NOARC
+// expected-error@-2 {{implicit conversion of Objective-C pointer type 'id' to C pointer type 'void *' requires a bridged cast}}
+// expected-note@-3 {{use __bridge to convert directly (no change in ownership)}}
+// expected-note@-4 {{use __bridge_retained to make an ARC object available as a +1 'void *'}}
+#endif
+  return v >= x;
+#ifndef NOARC
+// expected-error@-2 {{implicit conversion of Objective-C pointer type 'id' to C pointer type 'void *' requires a bridged cast}}
+// expected-note@-3 {{use __bridge to convert directly (no change in ownership)}}
+// expected-note@-4 {{use __bridge_retained to make an ARC object available as a +1 'void *'}}
+#endif
+  return v == (id)(void *)0; // OK
+  return v == nullptr; // OK
+  return v == (void *)0;
+  return x == y;
+}