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

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL299080: [ARC][ObjC++] Use ObjC semantic rules for 
comparisons between a pointer and (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D31177?vs=92783&id=93475#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D31177

Files:
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/SemaObjCXX/arc-ptr-comparison.mm


Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -9423,7 +9423,10 @@
 //   If both operands are pointers, [...] bring them to their composite
 //   pointer type.
 if ((int)LHSType->isPointerType() + (int)RHSType->isPointerType() >=
-(IsRelational ? 2 : 1)) {
+(IsRelational ? 2 : 1) &&
+(!LangOpts.ObjCAutoRefCount ||
+ !(LHSType->isObjCObjectPointerType() ||
+   RHSType->isObjCObjectPointerType( {
   if (convertPointersToCompositeType(*this, Loc, LHS, RHS))
 return QualType();
   else
Index: cfe/trunk/test/SemaObjCXX/arc-ptr-comparison.mm
===
--- cfe/trunk/test/SemaObjCXX/arc-ptr-comparison.mm
+++ cfe/trunk/test/SemaObjCXX/arc-ptr-comparison.mm
@@ -0,0 +1,24 @@
+// 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;
+}


Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -9423,7 +9423,10 @@
 //   If both operands are pointers, [...] bring them to their composite
 //   pointer type.
 if ((int)LHSType->isPointerType() + (int)RHSType->isPointerType() >=
-(IsRelational ? 2 : 1)) {
+(IsRelational ? 2 : 1) &&
+(!LangOpts.ObjCAutoRefCount ||
+ !(LHSType->isObjCObjectPointerType() ||
+   RHSType->isObjCObjectPointerType( {
   if (convertPointersToCompositeType(*this, Loc, LHS, RHS))
 return QualType();
   else
Index: cfe/trunk/test/SemaObjCXX/arc-ptr-comparison.mm
===
--- cfe/trunk/test/SemaObjCXX/arc-ptr-comparison.mm
+++ cfe/trunk/test/SemaObjCXX/arc-ptr-comparison.mm
@@ -0,0 +1,24 @@
+// 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;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak accepted this revision.
ahatanak added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D31177



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 92783.
arphaman marked an inline comment as done.
arphaman added a comment.

The condition in the if is now more clear.


Repository:
  rL LLVM

https://reviews.llvm.org/D31177

Files:
  lib/Sema/SemaExpr.cpp
  test/SemaObjCXX/arc-ptr-comparison.mm


Index: test/SemaObjCXX/arc-ptr-comparison.mm
===
--- /dev/null
+++ test/SemaObjCXX/arc-ptr-comparison.mm
@@ -0,0 +1,24 @@
+// 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;
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -9424,7 +9424,10 @@
 //   If both operands are pointers, [...] bring them to their composite
 //   pointer type.
 if ((int)LHSType->isPointerType() + (int)RHSType->isPointerType() >=
-(IsRelational ? 2 : 1)) {
+(IsRelational ? 2 : 1) &&
+(!LangOpts.ObjCAutoRefCount ||
+ !(LHSType->isObjCObjectPointerType() ||
+   RHSType->isObjCObjectPointerType( {
   if (convertPointersToCompositeType(*this, Loc, LHS, RHS))
 return QualType();
   else


Index: test/SemaObjCXX/arc-ptr-comparison.mm
===
--- /dev/null
+++ test/SemaObjCXX/arc-ptr-comparison.mm
@@ -0,0 +1,24 @@
+// 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;
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -9424,7 +9424,10 @@
 //   If both operands are pointers, [...] bring them to their composite
 //   pointer type.
 if ((int)LHSType->isPointerType() + (int)RHSType->isPointerType() >=
-(IsRelational ? 2 : 1)) {
+(IsRelational ? 2 : 1) &&
+(!LangOpts.ObjCAutoRefCount ||
+ !(LHSType->isObjCObjectPointerType() ||
+   RHSType->isObjCObjectPointerType( {
   if (convertPointersToCompositeType(*this, Loc, LHS, RHS))
 return QualType();
   else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:9431
+ ? 2
+ : 1)) {
   if (convertPointersToCompositeType(*this, Loc, LHS, RHS))

ahatanak wrote:
> 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()))
> ```
Yes, that's correct. I think a more clear condition would be better here, I 
agree.


Repository:
  rL LLVM

https://reviews.llvm.org/D31177



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:9431
+ ? 2
+ : 1)) {
   if (convertPointersToCompositeType(*this, Loc, LHS, RHS))

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()))
```


Repository:
  rL LLVM

https://reviews.llvm.org/D31177



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.

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.


Repository:
  rL LLVM

https://reviews.llvm.org/D31177

Files:
  lib/Sema/SemaExpr.cpp
  test/SemaObjCXX/arc-ptr-comparison.mm


Index: test/SemaObjCXX/arc-ptr-comparison.mm
===
--- /dev/null
+++ test/SemaObjCXX/arc-ptr-comparison.mm
@@ -0,0 +1,24 @@
+// 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;
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -9424,7 +9424,11 @@
 //   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


Index: test/SemaObjCXX/arc-ptr-comparison.mm
===
--- /dev/null
+++ test/SemaObjCXX/arc-ptr-comparison.mm
@@ -0,0 +1,24 @@
+// 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;
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -9424,7 +9424,11 @@
 //   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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits