Change implementation of NSError** coding-style check to be invoked at the end of...

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@56312 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/Driver/Analyses.def b/Driver/Analyses.def
index 691e4eee32e0a504f6f5a6978c73095ffc7b9a1c..841f0c1da1ec0c79fc71f02a66a78d0453ab4dd7 100644 (file)
--- a/Driver/Analyses.def
+++ b/Driver/Analyses.def
@@ -34,10 +34,6 @@ ANALYSIS(WarnObjCDealloc, "warn-objc-missing-dealloc",
  "Warn about Objective-C classes that lack a correct implementation of -dealloc",
  ObjCImplementation)
  
-ANALYSIS(WarnObjCNSError, "warn-objc-nserror-methods",
-  "Check coding rules for 'Creating and Returning NSError Objects'",
-  ObjCImplementation)
- 
 ANALYSIS(WarnObjCUnusedIvars, "warn-objc-unused-ivars",
   "Warn about private ivars that are never used", ObjCImplementation)
 

diff --git a/Driver/AnalysisConsumer.cpp b/Driver/AnalysisConsumer.cpp
index 9e321b6fd1dfd5f18e13aa025978ec413276320e..e22178b73eb5f5e4f0df3a7d1a909113e1c4127b 100644 (file)
--- a/Driver/AnalysisConsumer.cpp
+++ b/Driver/AnalysisConsumer.cpp
@@ -431,11 +431,6 @@ static void ActionWarnObjCMethSigs(AnalysisManager& mgr) {
                              BR);
 }
 
-static void ActionWarnObjCNSError(AnalysisManager& mgr) {
-  BugReporter BR(mgr);
-  CheckNSError(cast<ObjCImplementationDecl>(mgr.getCodeDecl()), BR);
-}
-
 //===----------------------------------------------------------------------===//
 // AnalysisConsumer creation.
 //===----------------------------------------------------------------------===//

