thanks for the review. i've addressed all the points, except the one about
changing createBegin() to createEnd() because actually the dealloc checker uses
createBegin() too so the change wouldn't make it similar...
Hi jordan_rose,
http://llvm-reviews.chandlerc.com/D78
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D78?vs=199&id=213#toc
Files:
test/Analysis/viewcontroller.m
lib/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp
lib/StaticAnalyzer/Checkers/CMakeLists.txt
Index: test/Analysis/viewcontroller.m
===================================================================
--- test/Analysis/viewcontroller.m
+++ test/Analysis/viewcontroller.m
@@ -0,0 +1,135 @@
+// RUN: %clang_cc1 -fblocks -analyze -analyzer-checker=alpha.osx.cocoa.MissingSuperCall -verify -Wno-objc-root-class %s
+
+@protocol NSObject
+- (id)retain;
+- (oneway void)release;
+@end
+@interface NSObject <NSObject> {}
+- (id)init;
++ (id)alloc;
+@end
+
+typedef char BOOL;
+typedef double NSTimeInterval;
+typedef enum UIViewAnimationOptions {
+ UIViewAnimationOptionLayoutSubviews = 1 << 0
+} UIViewAnimationOptions;
+
+@interface UIViewController : NSObject {}
+- (void)addChildViewController:(UIViewController *)childController;
+- (void)viewDidAppear:(BOOL)animated;
+- (void)viewDidDisappear:(BOOL)animated;
+- (void)viewDidUnload;
+- (void)viewDidLoad;
+- (void)viewWillUnload;
+- (void)viewWillAppear:(BOOL)animated;
+- (void)viewWillDisappear:(BOOL)animated;
+- (void)didReceiveMemoryWarning;
+- (void)removeFromParentViewController;
+- (void)transitionFromViewController:(UIViewController *)fromViewController
+ toViewController:(UIViewController *)toViewController
+ duration:(NSTimeInterval)duration options:(UIViewAnimationOptions)options
+ animations:(void (^)(void))animations
+ completion:(void (^)(BOOL finished))completion;
+@end
+
+// Do not warn if UIViewController isn't our superclass
+@interface TestA
+@end
+@implementation TestA
+
+- (void)addChildViewController:(UIViewController *)childController {}
+- (void)viewDidAppear:(BOOL)animated {}
+- (void)viewDidDisappear:(BOOL)animated {}
+- (void)viewDidUnload {}
+- (void)viewDidLoad {}
+- (void)viewWillUnload {}
+- (void)viewWillAppear:(BOOL)animated {}
+- (void)viewWillDisappear:(BOOL)animated {}
+- (void)didReceiveMemoryWarning {}
+- (void)removeFromParentViewController {}
+
+@end
+
+// Warn if UIViewController is our superclass and we do not call super
+@interface TestB : UIViewController {}
+@end
+@implementation TestB
+
+- (void)addChildViewController:(UIViewController *)childController { // expected-warning {{The 'addChildViewController:' instance method in UIViewController subclass 'TestB' is missing a [super addChildViewController:] call}}
+ int addChildViewController = 5;
+ for (int i = 0; i < addChildViewController; i++)
+ [self viewDidAppear:i];
+}
+- (void)viewDidAppear:(BOOL)animated {} // expected-warning {{The 'viewDidAppear:' instance method in UIViewController subclass 'TestB' is missing a [super viewDidAppear:] call}}
+- (void)viewDidDisappear:(BOOL)animated {} // expected-warning {{The 'viewDidDisappear:' instance method in UIViewController subclass 'TestB' is missing a [super viewDidDisappear:] call}}
+- (void)viewDidUnload {} // expected-warning {{The 'viewDidUnload' instance method in UIViewController subclass 'TestB' is missing a [super viewDidUnload] call}}
+- (void)viewDidLoad {} // expected-warning {{The 'viewDidLoad' instance method in UIViewController subclass 'TestB' is missing a [super viewDidLoad] call}}
+- (void)viewWillUnload {} // expected-warning {{The 'viewWillUnload' instance method in UIViewController subclass 'TestB' is missing a [super viewWillUnload] call}}
+- (void)viewWillAppear:(BOOL)animated {} // expected-warning {{The 'viewWillAppear:' instance method in UIViewController subclass 'TestB' is missing a [super viewWillAppear:] call}}
+- (void)viewWillDisappear:(BOOL)animated {} // expected-warning {{The 'viewWillDisappear:' instance method in UIViewController subclass 'TestB' is missing a [super viewWillDisappear:] call}}
+- (void)didReceiveMemoryWarning {} // expected-warning {{The 'didReceiveMemoryWarning' instance method in UIViewController subclass 'TestB' is missing a [super didReceiveMemoryWarning] call}}
+- (void)removeFromParentViewController {} // expected-warning {{The 'removeFromParentViewController' instance method in UIViewController subclass 'TestB' is missing a [super removeFromParentViewController] call}}
+
+// Do not warn for methods were it shouldn't
+- (void)shouldAutorotate {};
+@end
+
+// Do not warn if UIViewController is our superclass but we did call super
+@interface TestC : UIViewController {}
+@end
+@implementation TestC
+
+- (BOOL)methodReturningStuff {
+ return 1;
+}
+
+- (void)methodDoingStuff {
+ [super removeFromParentViewController];
+}
+
+- (void)addChildViewController:(UIViewController *)childController {
+ [super addChildViewController:childController];
+}
+
+- (void)viewDidAppear:(BOOL)animated {
+ [super viewDidAppear:animated];
+}
+
+- (void)viewDidDisappear:(BOOL)animated {
+ [super viewDidDisappear:animated];
+}
+
+- (void)viewDidUnload {
+ [super viewDidUnload];
+}
+
+- (void)viewDidLoad {
+ [super viewDidLoad];
+}
+
+- (void)viewWillUnload {
+ [super viewWillUnload];
+}
+
+- (void)viewWillAppear:(BOOL)animated {
+ int i = 0; // Also don't start warning just because we do additional stuff
+ i++;
+ [self viewDidDisappear:i];
+ [super viewWillAppear:animated];
+}
+
+- (void)viewWillDisappear:(BOOL)animated {
+ [super viewWillDisappear:[self methodReturningStuff]];
+}
+
+- (void)didReceiveMemoryWarning {
+ [super didReceiveMemoryWarning];
+}
+
+// We expect a warning here because at the moment the super-call can't be
+// done from another method.
+- (void)removeFromParentViewController { // expected-warning {{The 'removeFromParentViewController' instance method in UIViewController subclass 'TestC' is missing a [super removeFromParentViewController] call}}
+ [self methodDoingStuff];
+}
+@end
Index: lib/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- lib/StaticAnalyzer/Checkers/Checkers.td
+++ lib/StaticAnalyzer/Checkers/Checkers.td
@@ -416,6 +416,10 @@
HelpText<"Check that the invalidatable instance variables are invalidated in the methods annotated with objc_instance_variable_invalidator">,
DescFile<"DirectIvarAssignment.cpp">;
+def ObjCSuperCallChecker : Checker<"MissingSuperCall">,
+ HelpText<"Warn about Objective-C methods that lack a necessary call to super">,
+ DescFile<"ObjCMissingSuperCallChecker.cpp">;
+
} // end "alpha.osx.cocoa"
let ParentPackage = CoreFoundation in {
Index: lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp
@@ -0,0 +1,204 @@
+//==- ObjCMissingSuperCallChecker.cpp - Check missing super-calls in ObjC -==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines a ObjCMissingSuperCallChecker, a checker that
+// analyzes a UIViewController implementation to determine if it
+// correctly calls super in the methods where this is mandatory.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/AST/ExprObjC.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/DeclObjC.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/LangOptions.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallSet.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace clang;
+using namespace ento;
+
+//===----------------------------------------------------------------------===//
+// FindSupercallVisitor - Identify specific calls to the superclass.
+//===----------------------------------------------------------------------===//
+
+class FindSupercallVisitor : public RecursiveASTVisitor<FindSupercallVisitor> {
+public:
+ explicit FindSupercallVisitor(ASTContext *Context, Selector Sel)
+ : DoesCallSuper(0), Sel(Sel) {}
+
+ bool VisitObjCMessageExpr(ObjCMessageExpr *E) {
+ if (E->getSelector() == Sel) {
+ if (E->getReceiverKind() == ObjCMessageExpr::SuperInstance) {
+ DoesCallSuper = true;
+ }
+ }
+ // Recurse if we didn't find the super call yet.
+ return !DoesCallSuper;
+ }
+
+ bool DoesCallSuper;
+
+private:
+ Selector Sel;
+};
+
+//===----------------------------------------------------------------------===//
+// ObjCSuperCallChecker
+//===----------------------------------------------------------------------===//
+
+namespace {
+class ObjCSuperCallChecker : public Checker<
+ check::ASTDecl<ObjCImplementationDecl> > {
+public:
+ void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& mgr,
+ BugReporter &BR) const;
+};
+}
+
+void ObjCSuperCallChecker::checkASTDecl(const ObjCImplementationDecl *D,
+ AnalysisManager& mgr,
+ BugReporter &BR) const {
+ ASTContext &Ctx = BR.getContext();
+ const ObjCInterfaceDecl *ID = D->getClassInterface();
+ IdentifierInfo *ViewControllerII = &Ctx.Idents.get("UIViewController");
+ bool IsViewController = false;
+
+ // Determine if the class subclasses UIViewController.
+ for ( ; ID ; ID = ID->getSuperClass()) {
+ IdentifierInfo *II = ID->getIdentifier();
+
+ if (II == ViewControllerII)
+ IsViewController = true;
+ }
+
+ if (!IsViewController)
+ return;
+
+ const char *SelectorNames[] =
+ {"addChildViewController", "viewDidAppear", "viewDidDisappear",
+ "viewWillAppear", "viewWillDisappear", "removeFromParentViewController",
+ "didReceiveMemoryWarning", "viewDidUnload", "viewWillUnload", "viewDidLoad"};
+ const unsigned SelectorArgumentCounts[] =
+ {1, 1, 1, 1, 1, 0, 0, 0, 0, 0};
+ const size_t SelectorCount = llvm::array_lengthof(SelectorNames);
+ assert(llvm::array_lengthof(SelectorArgumentCounts) == SelectorCount);
+ llvm::SmallSet<Selector, 16> Selectors;
+
+
+ // Fill the Selectors SmallSet with all selectors we want to check.
+ for (size_t i = 0; i < SelectorCount; i++) {
+ unsigned ArgumentCount = SelectorArgumentCounts[i];
+ const char *SelectorCString = SelectorNames[i];
+
+ // Get the selector.
+ IdentifierInfo *II = &Ctx.Idents.get(SelectorCString);
+ Selectors.insert(Ctx.Selectors.getSelector(ArgumentCount, &II));
+ }
+
+ // Iterate over all instance methods.
+ for (ObjCImplementationDecl::instmeth_iterator I = D->instmeth_begin(),
+ E = D->instmeth_end(); I!=E; ++I) {
+
+ Selector S = (*I)->getSelector();
+ // Find out whether this is a selector that we want to check.
+ if (Selectors.count(S)) {
+ ObjCMethodDecl *MD = *I;
+
+ // Check if the method calls its superclass implementation.
+ if (MD->getBody())
+ {
+ FindSupercallVisitor Visitor = FindSupercallVisitor(&Ctx, S);
+ Visitor.TraverseDecl(MD);
+
+ // It doesn't call super, emit a diagnostic.
+ if (!Visitor.DoesCallSuper) {
+ PathDiagnosticLocation DLoc =
+ PathDiagnosticLocation::createBegin(MD, BR.getSourceManager());
+
+ const char *Name = "Missing call to superclass";
+ SmallString<320> Buf;
+ llvm::raw_svector_ostream os(Buf);
+
+ os << "The '" << S.getAsString()
+ << "' instance method in UIViewController subclass '" << *D
+ << "' is missing a [super " << S.getAsString() << "] call";
+
+ BR.EmitBasicReport(MD, Name, categories::CoreFoundationObjectiveC,
+ os.str(), DLoc);
+ }
+ }
+ }
+ }
+}
+
+
+//===----------------------------------------------------------------------===//
+// Check registration.
+//===----------------------------------------------------------------------===//
+
+void ento::registerObjCSuperCallChecker(CheckerManager &mgr) {
+ mgr.registerChecker<ObjCSuperCallChecker>();
+}
+
+
+/*
+ ToDo list for expanding this check in the future, the list is not exhaustive.
+ There are also cases where calling super is suggested but not "mandatory".
+ In addition to be able to check the classes and methods below, architectural
+ improvements like being able to allow for the super-call to be done in a called
+ method would be good too.
+
+*** trivial cases:
+UIResponder subclasses
+- resignFirstResponder
+
+NSResponder subclasses
+- cursorUpdate
+
+*** more difficult cases:
+
+UIDocument subclasses
+- finishedHandlingError:recovered: (is multi-arg)
+- finishedHandlingError:recovered: (is multi-arg)
+
+UIViewController subclasses
+- loadView (should *never* call super)
+- transitionFromViewController:toViewController:
+ duration:options:animations:completion: (is multi-arg)
+
+UICollectionViewController subclasses
+- loadView (take care because UIViewController subclasses should NOT call super
+ in loadView, but UICollectionViewController subclasses should)
+
+NSObject subclasses
+- doesNotRecognizeSelector (it only has to call super if it doesn't throw)
+
+UIPopoverBackgroundView subclasses (some of those are class methods)
+- arrowDirection (should *never* call super)
+- arrowOffset (should *never* call super)
+- arrowBase (should *never* call super)
+- arrowHeight (should *never* call super)
+- contentViewInsets (should *never* call super)
+
+UITextSelectionRect subclasses (some of those are properties)
+- rect (should *never* call super)
+- range (should *never* call super)
+- writingDirection (should *never* call super)
+- isVertical (should *never* call super)
+- containsStart (should *never* call super)
+- containsEnd (should *never* call super)
+*/
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -16,6 +16,7 @@
CallAndMessageChecker.cpp
CastSizeChecker.cpp
CastToStructChecker.cpp
+ ObjCMissingSuperCallChecker.cpp
CheckObjCDealloc.cpp
CheckObjCInstMethSignature.cpp
CheckSecuritySyntaxOnly.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
