r213423 - Objective-C. Patch to warn if the result of calling a property getter

[Fariborz Jahanian]

Author: fjahanian
Date: Fri Jul 18 17:59:10 2014
New Revision: 213423

URL: http://llvm.org/viewvc/llvm-project?rev=213423&view=rev
Log:
Objective-C. Patch to warn if the result of calling a property getter 
is unused (this is match behavior when property-dot syntax is used to
use same getter). rdar://17514245
Patch by Anders Carlsson with minor refactoring by me.

Modified:
    cfe/trunk/lib/AST/Expr.cpp
    cfe/trunk/lib/Sema/SemaStmt.cpp
    cfe/trunk/test/SemaObjC/class-property-access.m
    cfe/trunk/test/SemaObjC/conditional-expr.m
    cfe/trunk/test/SemaObjC/property-noninherited-availability-attr.m
    cfe/trunk/test/SemaObjC/unused.m

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=213423&r1=213422&r2=213423&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Fri Jul 18 17:59:10 2014
@@ -2161,12 +2161,15 @@ bool Expr::isUnusedResultAWarning(const
       return true;
     }
 
-    const ObjCMethodDecl *MD = ME->getMethodDecl();
-    if (MD && MD->hasAttr<WarnUnusedResultAttr>()) {
-      WarnE = this;
-      Loc = getExprLoc();
-      return true;
-    }
+    if (const ObjCMethodDecl *MD = ME->getMethodDecl())
+      if (MD->hasAttr<WarnUnusedResultAttr>() ||
+          (MD->isPropertyAccessor() && !MD->getReturnType()->isVoidType() &&
+           !ME->getReceiverType()->isObjCIdType())) {
+        WarnE = this;
+        Loc = getExprLoc();
+        return true;
+      }
+
     return false;
   }
 

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=213423&r1=213422&r2=213423&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Fri Jul 18 17:59:10 2014
@@ -253,9 +253,15 @@ void Sema::DiagnoseUnusedExprResult(cons
       return;
     }
     const ObjCMethodDecl *MD = ME->getMethodDecl();
-    if (MD && MD->hasAttr<WarnUnusedResultAttr>()) {
-      Diag(Loc, diag::warn_unused_result) << R1 << R2;
-      return;
+    if (MD) {
+      if (MD->hasAttr<WarnUnusedResultAttr>()) {
+        Diag(Loc, diag::warn_unused_result) << R1 << R2;
+        return;
+      }
+      if (MD->isPropertyAccessor()) {
+        Diag(Loc, diag::warn_unused_property_expr);
+        return;
+      }
     }
   } else if (const PseudoObjectExpr *POE = dyn_cast<PseudoObjectExpr>(E)) {
     const Expr *Source = POE->getSyntacticForm();

Modified: cfe/trunk/test/SemaObjC/class-property-access.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/class-property-access.m?rev=213423&r1=213422&r2=213423&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/class-property-access.m (original)
+++ cfe/trunk/test/SemaObjC/class-property-access.m Fri Jul 18 17:59:10 2014
@@ -41,8 +41,8 @@ void Test1() {
     (void)Subclass.classMethod;
 
     // also okay
-    [RootClass property];
-    [Subclass property];
+    (void)[RootClass property];
+    (void)[Subclass property];
     [RootClass method];
     [Subclass method];
     [RootClass classMethod];

Modified: cfe/trunk/test/SemaObjC/conditional-expr.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/conditional-expr.m?rev=213423&r1=213422&r2=213423&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/conditional-expr.m (original)
+++ cfe/trunk/test/SemaObjC/conditional-expr.m Fri Jul 18 17:59:10 2014
@@ -96,8 +96,8 @@ id f7(int a, id<P0> x, A* p) {
   return a ? x : p;
 }
 
-void f8(int a, A<P0> *x, A *y) {
-  [ (a ? x : y ) intProp ];
+int f8(int a, A<P0> *x, A *y) {
+  return [ (a ? x : y ) intProp ];
 }
 
 void f9(int a, A<P0> *x, A<P1> *y) {

Modified: cfe/trunk/test/SemaObjC/property-noninherited-availability-attr.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/property-noninherited-availability-attr.m?rev=213423&r1=213422&r2=213423&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/property-noninherited-availability-attr.m (original)
+++ cfe/trunk/test/SemaObjC/property-noninherited-availability-attr.m Fri Jul 18 17:59:10 2014
@@ -21,13 +21,13 @@
 
 void test(Foo *y, Bar *x, id<myProtocol> z) {
   y.myProperty = 0; // expected-warning {{'myProperty' is deprecated: first deprecated in OS X 10.8}}
-  [y myProperty];   // expected-warning {{'myProperty' is deprecated: first deprecated in OS X 10.8}} 
+  (void)[y myProperty];   // expected-warning {{'myProperty' is deprecated: first deprecated in OS X 10.8}} 
 
   x.myProperty = 1; // no-warning
-  [x myProperty]; // no-warning
+  (void)[x myProperty]; // no-warning
 
   x.myProtocolProperty = 0; // no-warning
 
-  [x myProtocolProperty]; // no-warning
-  [z myProtocolProperty]; // expected-warning {{'myProtocolProperty' is deprecated: first deprecated in OS X 10.8}}
+  (void)[x myProtocolProperty]; // no-warning
+  (void)[z myProtocolProperty]; // expected-warning {{'myProtocolProperty' is deprecated: first deprecated in OS X 10.8}}
 }

Modified: cfe/trunk/test/SemaObjC/unused.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/unused.m?rev=213423&r1=213422&r2=213423&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/unused.m (original)
+++ cfe/trunk/test/SemaObjC/unused.m Fri Jul 18 17:59:10 2014
@@ -81,3 +81,26 @@ void rdar15596883(id x) {
   rdar15596883_foo();
 }
 
+ at interface PropertyObject : NSObject 
+ at property int length;
+ at end
+
+ at protocol P
+ at property int property;
+ at end
+
+void test3(PropertyObject *o)
+{
+  [o length]; // expected-warning {{property access result unused - getters should not be used for side effects}}
+  (void)[o length];
+}
+
+void test4(id o)
+{
+  [o length]; // No warning.
+}
+
+void test5(id <P> p)
+{
+    [p property]; // expected-warning {{property access result unused - getters should not be used for side effects}}
+}