From 8c0dd36ede967f55a6be98ee15253d6bbda28f3b Mon Sep 17 00:00:00 2001 From: Anna Zaks Date: Thu, 27 Sep 2012 19:45:08 +0000 Subject: [PATCH] [analyzer] Address Jordan's code review comments for r164716. llvm-svn: 164788 --- clang/lib/StaticAnalyzer/Checkers/Checkers.td | 2 +- .../Checkers/IvarInvalidationChecker.cpp | 30 ++++++++++++---------- clang/test/Analysis/objc_invalidation.m | 15 ++++++----- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/Checkers.td b/clang/lib/StaticAnalyzer/Checkers/Checkers.td index 6e78560..f5b50fb 100644 --- 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 --- 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(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(); - if (!Ann) - return false; - if (Ann->getAnnotation() == "objc_instance_variable_invalidator") - return true; + for (specific_attr_iterator + AI = M->specific_attr_begin(), + AE = M->specific_attr_end(); 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(); if (!IvTy) continue; - const ObjCInterfaceDecl *IvInterf = IvTy->getObjectType()->getInterface(); + const ObjCInterfaceDecl *IvInterf = IvTy->getInterfaceDecl(); if (containsInvalidationMethod(IvInterf)) Ivars[cast(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(ID->getCanonicalDecl())] = true; + if (!SetterD) continue; - } // Store the mappings. PD = cast(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 --- 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 -- 2.7.4