Reapply "IntrusiveRefCntPtr -> std::shared_ptr for CompilerInvocation…

…Base and CodeCompleteConsumer"

Aleksey Shlypanikov pointed out my mistake in migrating an explicit
unique_ptr to auto - I was expecting the function returned a unique_ptr,
but instead it returned a raw pointer - introducing a leak.

Thanks Aleksey!

This reapplies r291184, reverted in r291249.

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

examples/clang-interpreter/main.cpp

@@ -145,7 +145,7 @@ int main(int argc, const char **argv, char * const *envp) {
 
   // Create a compiler instance to handle the actual work.
   CompilerInstance Clang;
-  Clang.setInvocation(CI.release());
+  Clang.setInvocation(std::move(CI));
 
   // Create the compilers actual diagnostics engine.
   Clang.createDiagnostics();

include/clang/Frontend/ASTUnit.h

@@ -108,8 +108,8 @@ class ASTUnit : public ModuleLoader {
 
   /// Optional owned invocation, just used to make the invocation used in
   /// LoadFromCommandLine available.
-  IntrusiveRefCntPtr<CompilerInvocation> Invocation;
-  
+  std::shared_ptr<CompilerInvocation> Invocation;
+
   // OnlyLocalDecls - when true, walking this AST should only visit declarations
   // that come from the AST itself, not from included precompiled headers.
   // FIXME: This is temporary; eventually, CIndex will always do this.
@@ -358,22 +358,21 @@ class ASTUnit : public ModuleLoader {
   }
 
   /// \brief Retrieve the allocator used to cache global code completions.
-  IntrusiveRefCntPtr<GlobalCodeCompletionAllocator>
+  std::shared_ptr<GlobalCodeCompletionAllocator>
   getCachedCompletionAllocator() {
     return CachedCompletionAllocator;
   }
 
   CodeCompletionTUInfo &getCodeCompletionTUInfo() {
     if (!CCTUInfo)
-      CCTUInfo.reset(new CodeCompletionTUInfo(
-                                            new GlobalCodeCompletionAllocator));
+      CCTUInfo = llvm::make_unique<CodeCompletionTUInfo>(
+          std::make_shared<GlobalCodeCompletionAllocator>());
     return *CCTUInfo;
   }
 
 private:
   /// \brief Allocator used to store cached code completions.
-  IntrusiveRefCntPtr<GlobalCodeCompletionAllocator>
-    CachedCompletionAllocator;
+  std::shared_ptr<GlobalCodeCompletionAllocator> CachedCompletionAllocator;
 
   std::unique_ptr<CodeCompletionTUInfo> CCTUInfo;
 
@@ -702,11 +701,11 @@ class ASTUnit : public ModuleLoader {
   /// remapped contents of that file.
   typedef std::pair<std::string, llvm::MemoryBuffer *> RemappedFile;
 
-  /// \brief Create a ASTUnit. Gets ownership of the passed CompilerInvocation. 
-  static ASTUnit *create(CompilerInvocation *CI,
-                         IntrusiveRefCntPtr<DiagnosticsEngine> Diags,
-                         bool CaptureDiagnostics,
-                         bool UserFilesAreVolatile);
+  /// \brief Create a ASTUnit. Gets ownership of the passed CompilerInvocation.
+  static std::unique_ptr<ASTUnit>
+  create(std::shared_ptr<CompilerInvocation> CI,
+         IntrusiveRefCntPtr<DiagnosticsEngine> Diags, bool CaptureDiagnostics,
+         bool UserFilesAreVolatile);
 
   /// \brief Create a ASTUnit from an AST file.
   ///
@@ -771,7 +770,7 @@ class ASTUnit : public ModuleLoader {
   /// created ASTUnit was passed in \p Unit then the caller can check that.
   ///
   static ASTUnit *LoadFromCompilerInvocationAction(
-      CompilerInvocation *CI,
+      std::shared_ptr<CompilerInvocation> CI,
       std::shared_ptr<PCHContainerOperations> PCHContainerOps,
       IntrusiveRefCntPtr<DiagnosticsEngine> Diags,
       FrontendAction *Action = nullptr, ASTUnit *Unit = nullptr,
@@ -798,7 +797,7 @@ class ASTUnit : public ModuleLoader {
   // FIXME: Move OnlyLocalDecls, UseBumpAllocator to setters on the ASTUnit, we
   // shouldn't need to specify them at construction time.
   static std::unique_ptr<ASTUnit> LoadFromCompilerInvocation(
-      CompilerInvocation *CI,
+      std::shared_ptr<CompilerInvocation> CI,
       std::shared_ptr<PCHContainerOperations> PCHContainerOps,
       IntrusiveRefCntPtr<DiagnosticsEngine> Diags, FileManager *FileMgr,
       bool OnlyLocalDecls = false, bool CaptureDiagnostics = false,

include/clang/Frontend/CompilerInstance.h

@@ -70,7 +70,7 @@ class TargetInfo;
 /// and a long form that takes explicit instances of any required objects.
 class CompilerInstance : public ModuleLoader {
   /// The options used in this compiler instance.
-  IntrusiveRefCntPtr<CompilerInvocation> Invocation;
+  std::shared_ptr<CompilerInvocation> Invocation;
 
   /// The diagnostics engine instance.
   IntrusiveRefCntPtr<DiagnosticsEngine> Diagnostics;
@@ -228,7 +228,7 @@ class CompilerInstance : public ModuleLoader {
   }
 
   /// setInvocation - Replace the current invocation.
-  void setInvocation(CompilerInvocation *Value);
+  void setInvocation(std::shared_ptr<CompilerInvocation> Value);
 
   /// \brief Indicates whether we should (re)build the global module index.
   bool shouldBuildGlobalModuleIndex() const;

include/clang/Frontend/CompilerInvocation.h

@@ -51,7 +51,7 @@ bool ParseDiagnosticArgs(DiagnosticOptions &Opts, llvm::opt::ArgList &Args,
                          bool DefaultDiagColor = true,
                          bool DefaultShowOpt = true);
 
-class CompilerInvocationBase : public RefCountedBase<CompilerInvocation> {
+class CompilerInvocationBase {
   void operator=(const CompilerInvocationBase &) = delete;
 
 public:

include/clang/Frontend/Utils.h

@@ -184,10 +184,10 @@ createChainedIncludesSource(CompilerInstance &CI,
 ///
 /// \return A CompilerInvocation, or 0 if none was built for the given
 /// argument vector.
-CompilerInvocation *
+std::unique_ptr<CompilerInvocation>
 createInvocationFromCommandLine(ArrayRef<const char *> Args,
-                            IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
-                                IntrusiveRefCntPtr<DiagnosticsEngine>());
+                                IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
+                                    IntrusiveRefCntPtr<DiagnosticsEngine>());
 
 /// Return the value of the last argument as an integer, or a default. If Diags
 /// is non-null, emits an error if the argument is given, but non-integral.

include/clang/Sema/CodeCompleteConsumer.h

@@ -509,23 +509,18 @@ class CodeCompletionAllocator : public llvm::BumpPtrAllocator {
 };
 
 /// \brief Allocator for a cached set of global code completions.
-class GlobalCodeCompletionAllocator 
-  : public CodeCompletionAllocator,
-    public RefCountedBase<GlobalCodeCompletionAllocator>
-{
-
-};
+class GlobalCodeCompletionAllocator : public CodeCompletionAllocator {};
 
 class CodeCompletionTUInfo {
   llvm::DenseMap<const DeclContext *, StringRef> ParentNames;
-  IntrusiveRefCntPtr<GlobalCodeCompletionAllocator> AllocatorRef;
+  std::shared_ptr<GlobalCodeCompletionAllocator> AllocatorRef;
 
 public:
   explicit CodeCompletionTUInfo(
-      IntrusiveRefCntPtr<GlobalCodeCompletionAllocator> Allocator)
+      std::shared_ptr<GlobalCodeCompletionAllocator> Allocator)
       : AllocatorRef(std::move(Allocator)) {}
 
-  IntrusiveRefCntPtr<GlobalCodeCompletionAllocator> getAllocatorRef() const {
+  std::shared_ptr<GlobalCodeCompletionAllocator> getAllocatorRef() const {
     return AllocatorRef;
   }
   CodeCompletionAllocator &getAllocator() const {
@@ -965,8 +960,8 @@ class PrintingCodeCompleteConsumer : public CodeCompleteConsumer {
   /// results to the given raw output stream.
   PrintingCodeCompleteConsumer(const CodeCompleteOptions &CodeCompleteOpts,
                                raw_ostream &OS)
-    : CodeCompleteConsumer(CodeCompleteOpts, false), OS(OS),
-      CCTUInfo(new GlobalCodeCompletionAllocator) {}
+      : CodeCompleteConsumer(CodeCompleteOpts, false), OS(OS),
+        CCTUInfo(std::make_shared<GlobalCodeCompletionAllocator>()) {}
 
   /// \brief Prints the finalized code-completion results.
   void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,

include/clang/Tooling/Tooling.h

@@ -69,7 +69,8 @@ class ToolAction {
 
   /// \brief Perform an action for an invocation.
   virtual bool
-  runInvocation(clang::CompilerInvocation *Invocation, FileManager *Files,
+  runInvocation(std::shared_ptr<clang::CompilerInvocation> Invocation,
+                FileManager *Files,
                 std::shared_ptr<PCHContainerOperations> PCHContainerOps,
                 DiagnosticConsumer *DiagConsumer) = 0;
 };
@@ -85,7 +86,8 @@ class FrontendActionFactory : public ToolAction {
   ~FrontendActionFactory() override;
 
   /// \brief Invokes the compiler with a FrontendAction created by create().
-  bool runInvocation(clang::CompilerInvocation *Invocation, FileManager *Files,
+  bool runInvocation(std::shared_ptr<clang::CompilerInvocation> Invocation,
+                     FileManager *Files,
                      std::shared_ptr<PCHContainerOperations> PCHContainerOps,
                      DiagnosticConsumer *DiagConsumer) override;
 
@@ -261,7 +263,7 @@ class ToolInvocation {
 
   bool runInvocation(const char *BinaryName,
                      clang::driver::Compilation *Compilation,
-                     clang::CompilerInvocation *Invocation,
+                     std::shared_ptr<clang::CompilerInvocation> Invocation,
                      std::shared_ptr<PCHContainerOperations> PCHContainerOps);
 
   std::vector<std::string> CommandLine;

lib/ARCMigrate/ARCMT.cpp

@@ -271,7 +271,7 @@ bool arcmt::checkForManualIssues(
   Diags->setClient(&errRec, /*ShouldOwnClient=*/false);
 
   std::unique_ptr<ASTUnit> Unit(ASTUnit::LoadFromCompilerInvocationAction(
-      CInvok.release(), PCHContainerOps, Diags));
+      std::move(CInvok), PCHContainerOps, Diags));
   if (!Unit) {
     errRec.FinishCapture();
     return true;
@@ -547,7 +547,7 @@ bool MigrationProcess::applyTransform(TransformFn trans,
   ASTAction.reset(new ARCMTMacroTrackerAction(ARCMTMacroLocs));
 
   std::unique_ptr<ASTUnit> Unit(ASTUnit::LoadFromCompilerInvocationAction(
-      CInvok.release(), PCHContainerOps, Diags, ASTAction.get()));
+      std::move(CInvok), PCHContainerOps, Diags, ASTAction.get()));
   if (!Unit) {
     errRec.FinishCapture();
     return true;

lib/Frontend/ASTUnit.cpp

@@ -245,7 +245,7 @@ ASTUnit::~ASTUnit() {
   // perform this operation here because we explicitly request that the
   // compiler instance *not* free these buffers for each invocation of the
   // parser.
-  if (Invocation.get() && OwnsRemappedFileBuffers) {
+  if (Invocation && OwnsRemappedFileBuffers) {
     PreprocessorOptions &PPOpts = Invocation->getPreprocessorOpts();
     for (const auto &RB : PPOpts.RemappedFileBuffers)
       delete RB.second;
@@ -348,7 +348,7 @@ void ASTUnit::CacheCodeCompletionResults() {
   // Gather the set of global code completions.
   typedef CodeCompletionResult Result;
   SmallVector<Result, 8> Results;
-  CachedCompletionAllocator = new GlobalCodeCompletionAllocator;
+  CachedCompletionAllocator = std::make_shared<GlobalCodeCompletionAllocator>();
   CodeCompletionTUInfo CCTUInfo(CachedCompletionAllocator);
   TheSema->GatherGlobalCodeCompletions(*CachedCompletionAllocator,
                                        CCTUInfo, Results);
@@ -1048,10 +1048,7 @@ bool ASTUnit::Parse(std::shared_ptr<PCHContainerOperations> PCHContainerOps,
   llvm::CrashRecoveryContextCleanupRegistrar<CompilerInstance>
     CICleanup(Clang.get());
 
-  IntrusiveRefCntPtr<CompilerInvocation>
-    CCInvocation(new CompilerInvocation(*Invocation));
-
-  Clang->setInvocation(CCInvocation.get());
+  Clang->setInvocation(std::make_shared<CompilerInvocation>(*Invocation));
   OriginalSourceFile = Clang->getFrontendOpts().Inputs[0].getFile();
 
   // Set up diagnostics, capturing any diagnostics that would
@@ -1344,8 +1341,8 @@ ASTUnit::getMainBufferWithPrecompiledPreamble(
     const CompilerInvocation &PreambleInvocationIn, bool AllowRebuild,
     unsigned MaxLines) {
 
-  IntrusiveRefCntPtr<CompilerInvocation>
-    PreambleInvocation(new CompilerInvocation(PreambleInvocationIn));
+  auto PreambleInvocation =
+      std::make_shared<CompilerInvocation>(PreambleInvocationIn);
   FrontendOptions &FrontendOpts = PreambleInvocation->getFrontendOpts();
   PreprocessorOptions &PreprocessorOpts
     = PreambleInvocation->getPreprocessorOpts();
@@ -1523,7 +1520,7 @@ ASTUnit::getMainBufferWithPrecompiledPreamble(
   llvm::CrashRecoveryContextCleanupRegistrar<CompilerInstance>
     CICleanup(Clang.get());
 
-  Clang->setInvocation(&*PreambleInvocation);
+  Clang->setInvocation(std::move(PreambleInvocation));
   OriginalSourceFile = Clang->getFrontendOpts().Inputs[0].getFile();
 
   // Set up diagnostics, capturing all of the diagnostics produced.
@@ -1709,30 +1706,29 @@ StringRef ASTUnit::getASTFileName() const {
   return Mod.FileName;
 }
 
-ASTUnit *ASTUnit::create(CompilerInvocation *CI,
-                         IntrusiveRefCntPtr<DiagnosticsEngine> Diags,
-                         bool CaptureDiagnostics,
-                         bool UserFilesAreVolatile) {
-  std::unique_ptr<ASTUnit> AST;
-  AST.reset(new ASTUnit(false));
+std::unique_ptr<ASTUnit>
+ASTUnit::create(std::shared_ptr<CompilerInvocation> CI,
+                IntrusiveRefCntPtr<DiagnosticsEngine> Diags,
+                bool CaptureDiagnostics, bool UserFilesAreVolatile) {
+  std::unique_ptr<ASTUnit> AST(new ASTUnit(false));
   ConfigureDiags(Diags, *AST, CaptureDiagnostics);
-  AST->Diagnostics = Diags;
-  AST->Invocation = CI;
-  AST->FileSystemOpts = CI->getFileSystemOpts();
   IntrusiveRefCntPtr<vfs::FileSystem> VFS =
       createVFSFromCompilerInvocation(*CI, *Diags);
   if (!VFS)
     return nullptr;
+  AST->Diagnostics = Diags;
+  AST->FileSystemOpts = CI->getFileSystemOpts();
+  AST->Invocation = std::move(CI);
   AST->FileMgr = new FileManager(AST->FileSystemOpts, VFS);
   AST->UserFilesAreVolatile = UserFilesAreVolatile;
   AST->SourceMgr = new SourceManager(AST->getDiagnostics(), *AST->FileMgr,
                                      UserFilesAreVolatile);
 
-  return AST.release();
+  return AST;
 }
 
 ASTUnit *ASTUnit::LoadFromCompilerInvocationAction(
-    CompilerInvocation *CI,
+    std::shared_ptr<CompilerInvocation> CI,
     std::shared_ptr<PCHContainerOperations> PCHContainerOps,
     IntrusiveRefCntPtr<DiagnosticsEngine> Diags, FrontendAction *Action,
     ASTUnit *Unit, bool Persistent, StringRef ResourceFilesPath,
@@ -1746,7 +1742,7 @@ ASTUnit *ASTUnit::LoadFromCompilerInvocationAction(
   ASTUnit *AST = Unit;
   if (!AST) {
     // Create the AST unit.
-    OwnAST.reset(create(CI, Diags, CaptureDiagnostics, UserFilesAreVolatile));
+    OwnAST = create(CI, Diags, CaptureDiagnostics, UserFilesAreVolatile);
     AST = OwnAST.get();
     if (!AST)
       return nullptr;
@@ -1785,7 +1781,7 @@ ASTUnit *ASTUnit::LoadFromCompilerInvocationAction(
   llvm::CrashRecoveryContextCleanupRegistrar<CompilerInstance>
     CICleanup(Clang.get());
 
-  Clang->setInvocation(CI);
+  Clang->setInvocation(std::move(CI));
   AST->OriginalSourceFile = Clang->getFrontendOpts().Inputs[0].getFile();
 
   // Set up diagnostics, capturing any diagnostics that would
@@ -1903,7 +1899,7 @@ bool ASTUnit::LoadFromCompilerInvocation(
 }
 
 std::unique_ptr<ASTUnit> ASTUnit::LoadFromCompilerInvocation(
-    CompilerInvocation *CI,
+    std::shared_ptr<CompilerInvocation> CI,
     std::shared_ptr<PCHContainerOperations> PCHContainerOps,
     IntrusiveRefCntPtr<DiagnosticsEngine> Diags, FileManager *FileMgr,
     bool OnlyLocalDecls, bool CaptureDiagnostics,
@@ -1920,7 +1916,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromCompilerInvocation(
   AST->ShouldCacheCodeCompletionResults = CacheCodeCompletionResults;
   AST->IncludeBriefCommentsInCodeCompletion
     = IncludeBriefCommentsInCodeCompletion;
-  AST->Invocation = CI;
+  AST->Invocation = std::move(CI);
   AST->FileSystemOpts = FileMgr->getFileSystemOpts();
   AST->FileMgr = FileMgr;
   AST->UserFilesAreVolatile = UserFilesAreVolatile;
@@ -1952,17 +1948,16 @@ ASTUnit *ASTUnit::LoadFromCommandLine(
   assert(Diags.get() && "no DiagnosticsEngine was provided");
 
   SmallVector<StoredDiagnostic, 4> StoredDiagnostics;
-  
-  IntrusiveRefCntPtr<CompilerInvocation> CI;
+
+  std::shared_ptr<CompilerInvocation> CI;
 
   {
 
     CaptureDroppedDiagnostics Capture(CaptureDiagnostics, *Diags, 
                                       StoredDiagnostics);
 
     CI = clang::createInvocationFromCommandLine(
-                                           llvm::makeArrayRef(ArgBegin, ArgEnd),
-                                           Diags);
+        llvm::makeArrayRef(ArgBegin, ArgEnd), Diags);
     if (!CI)
       return nullptr;
   }
@@ -2333,8 +2328,7 @@ void ASTUnit::CodeComplete(
   CompletionTimer.setOutput("Code completion @ " + File + ":" +
                             Twine(Line) + ":" + Twine(Column));
 
-  IntrusiveRefCntPtr<CompilerInvocation>
-    CCInvocation(new CompilerInvocation(*Invocation));
+  auto CCInvocation = std::make_shared<CompilerInvocation>(*Invocation);
 
   FrontendOptions &FrontendOpts = CCInvocation->getFrontendOpts();
   CodeCompleteOptions &CodeCompleteOpts = FrontendOpts.CodeCompleteOpts;
@@ -2366,16 +2360,17 @@ void ASTUnit::CodeComplete(
   llvm::CrashRecoveryContextCleanupRegistrar<CompilerInstance>
     CICleanup(Clang.get());
 
-  Clang->setInvocation(&*CCInvocation);
+  auto &Inv = *CCInvocation;
+  Clang->setInvocation(std::move(CCInvocation));
   OriginalSourceFile = Clang->getFrontendOpts().Inputs[0].getFile();
 
   // Set up diagnostics, capturing any diagnostics produced.
   Clang->setDiagnostics(&Diag);
   CaptureDroppedDiagnostics Capture(true, 
                                     Clang->getDiagnostics(), 
                                     StoredDiagnostics);
-  ProcessWarningOptions(Diag, CCInvocation->getDiagnosticOpts());
-  
+  ProcessWarningOptions(Diag, Inv.getDiagnosticOpts());
+
   // Create the target instance.
   Clang->setTarget(TargetInfo::CreateTargetInfo(
       Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
@@ -2431,7 +2426,7 @@ void ASTUnit::CodeComplete(
       if (!llvm::sys::fs::getUniqueID(MainPath, MainID)) {
         if (CompleteFileID == MainID && Line > 1)
           OverrideMainBuffer = getMainBufferWithPrecompiledPreamble(
-              PCHContainerOps, *CCInvocation, false, Line - 1);
+              PCHContainerOps, Inv, false, Line - 1);
       }
     }
   }