Changed FrontendActionFactory::create to return a std::unique_ptr

[ilya-biryukov]

LGTM!

[lebedev.ri]

Commented on wrong patch.

In D66960#1651198, @lebedev.ri wrote:

This is missing documentation changes.
And this likely would be good to mention in releasenotes.

clang-tools-extra/clang-doc/ClangDoc.cpp

 #include "clang/Frontend/FrontendActions.h"
 
 namespace clang {
 namespace doc {
 
 class MapperActionFactory : public tooling::FrontendActionFactory {
 public:
   MapperActionFactory(ClangDocContext CDCtx) : CDCtx(CDCtx) {}
-  clang::FrontendAction *create() override;
+  std::unique_ptr<FrontendAction> create() override;
 
 private:
   ClangDocContext CDCtx;
 };
 
-clang::FrontendAction *MapperActionFactory::create() {
+std::unique_ptr<FrontendAction> MapperActionFactory::create() {
   class ClangDocAction : public clang::ASTFrontendAction {
   public:
     ClangDocAction(ClangDocContext CDCtx) : CDCtx(CDCtx) {}
 
     std::unique_ptr<clang::ASTConsumer>
     CreateASTConsumer(clang::CompilerInstance &Compiler,
                       llvm::StringRef InFile) override {
       return std::make_unique<MapASTVisitor>(&Compiler.getASTContext(), CDCtx);
     }
 
   private:
     ClangDocContext CDCtx;
   };
-  return new ClangDocAction(CDCtx);
+  return std::make_unique<ClangDocAction>(CDCtx);
 }
 
 std::unique_ptr<tooling::FrontendActionFactory>
 newMapperActionFactory(ClangDocContext CDCtx) {
   return std::make_unique<MapperActionFactory>(CDCtx);
 }
 
 } // namespace doc
 } // namespace clang

clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbolsAction.h

 
 class FindAllSymbolsActionFactory : public tooling::FrontendActionFactory {
 public:
   FindAllSymbolsActionFactory(
       SymbolReporter *Reporter,
       const HeaderMapCollector::RegexHeaderMap *RegexHeaderMap = nullptr)
       : Reporter(Reporter), RegexHeaderMap(RegexHeaderMap) {}
 
-  clang::FrontendAction *create() override {
-    return new FindAllSymbolsAction(Reporter, RegexHeaderMap);
+  std::unique_ptr<FrontendAction> create() override {
+    return std::make_unique<FindAllSymbolsAction>(Reporter, RegexHeaderMap);
   }
 
 private:
   SymbolReporter *const Reporter;
   const HeaderMapCollector::RegexHeaderMap *const RegexHeaderMap;
 };
 
 } // namespace find_all_symbols
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_FIND_ALL_SYMBOLS_FIND_ALL_SYMBOLS_ACTION_H

clang-tools-extra/clang-move/Move.h

 };
 
 class ClangMoveActionFactory : public tooling::FrontendActionFactory {
 public:
   ClangMoveActionFactory(ClangMoveContext *const Context,
                          DeclarationReporter *const Reporter = nullptr)
       : Context(Context), Reporter(Reporter) {}
 
-  clang::FrontendAction *create() override {
-    return new ClangMoveAction(Context, Reporter);
+  std::unique_ptr<clang::FrontendAction> create() override {
+    return std::make_unique<ClangMoveAction>(Context, Reporter);
   }
 
 private:
   // Not owned.
   ClangMoveContext *const Context;
   DeclarationReporter *const Reporter;
 };
 
 } // namespace move
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_MOVE_CLANGMOVE_H

clang-tools-extra/clang-tidy/ClangTidy.cpp

