[Analyzer] As suggested, use value storage for BodyFarm

[george.karpenkov]

Summary

I've also removed copy constructor to help clients not to shoot themselves in the foot with BodyFarm stored = Manager.getBodyFarm().

[dblaikie]

Looks good - thanks!

[xazax.hun]

Some minor nits otherwise LGTM!

include/clang/Analysis/AnalysisDeclContext.h

479 ↗

(On Diff #120861)

The reference is on the wrong side.

include/clang/Analysis/BodyFarm.h

43 ↗

(On Diff #120861)

Reference is on the wrong side.

lib/Analysis/AnalysisDeclContext.cpp

308 ↗

(On Diff #120861)

Same as above.

[george.karpenkov]

@xazax.hun I'm really not convinced:

george@/Volumes/Transcend/code/llvm (master)≻ rg "\w+\&" tools/clang/include/clang/StaticAnalyzer
tools/clang/include/clang/StaticAnalyzer/Core/Checker.h
31:  static void _checkDecl(void *checker, const Decl *D, AnalysisManager& mgr,
50:  static void _checkBody(void *checker, const Decl *D, AnalysisManager& mgr,
67:                                         AnalysisManager& mgr,
489:raw_ostream& operator<<(raw_ostream &Out, const CheckerBase &Checker);
554:  /*implicit*/ operator bool&() { return val; }
555:  /*implicit*/ operator const bool&() const { return val; }

tools/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
170:  void runCheckersOnASTDecl(const Decl *D, AnalysisManager& mgr,
174:  void runCheckersOnASTBody(const Decl *D, AnalysisManager& mgr,
400:  typedef CheckerFn<void (const Decl *, AnalysisManager&, BugReporter &)>
469:                          AnalysisManager&, BugReporter &)>
...

On top of that, reference is part of the type, not part of the variable name,

[xazax.hun]

In D39428#912126, @george.karpenkov wrote:

@xazax.hun I'm really not convinced:

Unfortunately, the LLVM codebase right now does not strictly follow the style guide. But clang-format puts the references next to the variable (or function in case of return type) names. Also if you check the code snippets in the coding standard: https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto you can see that where we officially put the references.

[zaks.anna]

Also if you check the code snippets in the coding standard: https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto you can see that where we officially put the references.

Correct! The reference should not go with the type name.
George, please, address the comments before committing.

[dcoughlin]

Looks good to me with the comments by Gabor addressed.

In D39428#912126, @george.karpenkov wrote:

On top of that, reference is part of the type, not part of the variable name,

The reference is parsed as part of the declarator (associated with the variable name) rather than as part of the type.

Where this rears its ugly head is when declaring multiple variables in the same statement:

int g = 7;
void foo() {
  int& p = g, q = 7;
}

Here p has type 'int &' but q has type 'int'. The same holds for '*' for pointer types in C.

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

class AnalysisDeclContextManager {
   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
+  /// A factory for creating and caching implementations for common
   /// methods during the analysis.
-  std::unique_ptr<BodyFarm> FunctionBodyFarm;
+  BodyFarm FunctionBodyFarm;
 
   /// Flag to indicate whether or not bodies should be synthesized
   /// for well-known functions.
   bool SynthesizeBodies;
 
 public:
   AnalysisDeclContextManager(ASTContext &ASTCtx, bool useUnoptimizedCFG = false,
                              bool addImplicitDtors = false,
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();
+  /// Get a reference to {@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

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);
 
+  /// Remove copy constructor to avoid accidental copying.
+  BodyFarm(const BodyFarm &other) = delete;
+
 private:
   typedef llvm::DenseMap<const Decl *, Optional<Stmt *>> BodyMap;
 
   ASTContext &C;
   BodyMap Bodies;
   CodeInjector *Injector;
 };
 } // namespace clang
 
 #endif

cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp

: Manager(Mgr),
   cfgBuildOptions.forcedBlkExprs = &forcedBlkExprs;
 }
 
 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) {
+    : ASTCtx(ASTCtx), Injector(injector), FunctionBodyFarm(ASTCtx, 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(); }
 
 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 = Manager->getBodyFarm()->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 = Manager->getBodyFarm()->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 (!FunctionBodyFarm)
-    FunctionBodyFarm = llvm::make_unique<BodyFarm>(ASTCtx, Injector.get());
-  return FunctionBodyFarm.get();
-}
+BodyFarm &AnalysisDeclContextManager::getBodyFarm() { return FunctionBodyFarm; }
 
 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 *