Merge topic 'ninja_cleanups'

054954d855 Ninja: Use clean target instead of clean tool in `cmake --target clean` calls
30a550d6ad Ninja: In cmNinjaTargetGenerator use std::unique_ptr to manage new instances
0024356f8e Ninja: In cmNinjaTargetGenerator optimize string composition
8691b3cf91 Ninja: Inline range loop range arguments
47da9859e8 Ninja: In cmGlobalNinjaGenerator use std::unique_ptr to manage output streams
9902702134 Ninja: Add and use cmGlobalNinjaGenerator::CMakeCmd method
12aa6fe07b Ninja: Fix message in cmGlobalNinjaGenerator::WriteBuild method
02293841e7 Ninja: Simplify cmGlobalNinjaGenerator::AddRule and HasRule methods
...

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !3312

5 files changed, 223 insertions, 241 deletions

diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx
index 4fa6ee6..1b973e0 100644
--- a/Source/cmGlobalNinjaGenerator.cxx
+++ b/Source/cmGlobalNinjaGenerator.cxx
@@ -136,17 +136,15 @@ void cmGlobalNinjaGenerator::WriteBuild(
 {
   // Make sure there is a rule.
   if (rule.empty()) {
-    cmSystemTools::Error("No rule for WriteBuildStatement! called "
-                         "with comment: " +
+    cmSystemTools::Error("No rule for WriteBuild! called with comment: " +
                          comment);
     return;
   }
 
   // Make sure there is at least one output file.
   if (outputs.empty()) {
-    cmSystemTools::Error("No output files for WriteBuildStatement! called "
-                         "with comment: " +
-                         comment);
+    cmSystemTools::Error(
+      "No output files for WriteBuild! called with comment: " + comment);
     return;
   }
 
@@ -292,14 +290,8 @@ void cmGlobalNinjaGenerator::WriteCustomCommandBuild(
 
 void cmGlobalNinjaGenerator::AddMacOSXContentRule()
 {
-  cmLocalGenerator* lg = this->LocalGenerators[0];
-
-  std::ostringstream cmd;
-  cmd << lg->ConvertToOutputFormat(cmSystemTools::GetCMakeCommand(),
-                                   cmOutputConverter::SHELL)
-      << " -E copy $in $out";
-
-  this->AddRule("COPY_OSX_CONTENT", cmd.str(), "Copying OS X Content $out",
+  this->AddRule("COPY_OSX_CONTENT", CMakeCmd() + " -E copy $in $out",
+                "Copying OS X Content $out",
                 "Rule for copying OS X bundle content file.",
                 /*depfile*/ "",
                 /*deptype*/ "",
@@ -332,71 +324,57 @@ void cmGlobalNinjaGenerator::WriteRule(
   const std::string& rspfile, const std::string& rspcontent,
   const std::string& restat, bool generator)
 {
+  // -- Parameter checks
   // Make sure the rule has a name.
   if (name.empty()) {
-    cmSystemTools::Error("No name given for WriteRuleStatement! called "
-                         "with comment: " +
+    cmSystemTools::Error("No name given for WriteRule! called with comment: " +
                          comment);
     return;
   }
 
   // Make sure a command is given.
   if (command.empty()) {
-    cmSystemTools::Error("No command given for WriteRuleStatement! called "
-                         "with comment: " +
-                         comment);
+    cmSystemTools::Error(
+      "No command given for WriteRule! called with comment: " + comment);
     return;
   }
 
-  cmGlobalNinjaGenerator::WriteComment(os, comment);
-
-  // Write the rule.
-  os << "rule " << name << "\n";
-
-  // Write the depfile if any.
-  if (!depfile.empty()) {
-    cmGlobalNinjaGenerator::Indent(os, 1);
-    os << "depfile = " << depfile << "\n";
-  }
-
-  // Write the deptype if any.
-  if (!deptype.empty()) {
-    cmGlobalNinjaGenerator::Indent(os, 1);
-    os << "deps = " << deptype << "\n";
+  // Make sure response file content is given
+  if (!rspfile.empty() && rspcontent.empty()) {
+    cmSystemTools::Error("rspfile but no rspfile_content given for WriteRule! "
+                         "called with comment: " +
+                         comment);
+    return;
   }
 
-  // Write the command.
-  cmGlobalNinjaGenerator::Indent(os, 1);
-  os << "command = " << command << "\n";
-
-  // Write the description if any.
-  if (!description.empty()) {
-    cmGlobalNinjaGenerator::Indent(os, 1);
-    os << "description = " << description << "\n";
-  }
+  // -- Write rule
+  // Write rule intro
+  cmGlobalNinjaGenerator::WriteComment(os, comment);
+  os << "rule " << name << '\n';
 
-  if (!rspfile.empty()) {
-    if (rspcontent.empty()) {
-      cmSystemTools::Error("No rspfile_content given!" + comment);
-      return;
+  // Write rule key/value pairs
+  auto writeKV = [&os](const char* key, std::string const& value) {
+    if (!value.empty()) {
+      cmGlobalNinjaGenerator::Indent(os, 1);
+      os << key << " = " << value << '\n';
     }
-    cmGlobalNinjaGenerator::Indent(os, 1);
-    os << "rspfile = " << rspfile << "\n";
-    cmGlobalNinjaGenerator::Indent(os, 1);
-    os << "rspfile_content = " << rspcontent << "\n";
-  }
+  };
 
-  if (!restat.empty()) {
-    cmGlobalNinjaGenerator::Indent(os, 1);
-    os << "restat = " << restat << "\n";
+  writeKV("depfile", depfile);
+  writeKV("deps", deptype);
+  writeKV("command", command);
+  writeKV("description", description);
+  if (!rspfile.empty()) {
+    writeKV("rspfile", rspfile);
+    writeKV("rspfile_content", rspcontent);
   }
-
+  writeKV("restat", restat);
   if (generator) {
-    cmGlobalNinjaGenerator::Indent(os, 1);
-    os << "generator = 1\n";
+    writeKV("generator", "1");
   }
 
-  os << "\n";
+  // Finish rule
+  os << '\n';
 }
 
 void cmGlobalNinjaGenerator::WriteVariable(std::ostream& os,
@@ -446,9 +424,6 @@ void cmGlobalNinjaGenerator::WriteDefault(std::ostream& os,
 
 cmGlobalNinjaGenerator::cmGlobalNinjaGenerator(cmake* cm)
   : cmGlobalCommonGenerator(cm)
-  , BuildFileStream(nullptr)
-  , RulesFileStream(nullptr)
-  , CompileCommandsStream(nullptr)
   , UsingGCCOnWindows(false)
   , ComputingUnknownDependencies(false)
   , PolicyCMP0058(cmPolicies::WARN)
@@ -509,8 +484,12 @@ void cmGlobalNinjaGenerator::Generate()
                                            msg.str());
     return;
   }
-  this->OpenBuildFileStream();
-  this->OpenRulesFileStream();
+  if (!this->OpenBuildFileStream()) {
+    return;
+  }
+  if (!this->OpenRulesFileStream()) {
+    return;
+  }
 
   this->TargetDependsClosures.clear();
 
@@ -700,11 +679,7 @@ cmGlobalNinjaGenerator::GenerateBuildCommand(
   makeCommand.Add(makeOptions.begin(), makeOptions.end());
   for (const auto& tname : targetNames) {
     if (!tname.empty()) {
-      if (tname == "clean") {
-        makeCommand.Add("-t", "clean");
-      } else {
-        makeCommand.Add(tname);
-      }
+      makeCommand.Add(tname);
     }
   }
   return { std::move(makeCommand) };
@@ -720,22 +695,20 @@ void cmGlobalNinjaGenerator::AddRule(
   const std::string& restat, bool generator)
 {
   // Do not add the same rule twice.
-  if (this->HasRule(name)) {
+  if (!this->Rules.insert(name).second) {
     return;
   }
-
-  this->Rules.insert(name);
+  // Store command length
+  this->RuleCmdLength[name] = static_cast<int>(command.size());
+  // Write rule
   cmGlobalNinjaGenerator::WriteRule(*this->RulesFileStream, name, command,
                                     description, comment, depfile, deptype,
                                     rspfile, rspcontent, restat, generator);
-
-  this->RuleCmdLength[name] = static_cast<int>(command.size());
 }
 
 bool cmGlobalNinjaGenerator::HasRule(const std::string& name)
 {
-  RulesSetType::const_iterator rule = this->Rules.find(name);
-  return (rule != this->Rules.end());
+  return (this->Rules.find(name) != this->Rules.end());
 }
 
 // Private virtual overrides
@@ -761,7 +734,7 @@ void cmGlobalNinjaGenerator::ComputeTargetObjectDirectory(
 
 // Private methods
 
-void cmGlobalNinjaGenerator::OpenBuildFileStream()
+bool cmGlobalNinjaGenerator::OpenBuildFileStream()
 {
   // Compute Ninja's build file path.
   std::string buildFilePath =
@@ -771,12 +744,12 @@ void cmGlobalNinjaGenerator::OpenBuildFileStream()
 
   // Get a stream where to generate things.
   if (!this->BuildFileStream) {
-    this->BuildFileStream = new cmGeneratedFileStream(
+    this->BuildFileStream = cm::make_unique<cmGeneratedFileStream>(
       buildFilePath, false, this->GetMakefileEncoding());
-    if (!this->BuildFileStream) {
+    if (!(*this->BuildFileStream)) {
       // An error message is generated by the constructor if it cannot
       // open the file.
-      return;
+      return false;
     }
   }
 
@@ -787,19 +760,20 @@ void cmGlobalNinjaGenerator::OpenBuildFileStream()
   *this->BuildFileStream
     << "# This file contains all the build statements describing the\n"
     << "# compilation DAG.\n\n";
+
+  return true;
 }
 
 void cmGlobalNinjaGenerator::CloseBuildFileStream()
 {
   if (this->BuildFileStream) {
-    delete this->BuildFileStream;
-    this->BuildFileStream = nullptr;
+    this->BuildFileStream.reset();
   } else {
     cmSystemTools::Error("Build file stream was not open.");
   }
 }
 
-void cmGlobalNinjaGenerator::OpenRulesFileStream()
+bool cmGlobalNinjaGenerator::OpenRulesFileStream()
 {
   // Compute Ninja's build file path.
   std::string rulesFilePath =
@@ -809,12 +783,12 @@ void cmGlobalNinjaGenerator::OpenRulesFileStream()
 
   // Get a stream where to generate things.
   if (!this->RulesFileStream) {
-    this->RulesFileStream = new cmGeneratedFileStream(
+    this->RulesFileStream = cm::make_unique<cmGeneratedFileStream>(
       rulesFilePath, false, this->GetMakefileEncoding());
-    if (!this->RulesFileStream) {
+    if (!(*this->RulesFileStream)) {
       // An error message is generated by the constructor if it cannot
       // open the file.
-      return;
+      return false;
     }
   }
 
@@ -829,13 +803,13 @@ void cmGlobalNinjaGenerator::OpenRulesFileStream()
     << "# It is included in the main '" << NINJA_BUILD_FILE << "'.\n\n"
     ;
   /* clang-format on */
+  return true;
 }
 
 void cmGlobalNinjaGenerator::CloseRulesFileStream()
 {
   if (this->RulesFileStream) {
-    delete this->RulesFileStream;
-    this->RulesFileStream = nullptr;
+    this->RulesFileStream.reset();
   } else {
     cmSystemTools::Error("Rules file stream was not open.");
   }
@@ -892,7 +866,8 @@ void cmGlobalNinjaGenerator::AddCXXCompileCommand(
     }
 
     // Get a stream where to generate things.
-    this->CompileCommandsStream = new cmGeneratedFileStream(buildFilePath);
+    this->CompileCommandsStream =
+      cm::make_unique<cmGeneratedFileStream>(buildFilePath);
     *this->CompileCommandsStream << "[";
   } else {
     *this->CompileCommandsStream << "," << std::endl;
@@ -920,8 +895,7 @@ void cmGlobalNinjaGenerator::CloseCompileCommandsStream()
 {
   if (this->CompileCommandsStream) {
     *this->CompileCommandsStream << "\n]";
-    delete this->CompileCommandsStream;
-    this->CompileCommandsStream = nullptr;
+    this->CompileCommandsStream.reset();
   }
 }
 
@@ -1017,8 +991,7 @@ void cmGlobalNinjaGenerator::AppendTargetDepends(
 {
   if (target->GetType() == cmStateEnums::GLOBAL_TARGET) {
     // These depend only on other CMake-provided targets, e.g. "all".
-    std::set<BT<std::string>> const& utils = target->GetUtilities();
-    for (BT<std::string> const& util : utils) {
+    for (BT<std::string> const& util : target->GetUtilities()) {
       std::string d =
         target->GetLocalGenerator()->GetCurrentBinaryDirectory() + "/" +
         util.Value;
@@ -1026,8 +999,8 @@ void cmGlobalNinjaGenerator::AppendTargetDepends(
     }
   } else {
     cmNinjaDeps outs;
-    cmTargetDependSet const& targetDeps = this->GetTargetDirectDepends(target);
-    for (cmTargetDepend const& targetDep : targetDeps) {
+    for (cmTargetDepend const& targetDep :
+         this->GetTargetDirectDepends(target)) {
       if (targetDep->GetType() == cmStateEnums::INTERFACE_LIBRARY) {
         continue;
       }
@@ -1061,10 +1034,9 @@ void cmGlobalNinjaGenerator::AppendTargetDependsClosure(
     // relevant for filling the cache entries properly isolated and a global
     // result set that is relevant for the result of the top level call to
     // AppendTargetDependsClosure.
-    auto const& targetDeps = this->GetTargetDirectDepends(target);
     cmNinjaOuts this_outs; // this will be the new cache entry
 
-    for (auto const& dep_target : targetDeps) {
+    for (auto const& dep_target : this->GetTargetDirectDepends(target)) {
       if (dep_target->GetType() == cmStateEnums::INTERFACE_LIBRARY) {
         continue;
       }
@@ -1160,9 +1132,7 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os)
 
     // The directory-level rule should depend on the directory-level
     // rules of the subdirectories.
-    std::vector<cmStateSnapshot> const& children =
-      lg->GetStateSnapshot().GetChildren();
-    for (cmStateSnapshot const& state : children) {
+    for (cmStateSnapshot const& state : lg->GetStateSnapshot().GetChildren()) {
       std::string const currentBinaryDir =
         state.GetDirectory().GetCurrentBinary();
 
@@ -1223,26 +1193,21 @@ void cmGlobalNinjaGenerator::WriteUnknownExplicitDependencies(std::ostream& os)
   for (cmLocalGenerator* lg : this->LocalGenerators) {
     // get the vector of files created by this makefile and convert them
     // to ninja paths, which are all relative in respect to the build directory
-    const std::vector<std::string>& files =
-      lg->GetMakefile()->GetOutputFiles();
-    for (std::string const& file : files) {
+    for (std::string const& file : lg->GetMakefile()->GetOutputFiles()) {
       knownDependencies.insert(this->ConvertToNinjaPath(file));
     }
     if (!this->GlobalSettingIsOn("CMAKE_SUPPRESS_REGENERATION")) {
       // get list files which are implicit dependencies as well and will be
       // phony for rebuild manifest
-      std::vector<std::string> const& lf = lg->GetMakefile()->GetListFiles();
-      for (std::string const& j : lf) {
+      for (std::string const& j : lg->GetMakefile()->GetListFiles()) {
         knownDependencies.insert(this->ConvertToNinjaPath(j));
       }
     }
-    std::vector<cmGeneratorExpressionEvaluationFile*> const& ef =
-      lg->GetMakefile()->GetEvaluationFiles();
-    for (cmGeneratorExpressionEvaluationFile* li : ef) {
+    for (cmGeneratorExpressionEvaluationFile* li :
+         lg->GetMakefile()->GetEvaluationFiles()) {
       // get all the files created by generator expressions and convert them
       // to ninja paths
-      std::vector<std::string> evaluationFiles = li->GetFiles();
-      for (std::string const& evaluationFile : evaluationFiles) {
+      for (std::string const& evaluationFile : li->GetFiles()) {
         knownDependencies.insert(this->ConvertToNinjaPath(evaluationFile));
       }
     }
@@ -1348,30 +1313,28 @@ void cmGlobalNinjaGenerator::WriteTargetRebuildManifest(std::ostream& os)
   }
   cmLocalGenerator* lg = this->LocalGenerators[0];
 
-  std::ostringstream cmd;
-  cmd << lg->ConvertToOutputFormat(cmSystemTools::GetCMakeCommand(),
-                                   cmOutputConverter::SHELL)
-      << " -S"
-      << lg->ConvertToOutputFormat(lg->GetSourceDirectory(),
-                                   cmOutputConverter::SHELL)
-      << " -B"
-      << lg->ConvertToOutputFormat(lg->GetBinaryDirectory(),
-                                   cmOutputConverter::SHELL);
-  WriteRule(*this->RulesFileStream, "RERUN_CMAKE", cmd.str(),
-            "Re-running CMake...", "Rule for re-running cmake.",
-            /*depfile=*/"",
-            /*deptype=*/"",
-            /*rspfile=*/"",
-            /*rspcontent*/ "",
-            /*restat=*/"",
-            /*generator=*/true);
+  {
+    std::string cmd = CMakeCmd();
+    cmd += " -S";
+    cmd += lg->ConvertToOutputFormat(lg->GetSourceDirectory(),
+                                     cmOutputConverter::SHELL);
+    cmd += " -B";
+    cmd += lg->ConvertToOutputFormat(lg->GetBinaryDirectory(),
+                                     cmOutputConverter::SHELL);
+    WriteRule(*this->RulesFileStream, "RERUN_CMAKE", cmd,
+              "Re-running CMake...", "Rule for re-running cmake.",
+              /*depfile=*/"",
+              /*deptype=*/"",
+              /*rspfile=*/"",
+              /*rspcontent*/ "",
+              /*restat=*/"",
+              /*generator=*/true);
+  }
 
   cmNinjaDeps implicitDeps;
   cmNinjaDeps explicitDeps;
   for (cmLocalGenerator* localGen : this->LocalGenerators) {
-    std::vector<std::string> const& lf =
-      localGen->GetMakefile()->GetListFiles();
-    for (std::string const& fi : lf) {
+    for (std::string const& fi : localGen->GetMakefile()->GetListFiles()) {
       implicitDeps.push_back(this->ConvertToNinjaPath(fi));
     }
   }
@@ -1386,22 +1349,22 @@ void cmGlobalNinjaGenerator::WriteTargetRebuildManifest(std::ostream& os)
 
   cmake* cm = this->GetCMakeInstance();
   if (this->SupportsManifestRestat() && cm->DoWriteGlobVerifyTarget()) {
-    std::ostringstream verify_cmd;
-    verify_cmd << lg->ConvertToOutputFormat(cmSystemTools::GetCMakeCommand(),
-                                            cmOutputConverter::SHELL)
-               << " -P "
-               << lg->ConvertToOutputFormat(cm->GetGlobVerifyScript(),
-                                            cmOutputConverter::SHELL);
-
-    WriteRule(*this->RulesFileStream, "VERIFY_GLOBS", verify_cmd.str(),
-              "Re-checking globbed directories...",
-              "Rule for re-checking globbed directories.",
-              /*depfile=*/"",
-              /*deptype=*/"",
-              /*rspfile=*/"",
-              /*rspcontent*/ "",
-              /*restat=*/"",
-              /*generator=*/true);
+    {
+      std::string cmd = CMakeCmd();
+      cmd += " -P ";
+      cmd += lg->ConvertToOutputFormat(cm->GetGlobVerifyScript(),
+                                       cmOutputConverter::SHELL);
+
+      WriteRule(*this->RulesFileStream, "VERIFY_GLOBS", cmd,
+                "Re-checking globbed directories...",
+                "Rule for re-checking globbed directories.",
+                /*depfile=*/"",
+                /*deptype=*/"",
+                /*rspfile=*/"",
+                /*rspcontent*/ "",
+                /*restat=*/"",
+                /*generator=*/true);
+    }
 
     std::string verifyForce = cm->GetGlobVerifyScript() + "_force";
     cmNinjaDeps verifyForceDeps(1, this->NinjaOutputPath(verifyForce));
@@ -1464,10 +1427,17 @@ void cmGlobalNinjaGenerator::WriteTargetRebuildManifest(std::ostream& os)
                         missingInputs, cmNinjaDeps());
 }
 
-std::string cmGlobalNinjaGenerator::ninjaCmd() const
+std::string cmGlobalNinjaGenerator::CMakeCmd() const
+{
+  cmLocalGenerator* lgen = this->LocalGenerators.at(0);
+  return lgen->ConvertToOutputFormat(cmSystemTools::GetCMakeCommand(),
+                                     cmOutputConverter::SHELL);
+}
+
+std::string cmGlobalNinjaGenerator::NinjaCmd() const
 {
   cmLocalGenerator* lgen = this->LocalGenerators[0];
-  if (lgen) {
+  if (lgen != nullptr) {
     return lgen->ConvertToOutputFormat(this->NinjaCommand,
                                        cmOutputConverter::SHELL);
   }
@@ -1496,7 +1466,7 @@ bool cmGlobalNinjaGenerator::SupportsMultilineDepfile() const
 
 void cmGlobalNinjaGenerator::WriteTargetClean(std::ostream& os)
 {
-  WriteRule(*this->RulesFileStream, "CLEAN", ninjaCmd() + " -t clean",
+  WriteRule(*this->RulesFileStream, "CLEAN", NinjaCmd() + " -t clean",
             "Cleaning all built files...",
             "Rule for cleaning all built files.",
             /*depfile=*/"",
@@ -1516,7 +1486,7 @@ void cmGlobalNinjaGenerator::WriteTargetClean(std::ostream& os)
 
 void cmGlobalNinjaGenerator::WriteTargetHelp(std::ostream& os)
 {
-  WriteRule(*this->RulesFileStream, "HELP", ninjaCmd() + " -t targets",
+  WriteRule(*this->RulesFileStream, "HELP", NinjaCmd() + " -t targets",
             "All primary targets available:",
             "Rule for printing all primary targets available.",
             /*depfile=*/"",
diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h
index efd1d8f..ffcea60 100644
--- a/Source/cmGlobalNinjaGenerator.h
+++ b/Source/cmGlobalNinjaGenerator.h
@@ -7,12 +7,15 @@
 
 #include <iosfwd>
 #include <map>
+#include <memory> // IWYU pragma: keep
 #include <set>
 #include <string>
 #include <unordered_map>
+#include <unordered_set>
 #include <utility>
 #include <vector>
 
+#include "cmGeneratedFileStream.h"
 #include "cmGlobalCommonGenerator.h"
 #include "cmGlobalGenerator.h"
 #include "cmGlobalGeneratorFactory.h"
@@ -21,7 +24,6 @@
 #include "cm_codecvt.hxx"
 
 class cmCustomCommand;
-class cmGeneratedFileStream;
 class cmGeneratorTarget;
 class cmLinkLineComputer;
 class cmLocalGenerator;
@@ -41,8 +43,6 @@ struct cmDocumentationEntry;
  *   it is handle by Ninja's -v option.
  * - We don't care about computing any progress status since Ninja manages
  *   it itself.
- * - We don't care about generating a clean target since Ninja already have
- *   a clean tool.
  * - We generate one build.ninja and one rules.ninja per project.
  * - We try to minimize the number of generated rules: one per target and
  *   language.
@@ -233,12 +233,12 @@ public:
 
   cmGeneratedFileStream* GetBuildFileStream() const
   {
-    return this->BuildFileStream;
+    return this->BuildFileStream.get();
   }
 
   cmGeneratedFileStream* GetRulesFileStream() const
   {
-    return this->RulesFileStream;
+    return this->RulesFileStream.get();
   }
 
   std::string const& ConvertToNinjaPath(const std::string& path) const;
@@ -378,12 +378,12 @@ private:
                       cmMakefile* mf) const override;
   bool CheckFortran(cmMakefile* mf) const;
 
-  void OpenBuildFileStream();
+  bool OpenBuildFileStream();
   void CloseBuildFileStream();
 
   void CloseCompileCommandsStream();
 
-  void OpenRulesFileStream();
+  bool OpenRulesFileStream();
   void CloseRulesFileStream();
 
   /// Write the common disclaimer text at the top of each build file.
@@ -405,25 +405,22 @@ private:
     cmGeneratorTarget const* target,
     std::set<cmGeneratorTarget const*>& depends);
 
-  std::string ninjaCmd() const;
+  std::string CMakeCmd() const;
+  std::string NinjaCmd() const;
 
   /// The file containing the build statement. (the relationship of the
   /// compilation DAG).
-  cmGeneratedFileStream* BuildFileStream;
+  std::unique_ptr<cmGeneratedFileStream> BuildFileStream;
   /// The file containing the rule statements. (The action attached to each
   /// edge of the compilation DAG).
-  cmGeneratedFileStream* RulesFileStream;
-  cmGeneratedFileStream* CompileCommandsStream;
-
-  /// The type used to store the set of rules added to the generated build
-  /// system.
-  typedef std::set<std::string> RulesSetType;
+  std::unique_ptr<cmGeneratedFileStream> RulesFileStream;
+  std::unique_ptr<cmGeneratedFileStream> CompileCommandsStream;
 
   /// The set of rules added to the generated build system.
-  RulesSetType Rules;
+  std::unordered_set<std::string> Rules;
 
   /// Length of rule command, used by rsp file evaluation
-  std::map<std::string, int> RuleCmdLength;
+  std::unordered_map<std::string, int> RuleCmdLength;
 
   /// The set of dependencies to add to the "all" target.
   cmNinjaDeps AllDependencies;
diff --git a/Source/cmLocalNinjaGenerator.cxx b/Source/cmLocalNinjaGenerator.cxx
index f4e3ed8..e6d4457 100644
--- a/Source/cmLocalNinjaGenerator.cxx
+++ b/Source/cmLocalNinjaGenerator.cxx
@@ -79,19 +79,17 @@ void cmLocalNinjaGenerator::Generate()
     }
   }
 
-  const std::vector<cmGeneratorTarget*>& targets = this->GetGeneratorTargets();
-  for (cmGeneratorTarget* target : targets) {
+  for (cmGeneratorTarget* target : this->GetGeneratorTargets()) {
     if (target->GetType() == cmStateEnums::INTERFACE_LIBRARY) {
       continue;
     }
-    cmNinjaTargetGenerator* tg = cmNinjaTargetGenerator::New(target);
+    auto tg = cmNinjaTargetGenerator::New(target);
     if (tg) {
       tg->Generate();
       // Add the target to "all" if required.
       if (!this->GetGlobalNinjaGenerator()->IsExcluded(target)) {
         this->GetGlobalNinjaGenerator()->AddDependencyToAll(target);
       }
-      delete tg;
     }
   }
 
@@ -283,8 +281,7 @@ void cmLocalNinjaGenerator::AppendTargetDepends(cmGeneratorTarget* target,
 void cmLocalNinjaGenerator::AppendCustomCommandDeps(
   cmCustomCommandGenerator const& ccg, cmNinjaDeps& ninjaDeps)
 {
-  const std::vector<std::string>& deps = ccg.GetDepends();
-  for (std::string const& i : deps) {
+  for (std::string const& i : ccg.GetDepends()) {
     std::string dep;
     if (this->GetRealDependency(i, this->GetConfigName(), dep)) {
       ninjaDeps.push_back(
diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx
index 2324839..506711a 100644
--- a/Source/cmNinjaTargetGenerator.cxx
+++ b/Source/cmNinjaTargetGenerator.cxx
@@ -9,7 +9,7 @@
 #include <iterator>
 #include <map>
 #include <memory> // IWYU pragma: keep
-#include <sstream>
+#include <ostream>
 #include <utility>
 
 #include "cmAlgorithms.h"
@@ -33,7 +33,8 @@
 #include "cmSystemTools.h"
 #include "cmake.h"
 
-cmNinjaTargetGenerator* cmNinjaTargetGenerator::New(cmGeneratorTarget* target)
+std::unique_ptr<cmNinjaTargetGenerator> cmNinjaTargetGenerator::New(
+  cmGeneratorTarget* target)
 {
   switch (target->GetType()) {
     case cmStateEnums::EXECUTABLE:
@@ -41,14 +42,14 @@ cmNinjaTargetGenerator* cmNinjaTargetGenerator::New(cmGeneratorTarget* target)
     case cmStateEnums::STATIC_LIBRARY:
     case cmStateEnums::MODULE_LIBRARY:
     case cmStateEnums::OBJECT_LIBRARY:
-      return new cmNinjaNormalTargetGenerator(target);
+      return cm::make_unique<cmNinjaNormalTargetGenerator>(target);
 
     case cmStateEnums::UTILITY:
     case cmStateEnums::GLOBAL_TARGET:
-      return new cmNinjaUtilityTargetGenerator(target);
+      return cm::make_unique<cmNinjaUtilityTargetGenerator>(target);
 
     default:
-      return nullptr;
+      return std::unique_ptr<cmNinjaTargetGenerator>();
   }
 }
 
@@ -496,12 +497,11 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang)
     launcher += " ";
   }
 
-  if (explicitPP) {
-    // Lookup the explicit preprocessing rule.
-    std::string const ppVar = "CMAKE_" + lang + "_PREPROCESS_SOURCE";
-    std::string const& ppCmd =
-      this->GetMakefile()->GetRequiredDefinition(ppVar);
+  std::string const cmakeCmd =
+    this->GetLocalGenerator()->ConvertToOutputFormat(
+      cmSystemTools::GetCMakeCommand(), cmLocalGenerator::SHELL);
 
+  if (explicitPP) {
     // Explicit preprocessing always uses a depfile.
     std::string const ppDeptype; // no deps= for multiple outputs
     std::string const ppDepfile = "$DEP_FILE";
@@ -535,8 +535,12 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang)
     std::string ppRspContent;
     if (!responseFlag.empty()) {
       ppRspFile = "$RSP_FILE";
-      ppRspContent = std::string(" ") + ppVars.Defines + " " +
-        ppVars.Includes + " " + ppFlags;
+      ppRspContent = " ";
+      ppRspContent += ppVars.Defines;
+      ppRspContent += " ";
+      ppRspContent += ppVars.Includes;
+      ppRspContent += " ";
+      ppRspContent += ppFlags;
       ppFlags = responseFlag + ppRspFile;
       ppVars.Defines = "";
       ppVars.Includes = "";
@@ -546,7 +550,13 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang)
 
     // Rule for preprocessing source file.
     std::vector<std::string> ppCmds;
-    cmSystemTools::ExpandListArgument(ppCmd, ppCmds);
+    {
+      // Lookup the explicit preprocessing rule.
+      std::string ppVar = "CMAKE_" + lang;
+      ppVar += "_PREPROCESS_SOURCE";
+      cmSystemTools::ExpandListArgument(
+        this->GetMakefile()->GetRequiredDefinition(ppVar), ppCmds);
+    }
 
     for (std::string& i : ppCmds) {
       i = launcher + i;
@@ -555,67 +565,73 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang)
     }
 
     // Run CMake dependency scanner on preprocessed output.
-    std::string const cmake = this->GetLocalGenerator()->ConvertToOutputFormat(
-      cmSystemTools::GetCMakeCommand(), cmLocalGenerator::SHELL);
-    ppCmds.push_back(
-      cmake +
-      " -E cmake_ninja_depends"
-      " --tdi=" +
-      tdi + " --lang=" + lang +
-      " --pp=$out"
-      " --dep=$DEP_FILE" +
-      (needDyndep ? " --obj=$OBJ_FILE --ddi=$DYNDEP_INTERMEDIATE_FILE" : ""));
-
+    {
+      std::string ccmd = cmakeCmd;
+      ccmd += " -E cmake_ninja_depends --tdi=";
+      ccmd += tdi;
+      ccmd += " --lang=";
+      ccmd += lang;
+      ccmd += " --pp=$out --dep=$DEP_FILE";
+      if (needDyndep) {
+        ccmd += " --obj=$OBJ_FILE --ddi=$DYNDEP_INTERMEDIATE_FILE";
+      }
+      ppCmds.emplace_back(std::move(ccmd));
+    }
     std::string const ppCmdLine =
       this->GetLocalGenerator()->BuildCommandLine(ppCmds);
 
     // Write the rule for preprocessing file of the given language.
-    std::ostringstream ppComment;
-    ppComment << "Rule for preprocessing " << lang << " files.";
-    std::ostringstream ppDesc;
-    ppDesc << "Building " << lang << " preprocessed $out";
+    std::string ppComment = "Rule for preprocessing ";
+    ppComment += lang;
+    ppComment += " files.";
+    std::string ppDesc = "Building ";
+    ppDesc += lang;
+    ppDesc += " preprocessed $out";
     this->GetGlobalGenerator()->AddRule(
-      this->LanguagePreprocessRule(lang), ppCmdLine, ppDesc.str(),
-      ppComment.str(), ppDepfile, ppDeptype, ppRspFile, ppRspContent,
+      this->LanguagePreprocessRule(lang), ppCmdLine, ppDesc, ppComment,
+      ppDepfile, ppDeptype, ppRspFile, ppRspContent,
       /*restat*/ "",
       /*generator*/ false);
   }
 
   if (needDyndep) {
     // Write the rule for ninja dyndep file generation.
-    std::vector<std::string> ddCmds;
 
     // Command line length is almost always limited -> use response file for
     // dyndep rules
     std::string ddRspFile = "$out.rsp";
     std::string ddRspContent = "$in";
-    std::string ddInput = "@" + ddRspFile;
+    std::string ddCmdLine;
 
     // Run CMake dependency scanner on the source file (using the preprocessed
     // source if that was performed).
-    std::string const cmake = this->GetLocalGenerator()->ConvertToOutputFormat(
-      cmSystemTools::GetCMakeCommand(), cmLocalGenerator::SHELL);
-    ddCmds.push_back(cmake +
-                     " -E cmake_ninja_dyndep"
-                     " --tdi=" +
-                     tdi + " --lang=" + lang +
-                     " --dd=$out"
-                     " " +
-                     ddInput);
-
-    std::string const ddCmdLine =
-      this->GetLocalGenerator()->BuildCommandLine(ddCmds);
-
-    std::ostringstream ddComment;
-    ddComment << "Rule to generate ninja dyndep files for " << lang << ".";
-    std::ostringstream ddDesc;
-    ddDesc << "Generating " << lang << " dyndep file $out";
-    this->GetGlobalGenerator()->AddRule(
-      this->LanguageDyndepRule(lang), ddCmdLine, ddDesc.str(), ddComment.str(),
-      /*depfile*/ "",
-      /*deps*/ "", ddRspFile, ddRspContent,
-      /*restat*/ "",
-      /*generator*/ false);
+    {
+      std::vector<std::string> ddCmds;
+      {
+        std::string ccmd = cmakeCmd;
+        ccmd += " -E cmake_ninja_dyndep --tdi=";
+        ccmd += tdi;
+        ccmd += " --lang=";
+        ccmd += lang;
+        ccmd += " --dd=$out ";
+        ccmd += "@";
+        ccmd += ddRspFile;
+        ddCmds.emplace_back(std::move(ccmd));
+      }
+      ddCmdLine = this->GetLocalGenerator()->BuildCommandLine(ddCmds);
+    }
+    std::string ddComment = "Rule to generate ninja dyndep files for ";
+    ddComment += lang;
+    ddComment += ".";
+    std::string ddDesc = "Generating ";
+    ddDesc += lang;
+    ddDesc += " dyndep file $out";
+    this->GetGlobalGenerator()->AddRule(this->LanguageDyndepRule(lang),
+                                        ddCmdLine, ddDesc, ddComment,
+                                        /*depfile*/ "",
+                                        /*deps*/ "", ddRspFile, ddRspContent,
+                                        /*restat*/ "",
+                                        /*generator*/ false);
   }
 
   // If using a response file, move defines, includes, and flags into it.
@@ -716,8 +732,7 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang)
     const char* cppcheck = this->GeneratorTarget->GetProperty(cppcheck_prop);
     if ((iwyu && *iwyu) || (tidy && *tidy) || (cpplint && *cpplint) ||
         (cppcheck && *cppcheck)) {
-      std::string run_iwyu = this->GetLocalGenerator()->ConvertToOutputFormat(
-        cmSystemTools::GetCMakeCommand(), cmOutputConverter::SHELL);
+      std::string run_iwyu = cmakeCmd;
       run_iwyu += " -E __run_co_compile";
       if (!compilerLauncher.empty()) {
         // In __run_co_compile case the launcher command is supplied
@@ -776,15 +791,17 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang)
     this->GetLocalGenerator()->BuildCommandLine(compileCmds);
 
   // Write the rule for compiling file of the given language.
-  std::ostringstream comment;
-  comment << "Rule for compiling " << lang << " files.";
-  std::ostringstream description;
-  description << "Building " << lang << " object $out";
-  this->GetGlobalGenerator()->AddRule(
-    this->LanguageCompilerRule(lang), cmdLine, description.str(),
-    comment.str(), depfile, deptype, rspfile, rspcontent,
-    /*restat*/ "",
-    /*generator*/ false);
+  std::string comment = "Rule for compiling ";
+  comment += lang;
+  comment += " files.";
+  std::string description = "Building ";
+  description += lang;
+  description += " object $out";
+  this->GetGlobalGenerator()->AddRule(this->LanguageCompilerRule(lang),
+                                      cmdLine, description, comment, depfile,
+                                      deptype, rspfile, rspcontent,
+                                      /*restat*/ "",
+                                      /*generator*/ false);
 }
 
 void cmNinjaTargetGenerator::WriteObjectBuildStatements()
@@ -1212,8 +1229,7 @@ void cmNinjaTargetGenerator::WriteTargetDependInfo(std::string const& lang)
 
   Json::Value& tdi_linked_target_dirs = tdi["linked-target-dirs"] =
     Json::arrayValue;
-  std::vector<std::string> linked = this->GetLinkedTargetDirectories();
-  for (std::string const& l : linked) {
+  for (std::string const& l : this->GetLinkedTargetDirectories()) {
     tdi_linked_target_dirs.append(l);
   }
 
diff --git a/Source/cmNinjaTargetGenerator.h b/Source/cmNinjaTargetGenerator.h
index 51c9ac7..3dbc1b5 100644
--- a/Source/cmNinjaTargetGenerator.h
+++ b/Source/cmNinjaTargetGenerator.h
@@ -11,6 +11,7 @@
 #include "cmOSXBundleGenerator.h"
 
 #include <map>
+#include <memory> // IWYU pragma: keep
 #include <set>
 #include <string>
 #include <vector>
@@ -26,7 +27,8 @@ class cmNinjaTargetGenerator : public cmCommonTargetGenerator
 {
 public:
   /// Create a cmNinjaTargetGenerator according to the @a target's type.
-  static cmNinjaTargetGenerator* New(cmGeneratorTarget* target);
+  static std::unique_ptr<cmNinjaTargetGenerator> New(
+    cmGeneratorTarget* target);
 
   /// Build a NinjaTargetGenerator.
   cmNinjaTargetGenerator(cmGeneratorTarget* target);