runClangTidy(clang::tidy::ClangTidyContext &Context,
   Context.setDiagnosticsEngine(&DE);
   Tool.setDiagnosticConsumer(&DiagConsumer);
 
   class ActionFactory : public FrontendActionFactory {
   public:
     ActionFactory(ClangTidyContext &Context,
                   IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> BaseFS)
         : ConsumerFactory(Context, BaseFS) {}
-    FrontendAction *create() override { return new Action(&ConsumerFactory); }
+    std::unique_ptr<FrontendAction> create() override {
+      return std::make_unique<Action>(&ConsumerFactory);
+    }
 
     bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
                        FileManager *Files,
                        std::shared_ptr<PCHContainerOperations> PCHContainerOps,
                        DiagnosticConsumer *DiagConsumer) override {
       // Explicitly set ProgramAction to RunAnalysis to make the preprocessor
       // define __clang_analyzer__ macro. The frontend analyzer action will not
       // be called here.

clang-tools-extra/clangd/indexer/IndexerMain.cpp

Format("format", llvm::cl::desc("Format of the index to be written"),
                             clEnumValN(IndexFileFormat::RIFF, "binary",
                                        "binary RIFF format")),
            llvm::cl::init(IndexFileFormat::RIFF));
 
 class IndexActionFactory : public tooling::FrontendActionFactory {
 public:
   IndexActionFactory(IndexFileIn &Result) : Result(Result) {}
 
-  clang::FrontendAction *create() override {
+  std::unique_ptr<FrontendAction> create() override {
     SymbolCollector::Options Opts;
     Opts.CountReferences = true;
     return createStaticIndexingAction(
-               Opts,
-               [&](SymbolSlab S) {
-                 // Merge as we go.
-                 std::lock_guard<std::mutex> Lock(SymbolsMu);
-                 for (const auto &Sym : S) {
-                   if (const auto *Existing = Symbols.find(Sym.ID))
-                     Symbols.insert(mergeSymbol(*Existing, Sym));
-                   else
-                     Symbols.insert(Sym);
-                 }
-               },
-               [&](RefSlab S) {
-                 std::lock_guard<std::mutex> Lock(SymbolsMu);
-                 for (const auto &Sym : S) {
-                   // Deduplication happens during insertion.
-                   for (const auto &Ref : Sym.second)
-                     Refs.insert(Sym.first, Ref);
-                 }
-               },
-               [&](RelationSlab S) {
-                 std::lock_guard<std::mutex> Lock(SymbolsMu);
-                 for (const auto &R : S) {
-                   Relations.insert(R);
-                 }
-               },
-               /*IncludeGraphCallback=*/nullptr)
-        .release();
+        /*IncludeGraphCallback=*/nullptr);
   }
 
   // Awkward: we write the result in the destructor, because the executor
   // takes ownership so it's the easiest way to get our data back out.
   ~IndexActionFactory() {
     Result.Symbols = std::move(Symbols).build();
     Result.Refs = std::move(Refs).build();
     Result.Relations = std::move(Relations).build();

clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

 }
 
 class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
 public:
   SymbolIndexActionFactory(SymbolCollector::Options COpts,
                            CommentHandler *PragmaHandler)
       : COpts(std::move(COpts)), PragmaHandler(PragmaHandler) {}
 
-  clang::FrontendAction *create() override {
+  std::unique_ptr<FrontendAction> create() override {
     class IndexAction : public ASTFrontendAction {
     public:
       IndexAction(std::shared_ptr<index::IndexDataConsumer> DataConsumer,
                   const index::IndexingOptions &Opts,
                   CommentHandler *PragmaHandler)
           : DataConsumer(std::move(DataConsumer)), Opts(Opts),
             PragmaHandler(PragmaHandler) {}
 
private:
       index::IndexingOptions Opts;
       CommentHandler *PragmaHandler;
     };
     index::IndexingOptions IndexOpts;
     IndexOpts.SystemSymbolFilter =
         index::IndexingOptions::SystemSymbolFilterKind::All;
     IndexOpts.IndexFunctionLocals = false;
     Collector = std::make_shared<SymbolCollector>(COpts);
-    return new IndexAction(Collector, std::move(IndexOpts), PragmaHandler);
+    return std::make_unique<IndexAction>(Collector, std::move(IndexOpts),
+                                         PragmaHandler);
   }
 
   std::shared_ptr<SymbolCollector> Collector;
   SymbolCollector::Options COpts;
   CommentHandler *PragmaHandler;
 };
 
 class SymbolCollectorTest : public ::testing::Test {

clang-tools-extra/modularize/CoverageChecker.cpp

 #include "ModularizeUtilities.h"
 #include "clang/AST/ASTConsumer.h"
 #include "CoverageChecker.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Driver/Options.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
private:
   CoverageChecker &Checker;
 };
 
 class CoverageCheckerFrontendActionFactory : public FrontendActionFactory {
 public:
   CoverageCheckerFrontendActionFactory(CoverageChecker &Checker)
     : Checker(Checker) {}
 
-  CoverageCheckerAction *create() override {
-    return new CoverageCheckerAction(Checker);
+  std::unique_ptr<FrontendAction> create() override {
+    return std::make_unique<CoverageCheckerAction>(Checker);
   }
 
 private:
   CoverageChecker &Checker;
 };
 
 // CoverageChecker class implementation.
 

clang-tools-extra/modularize/Modularize.cpp

 #include "ModularizeUtilities.h"
 #include "PreprocessorTracker.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Driver/Options.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"

 class ModularizeFrontendActionFactory : public FrontendActionFactory {
 public:
   ModularizeFrontendActionFactory(EntityMap &Entities,
                                   PreprocessorTracker &preprocessorTracker,
                                   int &HadErrors)
       : Entities(Entities), PPTracker(preprocessorTracker),
         HadErrors(HadErrors) {}
 
-  CollectEntitiesAction *create() override {
-    return new CollectEntitiesAction(Entities, PPTracker, HadErrors);
+  std::unique_ptr<FrontendAction> create() override {
+    return std::make_unique<CollectEntitiesAction>(Entities, PPTracker,
+                                                   HadErrors);
   }
 
 private:
   EntityMap &Entities;
   PreprocessorTracker &PPTracker;
   int &HadErrors;
 };
 
std::unique_ptr<clang::ASTConsumer>
     return std::make_unique<CompileCheckConsumer>();
   }
 };
 
 class CompileCheckFrontendActionFactory : public FrontendActionFactory {
 public:
   CompileCheckFrontendActionFactory() {}
 
-  CompileCheckAction *create() override {
-    return new CompileCheckAction();
+  std::unique_ptr<FrontendAction> create() override {
+    return std::make_unique<CompileCheckAction>();
   }
 };
 
 int main(int Argc, const char **Argv) {
 
   // Save program name for error messages.
   Argv0 = Argv[0];
 
if (DisplayFileLists) {
     // First, make a pass to just get compile errors.
     for (auto &CompileCheckFile : ModUtil->HeaderFileNames) {
       llvm::SmallVector<std::string, 32> CompileCheckFileArray;
       CompileCheckFileArray.push_back(CompileCheckFile);
       ClangTool CompileCheckTool(*Compilations, CompileCheckFileArray);
       CompileCheckTool.appendArgumentsAdjuster(
         getModularizeArgumentsAdjuster(ModUtil->Dependencies));
       int CompileCheckFileErrors = 0;
+      // FIXME: use newFrontendActionFactory.
       CompileCheckFrontendActionFactory CompileCheckFactory;
       CompileCheckFileErrors |= CompileCheckTool.run(&CompileCheckFactory);
       if (CompileCheckFileErrors != 0) {
         ModUtil->addUniqueProblemFile(CompileCheckFile);   // Save problem file.
         HadErrors |= 1;
       }
       else
         ModUtil->addNoCompileErrorsFile(CompileCheckFile); // Save good file.

clang-tools-extra/pp-trace/PPTrace.cpp

 //===----------------------------------------------------------------------===//
 
 #include "PPCallbacksTracker.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Driver/Options.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Execution.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
private:
   std::vector<CallbackCall> CallbackCalls;
 };
 
 class PPTraceFrontendActionFactory : public tooling::FrontendActionFactory {
 public:
   PPTraceFrontendActionFactory(const FilterType &Filters, raw_ostream &OS)
       : Filters(Filters), OS(OS) {}
 
-  PPTraceAction *create() override { return new PPTraceAction(Filters, OS); }
+  std::unique_ptr<FrontendAction> create() override {
+    return std::make_unique<PPTraceAction>(Filters, OS);
+  }
 
 private:
   const FilterType &Filters;
   raw_ostream &OS;
 };
 } // namespace
 } // namespace pp_trace
 } // namespace clang

clang/include/clang/Tooling/Tooling.h

public:
 
   /// Invokes the compiler with a FrontendAction created by create().
   bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
                      FileManager *Files,
                      std::shared_ptr<PCHContainerOperations> PCHContainerOps,
                      DiagnosticConsumer *DiagConsumer) override;
 
   /// Returns a new clang::FrontendAction.
-  ///
-  /// The caller takes ownership of the returned action.
-  virtual FrontendAction *create() = 0;
+  virtual std::unique_ptr<FrontendAction> create() = 0;
 };
 
 /// Returns a new FrontendActionFactory for a given type.
 ///
 /// T must derive from clang::FrontendAction.
 ///
 /// Example:
 /// FrontendActionFactory *Factory =

 ///                         clang modules.
 ///
 /// \return - True if 'ToolAction' was successfully executed.
 bool runToolOnCode(FrontendAction *ToolAction, const Twine &Code,
                    const Twine &FileName = "input.cc",
                    std::shared_ptr<PCHContainerOperations> PCHContainerOps =
                        std::make_shared<PCHContainerOperations>());
 
