[Analyzer] Do not use static storage to for implementations created in BodyFarm.cpp

[dcoughlin]

Content-wise, LGTM. There is a style nit inline.

Also, can you avoid reformatting the lines that haven't changed? This will help preserve the history of the file and make it clear what changes are related to your intended change in functionality.

lib/Analysis/AnalysisDeclContext.cpp

308 ↗

(On Diff #119949)

Nit: Style-wise this is idiomatically written as:

if (!BdyFrm)

[george.karpenkov]

@dcoughlin that's not me, that's clang-format.
If we agree on always running it, I think the changes should stay there.

[dcoughlin]

I think a good strategy is to look at your diffs and consider whether the benefits of normalizing the style outweigh the cost of losing the history. In this case, I think keeping the history makes sense. (Imagine you are a future maintainer and want to know when and why a new option was added. Is is very helpful to use git annotate to understand why the code is the way it is.)

[george.karpenkov]

@dcoughlin the context I was thinking about is that if everyone consistently runs clang-format (if we want that), then we never would have discussion.
The alternative is that every run of clang-format would be followed by manually reverting changes which were introduced by mistake (in this case, because the file was moved).

[dcoughlin]

That would require going into the past and requiring everyone back then to run clang-format as well. Unfortunately they didn't -- so human judgment is needed when modifying code that doesn't obey the guidelines.

[george.karpenkov]

@dcoughlin OK sorry, I haven't considered the point that codebase predates the requirement, and the master format-all commit was never done.
I've committed this one already, but I will be more careful with applying clang-format to future changes.

[alexfh]

cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp

607

This is an almost guaranteed memory leak. I think, you meant if (BdyFrm) delete BdyFrm;. But delete deals well with nullptr, so just delete the pointer unconditionally. But first consider making BdyFrm a std::unique_ptr. From a cursory look I don't see any reasons not to.

[george.karpenkov]

cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp

607

@alexfh yes, this was fixed.
Also, yes, https://reviews.llvm.org/D39220

cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h

 // context data for path sensitive analysis.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_ANALYSIS_ANALYSISDECLCONTEXT_H
 #define LLVM_CLANG_ANALYSIS_ANALYSISDECLCONTEXT_H
 
 #include "clang/AST/Decl.h"
+#include "clang/Analysis/BodyFarm.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/Analysis/CodeInjector.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/Support/Allocator.h"
 #include <memory>
 
 namespace clang {
const LOC *getLocationContext(AnalysisDeclContext *ctx,
                                 const LocationContext *parent,
                                 const DATA *d);
 };
 
 class AnalysisDeclContextManager {
   typedef llvm::DenseMap<const Decl *, std::unique_ptr<AnalysisDeclContext>>
       ContextMap;
 
+  ASTContext &ASTCtx;
   ContextMap Contexts;
   LocationContextManager LocContexts;
   CFG::BuildOptions cfgBuildOptions;
 
   /// Pointer to an interface that can provide function bodies for
   /// declarations from external source.
   std::unique_ptr<CodeInjector> Injector;
-  
+  /// Pointer to a factory for creating and caching implementations for common
+  /// methods during the analysis.
+  BodyFarm *BdyFrm = nullptr;
+
   /// Flag to indicate whether or not bodies should be synthesized
   /// for well-known functions.
   bool SynthesizeBodies;
 
 public:
-  AnalysisDeclContextManager(bool useUnoptimizedCFG = false,
+  AnalysisDeclContextManager(ASTContext &ASTCtx, bool useUnoptimizedCFG = false,
                              bool addImplicitDtors = false,
                              bool addInitializers = false,
                              bool addTemporaryDtors = false,
-                             bool addLifetime = false,
-                             bool addLoopExit = false,
+                             bool addLifetime = false, bool addLoopExit = false,
                              bool synthesizeBodies = false,
                              bool addStaticInitBranches = false,
                              bool addCXXNewAllocator = true,
-                             CodeInjector* injector = nullptr);
+                             CodeInjector *injector = nullptr);
 
   ~AnalysisDeclContextManager();
 
   AnalysisDeclContext *getContext(const Decl *D);
 
   bool getUseUnoptimizedCFG() const {
     return !cfgBuildOptions.PruneTriviallyFalseEdges;
   }
public:
   StackFrameContext const *getStackFrame(const Decl *D,
                                          LocationContext const *Parent,
                                          const Stmt *S,
                                          const CFGBlock *Blk,
                                          unsigned Idx) {
     return LocContexts.getStackFrame(getContext(D), Parent, S, Blk, Idx);
   }
 
+  /// Get and lazily create a {@code BodyFarm} instance.
+  BodyFarm *getBodyFarm();
+
   /// Discard all previously created AnalysisDeclContexts.
   void clear();
 
 private:
   friend class AnalysisDeclContext;
 
   LocationContextManager &getLocationContextManager() {
     return LocContexts;
   }
 };
 
 } // end clang namespace
 #endif

