Merge topic 'smart_ptr/cmExportSet'

71f088f53a cmExportSet: subsume cmExportSetMap source files
6511fa6f33 cmExportSet: default destructor
9b8a1f7c28 cmExportSetMap: improve ownership of cmExportSet

Acked-by: Kitware Robot <[email protected]>
Merge-request: !3816

Source/CMakeLists.txt

@@ -226,8 +226,6 @@ set(SRCS
   cmExportTryCompileFileGenerator.cxx
   cmExportSet.h
   cmExportSet.cxx
-  cmExportSetMap.h
-  cmExportSetMap.cxx
   cmExternalMakefileProjectGenerator.cxx
   cmExternalMakefileProjectGenerator.h
   cmExtraCodeBlocksGenerator.cxx

Source/cmExportBuildFileGenerator.cxx

@@ -17,6 +17,7 @@
 #include "cmake.h"
 
 #include <map>
+#include <memory>
 #include <set>
 #include <sstream>
 #include <utility>
@@ -283,7 +284,8 @@ void cmExportBuildFileGenerator::GetTargets(
   std::vector<std::string>& targets) const
 {
   if (this->ExportSet) {
-    for (cmTargetExport* te : *this->ExportSet->GetTargetExports()) {
+    for (std::unique_ptr<cmTargetExport> const& te :
+         this->ExportSet->GetTargetExports()) {
       targets.push_back(te->TargetName);
     }
     return;

Source/cmExportCommand.cxx

@@ -13,7 +13,7 @@
 #include "cmArgumentParser.h"
 #include "cmExportBuildAndroidMKGenerator.h"
 #include "cmExportBuildFileGenerator.h"
-#include "cmExportSetMap.h"
+#include "cmExportSet.h"
 #include "cmGeneratedFileStream.h"
 #include "cmGlobalGenerator.h"
 #include "cmMakefile.h"
@@ -23,7 +23,6 @@
 #include "cmSystemTools.h"
 #include "cmTarget.h"
 
-class cmExportSet;
 class cmExecutionStatus;
 
 #if defined(__HAIKU__)
@@ -121,7 +120,7 @@ bool cmExportCommand::InitialPass(std::vector<std::string> const& args,
 
   cmGlobalGenerator* gg = this->Makefile->GetGlobalGenerator();
 
-  cmExportSet* ExportSet = nullptr;
+  cmExportSet* exportSet = nullptr;
   if (args[0] == "EXPORT") {
     cmExportSetMap& setMap = gg->GetExportSets();
     auto const it = setMap.find(arguments.ExportSetName);
@@ -131,7 +130,7 @@ bool cmExportCommand::InitialPass(std::vector<std::string> const& args,
       this->SetError(e.str());
       return false;
     }
-    ExportSet = it->second;
+    exportSet = &it->second;
   } else if (!arguments.Targets.empty() ||
              cmContains(keywordsMissingValue, "TARGETS")) {
     for (std::string const& currentTarget : arguments.Targets) {
@@ -180,8 +179,8 @@ bool cmExportCommand::InitialPass(std::vector<std::string> const& args,
   ebfg->SetExportFile(fname.c_str());
   ebfg->SetNamespace(arguments.Namespace);
   ebfg->SetAppendMode(arguments.Append);
-  if (ExportSet != nullptr) {
-    ebfg->SetExportSet(ExportSet);
+  if (exportSet != nullptr) {
+    ebfg->SetExportSet(exportSet);
   } else {
     ebfg->SetTargets(targets);
   }
@@ -197,7 +196,7 @@ bool cmExportCommand::InitialPass(std::vector<std::string> const& args,
   for (std::string const& ct : configurationTypes) {
     ebfg->AddConfiguration(ct);
   }
-  if (ExportSet != nullptr) {
+  if (exportSet != nullptr) {
     gg->AddBuildExportExportSet(ebfg);
   } else {
     gg->AddBuildExportSet(ebfg);

Source/cmExportInstallAndroidMKGenerator.cxx

@@ -3,6 +3,7 @@
 #include "cmExportInstallAndroidMKGenerator.h"
 
 #include <cstddef>
+#include <memory>
 #include <ostream>
 
 #include "cmExportBuildAndroidMKGenerator.h"
@@ -35,7 +36,8 @@ void cmExportInstallAndroidMKGenerator::GenerateImportHeaderCode(
   }
   os << "_IMPORT_PREFIX := "
      << "$(LOCAL_PATH)" << path << "\n\n";
-  for (cmTargetExport* te : *this->IEGen->GetExportSet()->GetTargetExports()) {
+  for (std::unique_ptr<cmTargetExport> const& te :
+       this->IEGen->GetExportSet()->GetTargetExports()) {
     // Collect import properties for this target.
     if (te->Target->GetType() == cmStateEnums::INTERFACE_LIBRARY) {
       continue;

Source/cmExportInstallFileGenerator.cxx

@@ -3,7 +3,6 @@
 #include "cmExportInstallFileGenerator.h"
 
 #include "cmExportSet.h"
-#include "cmExportSetMap.h"
 #include "cmGeneratedFileStream.h"
 #include "cmGeneratorExpression.h"
 #include "cmGeneratorTarget.h"
@@ -19,6 +18,7 @@
 #include "cmTarget.h"
 #include "cmTargetExport.h"
 
+#include <memory>
 #include <sstream>
 #include <utility>
 
@@ -40,12 +40,12 @@ bool cmExportInstallFileGenerator::GenerateMainFile(std::ostream& os)
   {
     std::string expectedTargets;
     std::string sep;
-    for (cmTargetExport* te :
-         *this->IEGen->GetExportSet()->GetTargetExports()) {
+    for (std::unique_ptr<cmTargetExport> const& te :
+         this->IEGen->GetExportSet()->GetTargetExports()) {
       expectedTargets += sep + this->Namespace + te->Target->GetExportName();
       sep = " ";
       if (this->ExportedTargets.insert(te->Target).second) {
-        allTargets.push_back(te);
+        allTargets.push_back(te.get());
       } else {
         std::ostringstream e;
         e << "install(EXPORT \"" << this->IEGen->GetExportSet()->GetName()
@@ -314,9 +314,11 @@ void cmExportInstallFileGenerator::GenerateImportTargetsConfig(
   std::vector<std::string>& missingTargets)
 {
   // Add each target in the set to the export.
-  for (cmTargetExport* te : *this->IEGen->GetExportSet()->GetTargetExports()) {
+  for (std::unique_ptr<cmTargetExport> const& te :
+       this->IEGen->GetExportSet()->GetTargetExports()) {
     // Collect import properties for this target.
-    if (this->GetExportTargetType(te) == cmStateEnums::INTERFACE_LIBRARY) {
+    if (this->GetExportTargetType(te.get()) ==
+        cmStateEnums::INTERFACE_LIBRARY) {
       continue;
     }
 
@@ -474,12 +476,10 @@ cmExportInstallFileGenerator::FindNamespaces(cmGlobalGenerator* gg,
   const cmExportSetMap& exportSets = gg->GetExportSets();
 
   for (auto const& expIt : exportSets) {
-    const cmExportSet* exportSet = expIt.second;
-    std::vector<cmTargetExport*> const* targets =
-      exportSet->GetTargetExports();
+    const cmExportSet& exportSet = expIt.second;
 
     bool containsTarget = false;
-    for (cmTargetExport* target : *targets) {
+    for (auto const& target : exportSet.GetTargetExports()) {
       if (name == target->TargetName) {
         containsTarget = true;
         break;
@@ -488,7 +488,7 @@ cmExportInstallFileGenerator::FindNamespaces(cmGlobalGenerator* gg,
 
     if (containsTarget) {
       std::vector<cmInstallExportGenerator const*> const* installs =
-        exportSet->GetInstallations();
+        exportSet.GetInstallations();
       for (cmInstallExportGenerator const* install : *installs) {
         exportFiles.push_back(install->GetDestinationFile());
         ns = install->GetNamespace();

Source/cmExportSet.cxx

@@ -2,28 +2,43 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmExportSet.h"
 
-#include "cmAlgorithms.h"
+#include <tuple>
+#include <utility>
+
 #include "cmLocalGenerator.h"
 #include "cmTargetExport.h"
 
-cmExportSet::~cmExportSet()
+cmExportSet::cmExportSet(std::string name)
+  : Name(std::move(name))
 {
-  cmDeleteAll(this->TargetExports);
 }
 
+cmExportSet::~cmExportSet() = default;
+
 void cmExportSet::Compute(cmLocalGenerator* lg)
 {
-  for (cmTargetExport* tgtExport : this->TargetExports) {
+  for (std::unique_ptr<cmTargetExport>& tgtExport : this->TargetExports) {
     tgtExport->Target = lg->FindGeneratorTargetToUse(tgtExport->TargetName);
   }
 }
 
-void cmExportSet::AddTargetExport(cmTargetExport* te)
+void cmExportSet::AddTargetExport(std::unique_ptr<cmTargetExport> te)
 {
-  this->TargetExports.push_back(te);
+  this->TargetExports.emplace_back(std::move(te));
 }
 
 void cmExportSet::AddInstallation(cmInstallExportGenerator const* installation)
 {
   this->Installations.push_back(installation);
 }
+
+cmExportSet& cmExportSetMap::operator[](const std::string& name)
+{
+  auto it = this->find(name);
+  if (it == this->end()) // Export set not found
+  {
+    auto tup_name = std::make_tuple(name);
+    it = this->emplace(std::piecewise_construct, tup_name, tup_name).first;
+  }
+  return it->second;
+}

Source/cmExportSet.h

@@ -5,8 +5,9 @@
 
 #include "cmConfigure.h" // IWYU pragma: keep
 
+#include <map>
+#include <memory>
 #include <string>
-#include <utility>
 #include <vector>
 
 class cmInstallExportGenerator;
@@ -18,10 +19,7 @@ class cmExportSet
 {
 public:
   /// Construct an empty export set named \a name
-  cmExportSet(std::string name)
-    : Name(std::move(name))
-  {
-  }
+  cmExportSet(std::string name);
   /// Destructor
   ~cmExportSet();
 
@@ -30,15 +28,15 @@ public:
 
   void Compute(cmLocalGenerator* lg);
 
-  void AddTargetExport(cmTargetExport* tgt);
+  void AddTargetExport(std::unique_ptr<cmTargetExport> tgt);
 
   void AddInstallation(cmInstallExportGenerator const* installation);
 
   std::string const& GetName() const { return this->Name; }
 
-  std::vector<cmTargetExport*> const* GetTargetExports() const
+  std::vector<std::unique_ptr<cmTargetExport>> const& GetTargetExports() const
   {
-    return &this->TargetExports;
+    return this->TargetExports;
   }
 
   std::vector<cmInstallExportGenerator const*> const* GetInstallations() const
@@ -47,9 +45,21 @@ public:
   }
 
 private:
-  std::vector<cmTargetExport*> TargetExports;
+  std::vector<std::unique_ptr<cmTargetExport>> TargetExports;
   std::string Name;
   std::vector<cmInstallExportGenerator const*> Installations;
 };
 
+/// A name -> cmExportSet map with overloaded operator[].
+class cmExportSetMap : public std::map<std::string, cmExportSet>
+{
+public:
+  /** \brief Overloaded operator[].
+   *
+   * The operator is overloaded because cmExportSet has no default constructor:
+   * we do not want unnamed export sets.
+   */
+  cmExportSet& operator[](const std::string& name);
+};
+
 #endif

Source/cmExportSetMap.cxx

@@ -1,31 +0,0 @@
-/* Distributed under the OSI-approved BSD 3-Clause License.  See accompanying
-   file Copyright.txt or https://cmake.org/licensing for details.  */
-#include "cmExportSetMap.h"
-
-#include "cmAlgorithms.h"
-#include "cmExportSet.h"
-
-#include <utility>
-
-cmExportSet* cmExportSetMap::operator[](const std::string& name)
-{
-  auto it = this->find(name);
-  if (it == this->end()) // Export set not found
-  {
-    it = this->insert(std::make_pair(name, new cmExportSet(name))).first;
-  }
-  return it->second;
-}
-
-void cmExportSetMap::clear()
-{
-  cmDeleteAll(*this);
-  this->derived::clear();
-}
-
-cmExportSetMap::cmExportSetMap() = default;
-
-cmExportSetMap::~cmExportSetMap()
-{
-  this->clear();
-}

Source/cmExportSetMap.h

@@ -1,37 +0,0 @@
-/* Distributed under the OSI-approved BSD 3-Clause License.  See accompanying
-   file Copyright.txt or https://cmake.org/licensing for details.  */
-#ifndef cmExportSetMap_h
-#define cmExportSetMap_h
-
-#include "cmConfigure.h" // IWYU pragma: keep
-
-#include <map>
-#include <string>
-
-class cmExportSet;
-
-/// A name -> cmExportSet map with overloaded operator[].
-class cmExportSetMap : public std::map<std::string, cmExportSet*>
-{
-  using derived = std::map<std::string, cmExportSet*>;
-
-public:
-  /** \brief Overloaded operator[].
-   *
-   * The operator is overloaded because cmExportSet has no default constructor:
-   * we do not want unnamed export sets.
-   */
-  cmExportSet* operator[](const std::string& name);
-
-  void clear();
-
-  cmExportSetMap();
-
-  /// Overloaded destructor deletes all member export sets.
-  ~cmExportSetMap();
-
-  cmExportSetMap(const cmExportSetMap&) = delete;
-  cmExportSetMap& operator=(const cmExportSetMap&) = delete;
-};
-
-#endif

Source/cmGlobalGenerator.h

@@ -17,7 +17,7 @@
 #include "cmAlgorithms.h"
 #include "cmCustomCommandLines.h"
 #include "cmDuration.h"
-#include "cmExportSetMap.h"
+#include "cmExportSet.h"
 #include "cmStateSnapshot.h"
 #include "cmStringAlgorithms.h"
 #include "cmSystemTools.h"

Source/cmInstallCommand.cxx

@@ -11,7 +11,6 @@
 
 #include "cmArgumentParser.h"
 #include "cmExportSet.h"
-#include "cmExportSetMap.h"
 #include "cmGeneratorExpression.h"
 #include "cmGlobalGenerator.h"
 #include "cmInstallCommandArguments.h"
@@ -736,7 +735,7 @@ bool cmInstallCommand::HandleTargetsMode(std::vector<std::string> const& args)
     // Add this install rule to an export if one was specified and
     // this is not a namelink-only rule.
     if (!exports.empty() && !namelinkOnly) {
-      cmTargetExport* te = new cmTargetExport;
+      auto te = cm::make_unique<cmTargetExport>();
       te->TargetName = target.GetName();
       te->ArchiveGenerator = archiveGenerator;
       te->BundleGenerator = bundleGenerator;
@@ -745,12 +744,12 @@ bool cmInstallCommand::HandleTargetsMode(std::vector<std::string> const& args)
       te->LibraryGenerator = libraryGenerator;
       te->RuntimeGenerator = runtimeGenerator;
       te->ObjectsGenerator = objectGenerator;
-      this->Makefile->GetGlobalGenerator()
-        ->GetExportSets()[exports]
-        ->AddTargetExport(te);
-
       te->InterfaceIncludeDirectories =
         cmJoin(includesArgs.GetIncludeDirs(), ";");
+
+      this->Makefile->GetGlobalGenerator()
+        ->GetExportSets()[exports]
+        .AddTargetExport(std::move(te));
     }
   }
 
@@ -1273,7 +1272,7 @@ bool cmInstallCommand::HandleExportAndroidMKMode(
     fname = "Android.mk";
   }
 
-  cmExportSet* exportSet =
+  cmExportSet& exportSet =
     this->Makefile->GetGlobalGenerator()->GetExportSets()[exp];
 
   cmInstallGenerator::MessageLevel message =
@@ -1281,7 +1280,7 @@ bool cmInstallCommand::HandleExportAndroidMKMode(
 
   // Create the export install generator.
   cmInstallExportGenerator* exportGenerator = new cmInstallExportGenerator(
-    exportSet, ica.GetDestination().c_str(), ica.GetPermissions().c_str(),
+    &exportSet, ica.GetDestination().c_str(), ica.GetPermissions().c_str(),
     ica.GetConfigurations(), ica.GetComponent().c_str(), message,
     ica.GetExcludeFromAll(), fname.c_str(), name_space.c_str(), exportOld,
     true);
@@ -1367,10 +1366,10 @@ bool cmInstallCommand::HandleExportMode(std::vector<std::string> const& args)
     }
   }
 
-  cmExportSet* exportSet =
+  cmExportSet& exportSet =
     this->Makefile->GetGlobalGenerator()->GetExportSets()[exp];
   if (exportOld) {
-    for (cmTargetExport* te : *exportSet->GetTargetExports()) {
+    for (auto const& te : exportSet.GetTargetExports()) {
       cmTarget* tgt =
         this->Makefile->GetGlobalGenerator()->FindTarget(te->TargetName);
       const bool newCMP0022Behavior =
@@ -1392,7 +1391,7 @@ bool cmInstallCommand::HandleExportMode(std::vector<std::string> const& args)
 
   // Create the export install generator.
   cmInstallExportGenerator* exportGenerator = new cmInstallExportGenerator(
-    exportSet, ica.GetDestination().c_str(), ica.GetPermissions().c_str(),
+    &exportSet, ica.GetDestination().c_str(), ica.GetPermissions().c_str(),
     ica.GetConfigurations(), ica.GetComponent().c_str(), message,
     ica.GetExcludeFromAll(), fname.c_str(), name_space.c_str(), exportOld,
     false);

Source/cmInstallExportGenerator.cxx

@@ -123,7 +123,7 @@ size_t cmInstallExportGenerator::GetMaxConfigLength() const
 void cmInstallExportGenerator::GenerateScript(std::ostream& os)
 {
   // Skip empty sets.
-  if (ExportSet->GetTargetExports()->empty()) {
+  if (ExportSet->GetTargetExports().empty()) {
     std::ostringstream e;
     e << "INSTALL(EXPORT) given unknown export \"" << ExportSet->GetName()
       << "\"";

bootstrap

@@ -307,7 +307,6 @@ CMAKE_CXX_SOURCES="\
   cmExportFileGenerator \
   cmExportInstallFileGenerator \
   cmExportSet \
-  cmExportSetMap \
   cmExportTryCompileFileGenerator \
   cmExprParserHelper \
   cmExternalMakefileProjectGenerator \