+inline bool
+runToolOnCode(std::unique_ptr<FrontendAction> ToolAction, const Twine &Code,
+              const Twine &FileName = "input.cc",
+              std::shared_ptr<PCHContainerOperations> PCHContainerOps =
+                  std::make_shared<PCHContainerOperations>()) {
+  return runToolOnCode(ToolAction.release(), Code, FileName, PCHContainerOps);
+}
+
 /// The first part of the pair is the filename, the second part the
 /// file-content.
 using FileContentMappings = std::vector<std::pair<std::string, std::string>>;
 
 /// Runs (and deletes) the tool on 'Code' with the -fsyntax-only flag and
 ///        with additional other flags.
 ///
 /// \param ToolAction The action to run over the code.

 bool runToolOnCodeWithArgs(
     FrontendAction *ToolAction, const Twine &Code,
     const std::vector<std::string> &Args, const Twine &FileName = "input.cc",
     const Twine &ToolName = "clang-tool",
     std::shared_ptr<PCHContainerOperations> PCHContainerOps =
         std::make_shared<PCHContainerOperations>(),
     const FileContentMappings &VirtualMappedFiles = FileContentMappings());
 
+inline bool runToolOnCodeWithArgs(
+    std::unique_ptr<FrontendAction> ToolAction, const Twine &Code,
+    const std::vector<std::string> &Args, const Twine &FileName = "input.cc",
+    const Twine &ToolName = "clang-tool",
+    std::shared_ptr<PCHContainerOperations> PCHContainerOps =
+        std::make_shared<PCHContainerOperations>(),
+    const FileContentMappings &VirtualMappedFiles = FileContentMappings()) {
+  return runToolOnCodeWithArgs(ToolAction.release(), Code, Args, FileName,
+                               ToolName, PCHContainerOps, VirtualMappedFiles);
+}
+
 // Similar to the overload except this takes a VFS.
 bool runToolOnCodeWithArgs(
     FrontendAction *ToolAction, const Twine &Code,
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
     const std::vector<std::string> &Args, const Twine &FileName = "input.cc",
     const Twine &ToolName = "clang-tool",
     std::shared_ptr<PCHContainerOperations> PCHContainerOps =
         std::make_shared<PCHContainerOperations>());
 