cfe/trunk/include/clang/Analysis/BodyFarm.h

+//== BodyFarm.h - Factory for conjuring up fake bodies -------------*- C++ -*-//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// BodyFarm is a factory for creating faux implementations for functions/methods
+// for analysis purposes.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_ANALYSIS_BODYFARM_H
+#define LLVM_CLANG_LIB_ANALYSIS_BODYFARM_H
+
+#include "clang/AST/DeclBase.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/Optional.h"
+
+namespace clang {
+
+class ASTContext;
+class FunctionDecl;
+class ObjCMethodDecl;
+class ObjCPropertyDecl;
+class Stmt;
+class CodeInjector;
+
+class BodyFarm {
+public:
+  BodyFarm(ASTContext &C, CodeInjector *injector) : C(C), Injector(injector) {}
+
+  /// Factory method for creating bodies for ordinary functions.
+  Stmt *getBody(const FunctionDecl *D);
+
+  /// Factory method for creating bodies for Objective-C properties.
+  Stmt *getBody(const ObjCMethodDecl *D);
+
+private:
+  typedef llvm::DenseMap<const Decl *, Optional<Stmt *>> BodyMap;
+
+  ASTContext &C;
+  BodyMap Bodies;
+  CodeInjector *Injector;
+};
+} // namespace clang
+
+#endif

cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp

 //== AnalysisDeclContext.cpp - Analysis context for Path Sens analysis -*- C++ -*-//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines AnalysisDeclContext, a class that manages the analysis context
 // data for path sensitive analysis.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Analysis/AnalysisDeclContext.h"
-#include "BodyFarm.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/ParentMap.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
 #include "clang/Analysis/Analyses/PseudoConstantAnalysis.h"
+#include "clang/Analysis/BodyFarm.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/Analysis/CFGStmtMap.h"
 #include "clang/Analysis/Support/BumpVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/SaveAndRestore.h"
 #include "llvm/Support/raw_ostream.h"
 
: Manager(Mgr),
   builtCFG(false),
   builtCompleteCFG(false),
   ReferencedBlockVars(nullptr),
   ManagedAnalyses(nullptr)
 {  
   cfgBuildOptions.forcedBlkExprs = &forcedBlkExprs;
 }
 