diff --git a/include/clang/Analysis/LocalCheckers.h b/include/clang/Analysis/LocalCheckers.h
index a27f27b567007c3034146e125beb27cf0efee1bd..23610f9a2d97fc0829c4205f273a801b358e5f54 100644 (file)
--- a/include/clang/Analysis/LocalCheckers.h
+++ b/include/clang/Analysis/LocalCheckers.h
@@ -46,7 +46,6 @@ void CheckObjCDealloc(ObjCImplementationDecl* D, const LangOptions& L,
   
 void CheckObjCInstMethSignature(ObjCImplementationDecl* ID, BugReporter& BR);
 void CheckObjCUnusedIvar(ObjCImplementationDecl* D, BugReporter& BR);
-void CheckNSError(ObjCImplementationDecl* D, BugReporter& BR);
   
 void RegisterAppleChecks(GRExprEngine& Eng);
   

diff --git a/include/clang/Analysis/PathSensitive/BugReporter.h b/include/clang/Analysis/PathSensitive/BugReporter.h
index 0cf656765260173a115055843121274163e799eb..39e67f779875b0ed886ee58f2ce29274e4cb4355 100644 (file)
--- a/include/clang/Analysis/PathSensitive/BugReporter.h
+++ b/include/clang/Analysis/PathSensitive/BugReporter.h
@@ -223,12 +223,18 @@ public:
   
   virtual ~GRBugReporter();
   
+  /// getEngine - Return the analysis engine used to analyze a given
+  ///  function or method.
   GRExprEngine& getEngine() {
     return Eng;
   }
 
+  /// getGraph - Get the exploded graph created by the analysis engine
+  ///  for the analyzed method or function.
   ExplodedGraph<GRState>& getGraph();
   
+  /// getStateManager - Return the state manager used by the analysis
+  ///  engine.
   GRStateManager& getStateManager();
   
   virtual void GeneratePathDiagnostic(PathDiagnostic& PD, BugReport& R);
@@ -240,7 +246,8 @@ public:
   bool isNotable(SymbolID Sym) const {
     return (bool) NotableSymbols.count(Sym);
   }
-  
+    
+  /// classof - Used by isa<>, cast<>, and dyn_cast<>.
   static bool classof(const BugReporter* R) {
     return R->getKind() == GRBugReporterKind;
   }

diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h
index b1f4e70d39b8a86dcd13e0aba7e0286c2eb56e77..d08e442e35c79840d6c0b42a360df76c385123dc 100644 (file)
--- a/include/clang/Analysis/PathSensitive/GRExprEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h
@@ -216,6 +216,9 @@ public:
   const_bug_type_iterator bug_types_begin() const { return BugTypes.begin(); }
   const_bug_type_iterator bug_types_end() const { return BugTypes.end(); }
   
+  /// Register - Register a BugType with the analyzer engine.  A registered
+  ///  BugType object will have its 'EmitWarnings' method called when the
+  ///  the analyzer finishes analyzing a method or function.
   void Register(BugType* B) {
     BugTypes.push_back(B);
   }

diff --git a/lib/Analysis/BasicObjCFoundationChecks.cpp b/lib/Analysis/BasicObjCFoundationChecks.cpp
index 2b25e16fca82842a3b87fbf8d1120489c8d03e34..47785aca20f67f31fdac3b73da0e4786bf670df7 100644 (file)
--- a/lib/Analysis/BasicObjCFoundationChecks.cpp
+++ b/lib/Analysis/BasicObjCFoundationChecks.cpp
@@ -559,5 +559,8 @@ void clang::RegisterAppleChecks(GRExprEngine& Eng) {
   Eng.AddCheck(CreateBasicObjCFoundationChecks(Ctx, VMgr),
                Stmt::ObjCMessageExprClass);
 
-  Eng.AddCheck(CreateAuditCFNumberCreate(Ctx, VMgr), Stmt::CallExprClass);
+  Eng.AddCheck(CreateAuditCFNumberCreate(Ctx, VMgr),
+               Stmt::CallExprClass);
+  
+  Eng.Register(CreateNSErrorCheck());
 }

diff --git a/lib/Analysis/BasicObjCFoundationChecks.h b/lib/Analysis/BasicObjCFoundationChecks.h
index 24415419cacbd096aa6d1294e03d98bc0d82d1b6..3ad2ca57aa33c298d3c7f0ec7860e6f785d16806 100644 (file)
--- a/lib/Analysis/BasicObjCFoundationChecks.h
+++ b/lib/Analysis/BasicObjCFoundationChecks.h
@@ -29,14 +29,16 @@ namespace clang {
 class GRSimpleAPICheck;
 class ASTContext;
 class GRStateManager;  
+class BugType;
   
 GRSimpleAPICheck* CreateBasicObjCFoundationChecks(ASTContext& Ctx,
                                                   GRStateManager* VMgr);
   
 GRSimpleAPICheck* CreateAuditCFNumberCreate(ASTContext& Ctx,
                                             GRStateManager* VMgr);
-
-
+  
+BugType* CreateNSErrorCheck();
+  
 } // end clang namespace
 
 #endif

diff --git a/lib/Analysis/CheckNSError.cpp b/lib/Analysis/CheckNSError.cpp
index 26386bc2bb6b5ec1f27324e94098d52e619baeae..4c7f2cf25d1a7d3f65645938bb0b2a116bd0ae67 100644 (file)
--- a/lib/Analysis/CheckNSError.cpp
+++ b/lib/Analysis/CheckNSError.cpp
@@ -17,56 +17,90 @@
 
 #include "clang/Analysis/LocalCheckers.h"
 #include "clang/Analysis/PathSensitive/BugReporter.h"
+#include "clang/Analysis/PathSensitive/GRExprEngine.h"
+#include "BasicObjCFoundationChecks.h"
+#include "llvm/Support/Compiler.h"
 #include "clang/AST/DeclObjC.h"
-#include "clang/AST/Type.h"
-#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "llvm/ADT/SmallVector.h"
 
 using namespace clang;
 
-void clang::CheckNSError(ObjCImplementationDecl* ID, BugReporter& BR) {
-  // Look at the @interface for this class.
-  ObjCInterfaceDecl* D = ID->getClassInterface();
+namespace {
+class VISIBILITY_HIDDEN NSErrorCheck : public BugTypeCacheLocation {
+
+  void EmitGRWarnings(GRBugReporter& BR);
   
-  // Get the ASTContext.  Useful for querying type information.
-  ASTContext &Ctx = BR.getContext();
+  void CheckSignature(ObjCMethodDecl& MD, QualType& ResultTy,
+                      llvm::SmallVectorImpl<VarDecl*>& Params,
+                      IdentifierInfo* NSErrorII);
+
+  bool CheckArgument(QualType ArgTy, IdentifierInfo* NSErrorII);
   
-  // Get the IdentifierInfo* for "NSError".
-  IdentifierInfo* NSErrorII = &Ctx.Idents.get("NSError");
+public:
+  void EmitWarnings(BugReporter& BR) { EmitGRWarnings(cast<GRBugReporter>(BR));}
+  const char* getName() const { return "NSError** null dereference"; }
+};  
+  
+} // end anonymous namespace
 
-  // Scan the methods.  See if any of them have an argument of type NSError**.  
-  for (ObjCInterfaceDecl::instmeth_iterator I=D->instmeth_begin(),
-        E=D->instmeth_end(); I!=E; ++I) {
+BugType* clang::CreateNSErrorCheck() {
+  return new NSErrorCheck();
+}
 
-    // Get the method declaration.
-    ObjCMethodDecl* M = *I;
-    
-    // Check for a non-void return type.
-    if (M->getResultType() != Ctx.VoidTy)
-      continue;
+void NSErrorCheck::EmitGRWarnings(GRBugReporter& BR) {
+  // Get the analysis engine and the exploded analysis graph.
+  GRExprEngine& Eng = BR.getEngine();
+  GRExprEngine::GraphTy& G = Eng.getGraph();
+  
+  // Get the declaration of the method/function that was analyzed.
+  Decl& CodeDecl = G.getCodeDecl();
+  
+  ObjCMethodDecl* MD = dyn_cast<ObjCMethodDecl>(&CodeDecl);
+  if (!MD)
+    return;
+  
+  // Get the ASTContext, which is useful for querying type information.
+  ASTContext &Ctx = BR.getContext();
 
-    for (ObjCMethodDecl::param_iterator PI=M->param_begin(), 
-         PE=M->param_end(); PI!=PE; ++PI) {
-      
-      const PointerType* PPT = (*PI)->getType()->getAsPointerType();
-      if (!PPT) continue;
-      
-      const PointerType* PT = PPT->getPointeeType()->getAsPointerType();
-      if (!PT) continue;
-      
-      const ObjCInterfaceType *IT =
-        PT->getPointeeType()->getAsObjCInterfaceType();
-      
-      if (!IT) continue;
-      
-      // Check if IT is "NSError".
-      if (IT->getDecl()->getIdentifier() == NSErrorII) {
-        // Documentation: "Creating and Returning NSError Objects"
-        BR.EmitBasicReport("Bad return type when passing NSError**",
-         "Method accepting NSError** argument should have "
-         "non-void return value to indicate that an error occurred.",
-         M->getLocStart());
-        break;
-      }
-    }
+  QualType ResultTy;
+  llvm::SmallVector<VarDecl*, 5> Params;  
+  CheckSignature(*MD, ResultTy, Params, &Ctx.Idents.get("NSError"));
+  
+  if (Params.empty())
+    return;
+  
+  if (ResultTy == Ctx.VoidTy) {
+    BR.EmitBasicReport("Bad return type when passing NSError**",
+              "Method accepting NSError** argument should have "
+              "non-void return value to indicate that an error occurred.",
+              CodeDecl.getLocation());
+    
   }
 }
+
+void NSErrorCheck::CheckSignature(ObjCMethodDecl& M, QualType& ResultTy,
+                                  llvm::SmallVectorImpl<VarDecl*>& Params,
+                                  IdentifierInfo* NSErrorII) {
+
+  ResultTy = M.getResultType();
+  
+  for (ObjCMethodDecl::param_iterator I=M.param_begin(), 
+       E=M.param_end(); I!=E; ++I) 
+    if (CheckArgument((*I)->getType(), NSErrorII))
+      Params.push_back(*I);
+}
+
+bool NSErrorCheck::CheckArgument(QualType ArgTy, IdentifierInfo* NSErrorII) {
+  const PointerType* PPT = ArgTy->getAsPointerType();
+  if (!PPT) return false;
+  
+  const PointerType* PT = PPT->getPointeeType()->getAsPointerType();
+  if (!PT) return false;
+  
+  const ObjCInterfaceType *IT =
+  PT->getPointeeType()->getAsObjCInterfaceType();
+  
+  if (!IT) return false;
+  return IT->getDecl()->getIdentifier() == NSErrorII;
+}