[cfe-commits] r164788 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp test/Analysis/objc_invalidation.m

[Anna Zaks]

Author: zaks
Date: Thu Sep 27 14:45:08 2012
New Revision: 164788

URL: http://llvm.org/viewvc/llvm-project?rev=164788&view=rev
Log:
[analyzer] Address Jordan's code review comments for r164716.

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
    cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
    cfe/trunk/test/Analysis/objc_invalidation.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td?rev=164788&r1=164787&r2=164788&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td Thu Sep 27 14:45:08 2012
@@ -415,7 +415,7 @@
 
 def IvarInvalidationChecker : Checker<"InstanceVariableInvalidation">,
   HelpText<"Check that the invalidatable instance variables are invalidated in the methods annotated with objc_instance_variable_invalidator">,
-  DescFile<"IvarInvalidationCheckerChecker.cpp">;
+  DescFile<"IvarInvalidationChecker.cpp">;
 
 } // end "alpha.osx.cocoa"
 

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp?rev=164788&r1=164787&r2=164788&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp Thu Sep 27 14:45:08 2012
@@ -74,7 +74,7 @@
     void VisitChildren(const Stmt *S) {
       for (Stmt::const_child_range I = S->children(); I; ++I)
         if (*I)
-          static_cast<MethodCrawler*>(this)->Visit(*I);
+          this->Visit(*I);
     }
   };
 
@@ -94,14 +94,17 @@
   void checkASTDecl(const ObjCMethodDecl *D, AnalysisManager& Mgr,
                     BugReporter &BR) const;
 
+  // TODO: We are currently ignoring the ivars coming from class extensions.
 };
 
 bool isInvalidationMethod(const ObjCMethodDecl *M) {
-  const AnnotateAttr *Ann = M->getAttr<AnnotateAttr>();
-  if (!Ann)
-    return false;
-  if (Ann->getAnnotation() == "objc_instance_variable_invalidator")
-    return true;
+  for (specific_attr_iterator<AnnotateAttr>
+       AI = M->specific_attr_begin<AnnotateAttr>(),
+       AE = M->specific_attr_end<AnnotateAttr>(); AI != AE; ++AI) {
+    const AnnotateAttr *Ann = *AI;
+    if (Ann->getAnnotation() == "objc_instance_variable_invalidator")
+      return true;
+  }
   return false;
 }
 
@@ -203,7 +206,7 @@
     const ObjCObjectPointerType *IvTy = IvQTy->getAs<ObjCObjectPointerType>();
     if (!IvTy)
       continue;
-    const ObjCInterfaceDecl *IvInterf = IvTy->getObjectType()->getInterface();
+    const ObjCInterfaceDecl *IvInterf = IvTy->getInterfaceDecl();
     if (containsInvalidationMethod(IvInterf))
       Ivars[cast<ObjCIvarDecl>(Iv->getCanonicalDecl())] = false;
   }
@@ -224,10 +227,8 @@
     // Find the setter.
     const ObjCMethodDecl *SetterD = PD->getSetterMethodDecl();
     // If we don't know the setter, do not track this ivar.
-    if (!SetterD) {
-      Ivars[cast<ObjCIvarDecl>(ID->getCanonicalDecl())] = true;
+    if (!SetterD)
       continue;
-    }
 
     // Store the mappings.
     PD = cast<ObjCPropertyDecl>(PD->getCanonicalDecl());
@@ -248,14 +249,15 @@
       const ObjCIvarDecl *IvarDecl = I->first;
 
       PathDiagnosticLocation IvarDecLocation =
-          PathDiagnosticLocation::createBegin(IvarDecl, BR.getSourceManager());
+          PathDiagnosticLocation::createEnd(D->getBody(), BR.getSourceManager(),
+                                            Mgr.getAnalysisDeclContext(D));
 
       SmallString<128> sbuf;
       llvm::raw_svector_ostream os(sbuf);
-      os << "Ivar needs to be invalidated in the '" <<
-            D->getSelector().getAsString()<< "' method";
+      os << "Instance variable "<< IvarDecl->getName()
+         << " needs to be invalidated";
 
-      BR.EmitBasicReport(IvarDecl,
+      BR.EmitBasicReport(D,
           "Incomplete invalidation",
           categories::CoreFoundationObjectiveC, os.str(),
           IvarDecLocation);

Modified: cfe/trunk/test/Analysis/objc_invalidation.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/objc_invalidation.m?rev=164788&r1=164787&r2=164788&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/objc_invalidation.m (original)
+++ cfe/trunk/test/Analysis/objc_invalidation.m Thu Sep 27 14:45:08 2012
@@ -42,15 +42,15 @@
 @end
 
 @interface SomeSubclassInvalidatableObject : SomeInvalidationImplementingObject {
-  SomeInvalidationImplementingObject *Obj1; // expected-warning{{Ivar needs to be invalidated}}
-  SomeInvalidationImplementingObject *Obj2; // expected-warning{{Ivar needs to be invalidated}}
+  SomeInvalidationImplementingObject *Obj1; 
+  SomeInvalidationImplementingObject *Obj2;
   SomeInvalidationImplementingObject *Obj3;
   SomeInvalidationImplementingObject *_Prop1;
   SomeInvalidationImplementingObject *_Prop4;
   SomeInvalidationImplementingObject *_propIvar;
-  Invalidation1Class *MultipleProtocols; // expected-warning{{Ivar needs to be invalidated}}
-  Invalidation2Class *MultInheritance; // expected-warning{{Ivar needs to be invalidated}}
-
+  Invalidation1Class *MultipleProtocols;
+  Invalidation2Class *MultInheritance; 
+  
   // No warnings on these.
   NSObject *NObj1;
   NSObject *NObj2;
@@ -103,5 +103,8 @@
    [self setProp3:0];
    self.Prop4 = 0;
    [super invalidate];
-}
+}// expected-warning {{Instance variable Obj1 needs to be invalidated}}
+ // expected-warning at -1 {{Instance variable Obj2 needs to be invalidated}}
+ // expected-warning at -2 {{Instance variable MultipleProtocols needs to be invalidated}}
+ // expected-warning at -3 {{Instance variable MultInheritance needs to be invalidated}}
 @end