-AnalysisDeclContextManager::AnalysisDeclContextManager(bool useUnoptimizedCFG,
-                                                       bool addImplicitDtors,
-                                                       bool addInitializers,
-                                                       bool addTemporaryDtors,
-                                                       bool addLifetime,
-                                                       bool addLoopExit,
-                                                       bool synthesizeBodies,
-                                                       bool addStaticInitBranch,
-                                                       bool addCXXNewAllocator,
-                                                       CodeInjector *injector)
-  : Injector(injector), SynthesizeBodies(synthesizeBodies)
-{
+AnalysisDeclContextManager::AnalysisDeclContextManager(
+    ASTContext &ASTCtx, bool useUnoptimizedCFG, bool addImplicitDtors,
+    bool addInitializers, bool addTemporaryDtors, bool addLifetime,
+    bool addLoopExit, bool synthesizeBodies, bool addStaticInitBranch,
+    bool addCXXNewAllocator, CodeInjector *injector)
+    : ASTCtx(ASTCtx), Injector(injector), SynthesizeBodies(synthesizeBodies) {
   cfgBuildOptions.PruneTriviallyFalseEdges = !useUnoptimizedCFG;
   cfgBuildOptions.AddImplicitDtors = addImplicitDtors;
   cfgBuildOptions.AddInitializers = addInitializers;
   cfgBuildOptions.AddTemporaryDtors = addTemporaryDtors;
   cfgBuildOptions.AddLifetime = addLifetime;
   cfgBuildOptions.AddLoopExit = addLoopExit;
   cfgBuildOptions.AddStaticInitBranches = addStaticInitBranch;
   cfgBuildOptions.AddCXXNewAllocator = addCXXNewAllocator;
 }
 
 void AnalysisDeclContextManager::clear() { Contexts.clear(); }
 
-static BodyFarm &getBodyFarm(ASTContext &C, CodeInjector *injector = nullptr) {
-  static BodyFarm *BF = new BodyFarm(C, injector);
-  return *BF;
-}
-
 Stmt *AnalysisDeclContext::getBody(bool &IsAutosynthesized) const {
   IsAutosynthesized = false;
   if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
     Stmt *Body = FD->getBody();
     if (auto *CoroBody = dyn_cast_or_null<CoroutineBodyStmt>(Body))
       Body = CoroBody->getBody();
     if (Manager && Manager->synthesizeBodies()) {
-      Stmt *SynthesizedBody =
-          getBodyFarm(getASTContext(), Manager->Injector.get()).getBody(FD);
+      Stmt *SynthesizedBody = Manager->getBodyFarm()->getBody(FD);
       if (SynthesizedBody) {
         Body = SynthesizedBody;
         IsAutosynthesized = true;
       }
     }
     return Body;
   }
   else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
     Stmt *Body = MD->getBody();
     if (Manager && Manager->synthesizeBodies()) {
-      Stmt *SynthesizedBody =
-          getBodyFarm(getASTContext(), Manager->Injector.get()).getBody(MD);
+      Stmt *SynthesizedBody = Manager->getBodyFarm()->getBody(MD);
       if (SynthesizedBody) {
         Body = SynthesizedBody;
         IsAutosynthesized = true;
       }
     }
     return Body;
   } else if (const BlockDecl *BD = dyn_cast<BlockDecl>(D))
     return BD->getBody();
AnalysisDeclContext *AnalysisDeclContextManager::getContext(const Decl *D) {
   }
 
   std::unique_ptr<AnalysisDeclContext> &AC = Contexts[D];
   if (!AC)
     AC = llvm::make_unique<AnalysisDeclContext>(this, D, cfgBuildOptions);
   return AC.get();
 }
 