+inline bool runToolOnCodeWithArgs(
+    std::unique_ptr<FrontendAction> ToolAction, const Twine &Code,
+    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
+    const std::vector<std::string> &Args, const Twine &FileName = "input.cc",
+    const Twine &ToolName = "clang-tool",
+    std::shared_ptr<PCHContainerOperations> PCHContainerOps =
+        std::make_shared<PCHContainerOperations>()) {
+  return runToolOnCodeWithArgs(ToolAction.release(), Code, VFS, Args, FileName,
+                               ToolName, PCHContainerOps);
+}
+
 /// Builds an AST for 'Code'.
 ///
 /// \param Code C++ code.
 /// \param FileName The file name which 'Code' will be mapped as.
 /// \param PCHContainerOps The PCHContainerOperations for loading and creating
 /// clang modules.
 ///
 /// \return The resulting AST or null if an error occurred.
public:
   /// ownership.
   /// \param PCHContainerOps The PCHContainerOperations for loading and creating
   /// clang modules.
   ToolInvocation(std::vector<std::string> CommandLine, FrontendAction *FAction,
                  FileManager *Files,
                  std::shared_ptr<PCHContainerOperations> PCHContainerOps =
                      std::make_shared<PCHContainerOperations>());
 
+  ToolInvocation(std::vector<std::string> CommandLine,
+                 std::unique_ptr<FrontendAction> FAction, FileManager *Files,
+                 std::shared_ptr<PCHContainerOperations> PCHContainerOps =
+                     std::make_shared<PCHContainerOperations>())
+      : ToolInvocation(std::move(CommandLine), FAction.release(), Files,
+                       PCHContainerOps) {}
+
   /// Create a tool invocation.
   ///
   /// \param CommandLine The command line arguments to clang.
   /// \param Action The action to be executed.
   /// \param Files The FileManager used for the execution.
   /// \param PCHContainerOps The PCHContainerOperations for loading and creating
   /// clang modules.
   ToolInvocation(std::vector<std::string> CommandLine, ToolAction *Action,
private:
   bool RestoreCWD = true;
   bool PrintErrorMessage = true;
 };
 
 template <typename T>
 std::unique_ptr<FrontendActionFactory> newFrontendActionFactory() {
   class SimpleFrontendActionFactory : public FrontendActionFactory {
   public:
-    FrontendAction *create() override { return new T; }
+    std::unique_ptr<FrontendAction> create() override {
+      return std::make_unique<T>();
+    }
   };
 
   return std::unique_ptr<FrontendActionFactory>(
       new SimpleFrontendActionFactory);
 }
 
 template <typename FactoryT>
 inline std::unique_ptr<FrontendActionFactory> newFrontendActionFactory(
     FactoryT *ConsumerFactory, SourceFileCallbacks *Callbacks) {
   class FrontendActionFactoryAdapter : public FrontendActionFactory {
   public:
     explicit FrontendActionFactoryAdapter(FactoryT *ConsumerFactory,
                                           SourceFileCallbacks *Callbacks)
         : ConsumerFactory(ConsumerFactory), Callbacks(Callbacks) {}
 
-    FrontendAction *create() override {
-      return new ConsumerFactoryAdaptor(ConsumerFactory, Callbacks);
+    std::unique_ptr<FrontendAction> create() override {
+      return std::make_unique<ConsumerFactoryAdaptor>(ConsumerFactory,
+                                                      Callbacks);
     }
 
   private:
     class ConsumerFactoryAdaptor : public ASTFrontendAction {
     public:
       ConsumerFactoryAdaptor(FactoryT *ConsumerFactory,
                              SourceFileCallbacks *Callbacks)
           : ConsumerFactory(ConsumerFactory), Callbacks(Callbacks) {}

clang/lib/Tooling/Tooling.cpp

 }
 
 } // namespace tooling
 } // namespace clang
 
 namespace {
 
 class SingleFrontendActionFactory : public FrontendActionFactory {
-  FrontendAction *Action;
+  std::unique_ptr<FrontendAction> Action;
 
 public:
-  SingleFrontendActionFactory(FrontendAction *Action) : Action(Action) {}
+  SingleFrontendActionFactory(std::unique_ptr<FrontendAction> Action)
+      : Action(std::move(Action)) {}
 
-  FrontendAction *create() override { return Action; }
+  std::unique_ptr<FrontendAction> create() override {
+    return std::move(Action);
+  }
 };
 
 } // namespace
 
 ToolInvocation::ToolInvocation(
     std::vector<std::string> CommandLine, ToolAction *Action,
     FileManager *Files, std::shared_ptr<PCHContainerOperations> PCHContainerOps)
     : CommandLine(std::move(CommandLine)), Action(Action), OwnsAction(false),
       Files(Files), PCHContainerOps(std::move(PCHContainerOps)) {}
 
 ToolInvocation::ToolInvocation(
     std::vector<std::string> CommandLine, FrontendAction *FAction,
     FileManager *Files, std::shared_ptr<PCHContainerOperations> PCHContainerOps)
     : CommandLine(std::move(CommandLine)),
-      Action(new SingleFrontendActionFactory(FAction)), OwnsAction(true),
-      Files(Files), PCHContainerOps(std::move(PCHContainerOps)) {}
+      Action(new SingleFrontendActionFactory(
+          std::unique_ptr<FrontendAction>(FAction))),
+      OwnsAction(true), Files(Files),
+      PCHContainerOps(std::move(PCHContainerOps)) {}
 
 ToolInvocation::~ToolInvocation() {
   if (OwnsAction)
     delete Action;
 }
 
 void ToolInvocation::mapVirtualFile(StringRef FilePath, StringRef Content) {
   SmallString<1024> PathStorage;

clang/tools/clang-refactor/ClangRefactor.cpp

private:
       TUCallbackType Callback;
     };
 
     class ToolActionFactory : public FrontendActionFactory {
     public:
       ToolActionFactory(TUCallbackType Callback)
           : Callback(std::move(Callback)) {}
 
-      FrontendAction *create() override { return new ToolASTAction(Callback); }
+      std::unique_ptr<FrontendAction> create() override {
+        return std::make_unique<ToolASTAction>(Callback);
+      }
 
     private:
       TUCallbackType Callback;
     };
 
     return std::make_unique<ToolActionFactory>(
         [this](ASTContext &AST) { return callback(AST); });
   }

clang/unittests/Tooling/ExecutionTest.cpp

 
 private:
   ExecutionContext *const Context;
 };
 
 class ReportResultActionFactory : public FrontendActionFactory {
 public:
   ReportResultActionFactory(ExecutionContext *Context) : Context(Context) {}
-  FrontendAction *create() override { return new ReportResultAction(Context); }
+  std::unique_ptr<FrontendAction> create() override {
+    return std::make_unique<ReportResultAction>(Context);
+  }
 
 private:
   ExecutionContext *const Context;
 };
 
 } // namespace
 
 class TestToolExecutor : public ToolExecutor {