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

llvm-svn: 56312

clang/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)
 

clang/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.
 //===----------------------------------------------------------------------===//

clang/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);
   

clang/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);
@@ -241,6 +247,7 @@ public:
     return (bool) NotableSymbols.count(Sym);
   }
     
+  /// classof - Used by isa<>, cast<>, and dyn_cast<>.
   static bool classof(const BugReporter* R) {
     return R->getKind() == GRBugReporterKind;
   }

clang/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);
   }

clang/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());
 }

clang/lib/Analysis/BasicObjCFoundationChecks.h

@@ -29,6 +29,7 @@ namespace clang {
 class GRSimpleAPICheck;
 class ASTContext;
 class GRStateManager;  
+class BugType;
   
 GRSimpleAPICheck* CreateBasicObjCFoundationChecks(ASTContext& Ctx,
                                                   GRStateManager* VMgr);
@@ -36,6 +37,7 @@ GRSimpleAPICheck* CreateBasicObjCFoundationChecks(ASTContext& Ctx,
 GRSimpleAPICheck* CreateAuditCFNumberCreate(ASTContext& Ctx,
                                             GRStateManager* VMgr);
   
+BugType* CreateNSErrorCheck();
   
 } // end clang namespace
 

clang/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 {
 
-  // Get the ASTContext.  Useful for querying type information.
+  void EmitGRWarnings(GRBugReporter& BR);
+  
+  void CheckSignature(ObjCMethodDecl& MD, QualType& ResultTy,
+                      llvm::SmallVectorImpl<VarDecl*>& Params,
+                      IdentifierInfo* NSErrorII);
+
+  bool CheckArgument(QualType ArgTy, IdentifierInfo* NSErrorII);
+  
+public:
+  void EmitWarnings(BugReporter& BR) { EmitGRWarnings(cast<GRBugReporter>(BR));}
+  const char* getName() const { return "NSError** null dereference"; }
+};  
+  
+} // end anonymous namespace
+
+BugType* clang::CreateNSErrorCheck() {
+  return new NSErrorCheck();
+}
+
+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();
 
-  // Get the IdentifierInfo* for "NSError".
-  IdentifierInfo* NSErrorII = &Ctx.Idents.get("NSError");
+  QualType ResultTy;
+  llvm::SmallVector<VarDecl*, 5> Params;  
+  CheckSignature(*MD, ResultTy, Params, &Ctx.Idents.get("NSError"));
   
-  // 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) {
+  if (Params.empty())
+    return;
   
-    // Get the method declaration.
-    ObjCMethodDecl* M = *I;
+  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());
     
-    // Check for a non-void return type.
-    if (M->getResultType() != Ctx.VoidTy)
-      continue;
-
-    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;
-      }
-    }
   }
 }
+
+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;
+}