+BodyFarm *AnalysisDeclContextManager::getBodyFarm() {
+  if (!BdyFrm)
+    BdyFrm = new BodyFarm(ASTCtx, Injector.get());
+  return BdyFrm;
+}
+
 const StackFrameContext *
 AnalysisDeclContext::getStackFrame(LocationContext const *Parent, const Stmt *S,
                                const CFGBlock *Blk, unsigned Idx) {
   return getLocationContextManager().getStackFrame(this, Parent, S, Blk, Idx);
 }
 
 const BlockInvocationContext *
 AnalysisDeclContext::getBlockInvocationContext(const LocationContext *parent,
AnalysisDeclContext::~AnalysisDeclContext() {
   // Release the managed analyses.
   if (ManagedAnalyses) {
     ManagedAnalysisMap *M = (ManagedAnalysisMap*) ManagedAnalyses;
     llvm::DeleteContainerSeconds(*M);
     delete M;
   }
 }
 
-AnalysisDeclContextManager::~AnalysisDeclContextManager() {}
+AnalysisDeclContextManager::~AnalysisDeclContextManager() {
+  if (!BdyFrm)
+    delete BdyFrm;
+}
 
 LocationContext::~LocationContext() {}
 
 LocationContextManager::~LocationContextManager() {
   clear();
 }
 
 void LocationContextManager::clear() {

cfe/trunk/lib/Analysis/BodyFarm.h

-//== BodyFarm.h - Factory for conjuring up fake bodies -------------*- C++ -*-//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-//
-// BodyFarm is a factory for creating faux implementations for functions/methods
-// for analysis purposes.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_CLANG_LIB_ANALYSIS_BODYFARM_H
-#define LLVM_CLANG_LIB_ANALYSIS_BODYFARM_H
-
-#include "clang/AST/DeclBase.h"
-#include "clang/Basic/LLVM.h"
-#include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/Optional.h"
-
-namespace clang {
-
-class ASTContext;
-class FunctionDecl;
-class ObjCMethodDecl;
-class ObjCPropertyDecl;
-class Stmt;
-class CodeInjector;
-  
-class BodyFarm {
-public:
-  BodyFarm(ASTContext &C, CodeInjector *injector) : C(C), Injector(injector) {}
-  
-  /// Factory method for creating bodies for ordinary functions.
-  Stmt *getBody(const FunctionDecl *D);
-
-  /// Factory method for creating bodies for Objective-C properties.
-  Stmt *getBody(const ObjCMethodDecl *D);
-
-private:
-  typedef llvm::DenseMap<const Decl *, Optional<Stmt *> > BodyMap;
-
-  ASTContext &C;
-  BodyMap Bodies;
-  CodeInjector *Injector;
-};
-}
-
-#endif

cfe/trunk/lib/Analysis/BodyFarm.cpp

 //== BodyFarm.cpp  - Factory for conjuring up fake bodies ----------*- C++ -*-//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // BodyFarm is a factory for creating faux implementations for functions/methods
 // for analysis purposes.
 //
 //===----------------------------------------------------------------------===//
 
-#include "BodyFarm.h"
+#include "clang/Analysis/BodyFarm.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/Analysis/CodeInjector.h"

cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp

 //===-- AnalysisManager.cpp -------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 
 using namespace clang;
 using namespace ento;
 
 void AnalysisManager::anchor() { }
 
-AnalysisManager::AnalysisManager(ASTContext &ctx, DiagnosticsEngine &diags,
-                                 const LangOptions &lang,
-                                 const PathDiagnosticConsumers &PDC,
-                                 StoreManagerCreator storemgr,
-                                 ConstraintManagerCreator constraintmgr,
-                                 CheckerManager *checkerMgr,
-                                 AnalyzerOptions &Options,
-                                 CodeInjector *injector)
-  : AnaCtxMgr(Options.UnoptimizedCFG,
+AnalysisManager::AnalysisManager(
+    ASTContext &ASTCtx, DiagnosticsEngine &diags, const LangOptions &lang,
+    const PathDiagnosticConsumers &PDC, StoreManagerCreator storemgr,
+    ConstraintManagerCreator constraintmgr, CheckerManager *checkerMgr,
+    AnalyzerOptions &Options, CodeInjector *injector)
+    : AnaCtxMgr(ASTCtx, Options.UnoptimizedCFG,
                 Options.includeImplicitDtorsInCFG(),
-              /*AddInitializers=*/true,
-              Options.includeTemporaryDtorsInCFG(),
-              Options.includeLifetimeInCFG(),
+                /*AddInitializers=*/true, Options.includeTemporaryDtorsInCFG(),
+                Options.includeLifetimeInCFG(),
                 // Adding LoopExit elements to the CFG is a requirement for loop
                 // unrolling.
                 Options.includeLoopExitInCFG() || Options.shouldUnrollLoops(),
                 Options.shouldSynthesizeBodies(),
                 Options.shouldConditionalizeStaticInitializers(),
                 /*addCXXNewAllocator=*/true,
                 injector),
-    Ctx(ctx),
-    Diags(diags),
-    LangOpts(lang),
-    PathConsumers(PDC),
+      Ctx(ASTCtx), Diags(diags), LangOpts(lang), PathConsumers(PDC),
       CreateStoreMgr(storemgr), CreateConstraintMgr(constraintmgr),
-    CheckerMgr(checkerMgr),
-    options(Options) {
+      CheckerMgr(checkerMgr), options(Options) {
   AnaCtxMgr.getCFGBuildOptions().setAllAlwaysAdd();
 }
 
 AnalysisManager::~AnalysisManager() {
   FlushDiagnostics();
   for (PathDiagnosticConsumers::iterator I = PathConsumers.begin(),
        E = PathConsumers.end(); I != E; ++I) {
     delete *I;