[analyzer] Address Jordan's code review comments for r164716.

llvm-svn: 164788

diff --git a/clang/lib/StaticAnalyzer/Checkers/Checkers.td b/clang/lib/StaticAnalyzer/Checkers/Checkers.td
index 6e78560..f5b50fb 100644 (file)
--- a/clang/lib/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/lib/StaticAnalyzer/Checkers/Checkers.td
@@ -415,7 +415,7 @@ def ObjCDeallocChecker : Checker<"Dealloc">,
 
 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"
 

diff --git a/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
index 97b1f7c..0bf8073 100644 (file)
--- a/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
@@ -74,7 +74,7 @@ class IvarInvalidationChecker :
     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 @@ public:
   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 @@ void IvarInvalidationChecker::checkASTDecl(const ObjCMethodDecl *D,
     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 @@ void IvarInvalidationChecker::checkASTDecl(const ObjCMethodDecl *D,
     // 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 @@ void IvarInvalidationChecker::checkASTDecl(const ObjCMethodDecl *D,
       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);

diff --git a/clang/test/Analysis/objc_invalidation.m b/clang/test/Analysis/objc_invalidation.m
index 7050fdd..e0ce28b 100644 (file)
--- a/clang/test/Analysis/objc_invalidation.m
+++ b/clang/test/Analysis/objc_invalidation.m
@@ -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@-1 {{Instance variable Obj2 needs to be invalidated}}
+ // expected-warning@-2 {{Instance variable MultipleProtocols needs to be invalidated}}
+ // expected-warning@-3 {{Instance variable MultInheritance needs to be invalidated}}
 @end