Merge topic 'modernize_std_unique_ptr'

3bed969dac cmMakefile: Modernize AddFunctionBlocker method to accept a std::unique_ptr
faacb90a13 cmELF: Modernize to use std::unique_ptr instead of new/delete

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

Source/CTest/cmCTestScriptHandler.cxx

@@ -4,13 +4,6 @@
 
 #include "cmsys/Directory.hxx"
 #include "cmsys/Process.h"
-#include <map>
-#include <ratio>
-#include <sstream>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <utility>
 
 #include "cm_memory.hxx"
 
@@ -41,6 +34,15 @@
 #include "cmSystemTools.h"
 #include "cmake.h"
 
+#include <map>
+#include <memory>
+#include <ratio>
+#include <sstream>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <utility>
+
 #ifdef _WIN32
 #  include <windows.h>
 #else
@@ -372,9 +374,11 @@ int cmCTestScriptHandler::ReadInScript(const std::string& total_script_arg)
 #endif
 
   // always add a function blocker to update the elapsed time
-  cmCTestScriptFunctionBlocker* f = new cmCTestScriptFunctionBlocker();
-  f->CTestScriptHandler = this;
-  this->Makefile->AddFunctionBlocker(f);
+  {
+    auto fb = cm::make_unique<cmCTestScriptFunctionBlocker>();
+    fb->CTestScriptHandler = this;
+    this->Makefile->AddFunctionBlocker(std::move(fb));
+  }
 
   /* Execute CTestScriptMode.cmake, which loads CMakeDetermineSystem and
   CMakeSystemSpecificInformation, so

Source/cmELF.cxx

@@ -4,6 +4,7 @@
 
 #include "cmAlgorithms.h"
 #include "cm_kwiml.h"
+#include "cm_memory.hxx"
 #include "cmsys/FStream.hxx"
 #include <map>
 #include <memory>
@@ -109,10 +110,10 @@ public:
   };
 
   // Construct and take ownership of the file stream object.
-  cmELFInternal(cmELF* external, std::unique_ptr<cmsys::ifstream>& fin,
+  cmELFInternal(cmELF* external, std::unique_ptr<std::istream> fin,
                 ByteOrderType order)
     : External(external)
-    , Stream(*fin.release())
+    , Stream(std::move(fin))
     , ByteOrder(order)
     , ELFType(cmELF::FileTypeInvalid)
   {
@@ -132,7 +133,7 @@ public:
   }
 
   // Destruct and delete the file stream object.
-  virtual ~cmELFInternal() { delete &this->Stream; }
+  virtual ~cmELFInternal() = default;
 
   // Forward to the per-class implementation.
   virtual unsigned int GetNumberOfSections() const = 0;
@@ -171,7 +172,7 @@ protected:
   cmELF* External;
 
   // The stream from which to read.
-  std::istream& Stream;
+  std::unique_ptr<std::istream> Stream;
 
   // The byte order of the ELF file.
   ByteOrderType ByteOrder;
@@ -233,7 +234,7 @@ public:
   typedef typename Types::tagtype tagtype;
 
   // Construct with a stream and byte swap indicator.
-  cmELFInternalImpl(cmELF* external, std::unique_ptr<cmsys::ifstream>& fin,
+  cmELFInternalImpl(cmELF* external, std::unique_ptr<std::istream> fin,
                     ByteOrderType order);
 
   // Return the number of sections as specified by the ELF header.
@@ -352,7 +353,7 @@ private:
   bool Read(ELF_Ehdr& x)
   {
     // Read the header from the file.
-    if (!this->Stream.read(reinterpret_cast<char*>(&x), sizeof(x))) {
+    if (!this->Stream->read(reinterpret_cast<char*>(&x), sizeof(x))) {
       return false;
     }
 
@@ -382,26 +383,26 @@ private:
   }
   bool Read(ELF_Shdr& x)
   {
-    if (this->Stream.read(reinterpret_cast<char*>(&x), sizeof(x)) &&
+    if (this->Stream->read(reinterpret_cast<char*>(&x), sizeof(x)) &&
         this->NeedSwap) {
       ByteSwap(x);
     }
-    return !this->Stream.fail();
+    return !this->Stream->fail();
   }
   bool Read(ELF_Dyn& x)
   {
-    if (this->Stream.read(reinterpret_cast<char*>(&x), sizeof(x)) &&
+    if (this->Stream->read(reinterpret_cast<char*>(&x), sizeof(x)) &&
         this->NeedSwap) {
       ByteSwap(x);
     }
-    return !this->Stream.fail();
+    return !this->Stream->fail();
   }
 
   bool LoadSectionHeader(ELF_Half i)
   {
     // Read the section header from the file.
-    this->Stream.seekg(this->ELFHeader.e_shoff +
-                       this->ELFHeader.e_shentsize * i);
+    this->Stream->seekg(this->ELFHeader.e_shoff +
+                        this->ELFHeader.e_shentsize * i);
     if (!this->Read(this->SectionHeaders[i])) {
       return false;
     }
@@ -426,9 +427,10 @@ private:
 };
 
 template <class Types>
-cmELFInternalImpl<Types>::cmELFInternalImpl(
-  cmELF* external, std::unique_ptr<cmsys::ifstream>& fin, ByteOrderType order)
-  : cmELFInternal(external, fin, order)
+cmELFInternalImpl<Types>::cmELFInternalImpl(cmELF* external,
+                                            std::unique_ptr<std::istream> fin,
+                                            ByteOrderType order)
+  : cmELFInternal(external, std::move(fin), order)
 {
   // Read the main header.
   if (!this->Read(this->ELFHeader)) {
@@ -510,7 +512,7 @@ bool cmELFInternalImpl<Types>::LoadDynamicSection()
   // Read each entry.
   for (int j = 0; j < n; ++j) {
     // Seek to the beginning of the section entry.
-    this->Stream.seekg(sec.sh_offset + sec.sh_entsize * j);
+    this->Stream->seekg(sec.sh_offset + sec.sh_entsize * j);
     ELF_Dyn& dyn = this->DynamicSectionEntries[j];
 
     // Try reading the entry.
@@ -630,7 +632,7 @@ cmELF::StringEntry const* cmELFInternalImpl<Types>::GetDynamicSectionString(
       unsigned long first = static_cast<unsigned long>(dyn.d_un.d_val);
       unsigned long last = first;
       unsigned long end = static_cast<unsigned long>(strtab.sh_size);
-      this->Stream.seekg(strtab.sh_offset + first);
+      this->Stream->seekg(strtab.sh_offset + first);
 
       // Read the string.  It may be followed by more than one NULL
       // terminator.  Count the total size of the region allocated to
@@ -639,7 +641,7 @@ cmELF::StringEntry const* cmELFInternalImpl<Types>::GetDynamicSectionString(
       // assumption.
       bool terminated = false;
       char c;
-      while (last != end && this->Stream.get(c) && !(terminated && c)) {
+      while (last != end && this->Stream->get(c) && !(terminated && c)) {
         ++last;
         if (c) {
           se.Value += c;
@@ -649,7 +651,7 @@ cmELF::StringEntry const* cmELFInternalImpl<Types>::GetDynamicSectionString(
       }
 
       // Make sure the whole value was read.
-      if (!this->Stream) {
+      if (!(*this->Stream)) {
         this->SetErrorMessage("Dynamic section specifies unreadable RPATH.");
         se.Value = "";
         return nullptr;
@@ -679,10 +681,9 @@ const long cmELF::TagMipsRldMapRel = 0;
 #endif
 
 cmELF::cmELF(const char* fname)
-  : Internal(nullptr)
 {
   // Try to open the file.
-  std::unique_ptr<cmsys::ifstream> fin(new cmsys::ifstream(fname));
+  auto fin = cm::make_unique<cmsys::ifstream>(fname);
 
   // Quit now if the file could not be opened.
   if (!fin || !*fin) {
@@ -725,12 +726,14 @@ cmELF::cmELF(const char* fname)
   // parser implementation.
   if (ident[EI_CLASS] == ELFCLASS32) {
     // 32-bit ELF
-    this->Internal = new cmELFInternalImpl<cmELFTypes32>(this, fin, order);
+    this->Internal = cm::make_unique<cmELFInternalImpl<cmELFTypes32>>(
+      this, std::move(fin), order);
   }
 #ifndef _SCO_DS
   else if (ident[EI_CLASS] == ELFCLASS64) {
     // 64-bit ELF
-    this->Internal = new cmELFInternalImpl<cmELFTypes64>(this, fin, order);
+    this->Internal = cm::make_unique<cmELFInternalImpl<cmELFTypes64>>(
+      this, std::move(fin), order);
   }
 #endif
   else {
@@ -739,10 +742,7 @@ cmELF::cmELF(const char* fname)
   }
 }
 
-cmELF::~cmELF()
-{
-  delete this->Internal;
-}
+cmELF::~cmELF() = default;
 
 bool cmELF::Valid() const
 {

Source/cmELF.h

@@ -6,6 +6,7 @@
 #include "cmConfigure.h" // IWYU pragma: keep
 
 #include <iosfwd>
+#include <memory>
 #include <string>
 #include <utility>
 #include <vector>
@@ -108,7 +109,7 @@ public:
 private:
   friend class cmELFInternal;
   bool Valid() const;
-  cmELFInternal* Internal;
+  std::unique_ptr<cmELFInternal> Internal;
   std::string ErrorMessage;
 };
 

Source/cmForEachCommand.cxx

@@ -5,6 +5,7 @@
 #include <sstream>
 #include <stdio.h>
 #include <stdlib.h>
+#include <utility>
 
 #include "cm_memory.hxx"
 
@@ -121,7 +122,7 @@ bool cmForEachCommand::InitialPass(std::vector<std::string> const& args,
   }
 
   // create a function blocker
-  auto f = cm::make_unique<cmForEachFunctionBlocker>(this->Makefile);
+  auto fb = cm::make_unique<cmForEachFunctionBlocker>(this->Makefile);
   if (args.size() > 1) {
     if (args[1] == "RANGE") {
       int start = 0;
@@ -168,23 +169,22 @@ bool cmForEachCommand::InitialPass(std::vector<std::string> const& args,
           break;
         }
       }
-      f->Args = range;
+      fb->Args = range;
     } else {
-      f->Args = args;
+      fb->Args = args;
     }
   } else {
-    f->Args = args;
+    fb->Args = args;
   }
-  this->Makefile->AddFunctionBlocker(f.release());
+  this->Makefile->AddFunctionBlocker(std::move(fb));
 
   return true;
 }
 
 bool cmForEachCommand::HandleInMode(std::vector<std::string> const& args)
 {
-  std::unique_ptr<cmForEachFunctionBlocker> f(
-    new cmForEachFunctionBlocker(this->Makefile));
-  f->Args.push_back(args[0]);
+  auto fb = cm::make_unique<cmForEachFunctionBlocker>(this->Makefile);
+  fb->Args.push_back(args[0]);
 
   enum Doing
   {
@@ -195,7 +195,7 @@ bool cmForEachCommand::HandleInMode(std::vector<std::string> const& args)
   Doing doing = DoingNone;
   for (unsigned int i = 2; i < args.size(); ++i) {
     if (doing == DoingItems) {
-      f->Args.push_back(args[i]);
+      fb->Args.push_back(args[i]);
     } else if (args[i] == "LISTS") {
       doing = DoingLists;
     } else if (args[i] == "ITEMS") {
@@ -203,7 +203,7 @@ bool cmForEachCommand::HandleInMode(std::vector<std::string> const& args)
     } else if (doing == DoingLists) {
       const char* value = this->Makefile->GetDefinition(args[i]);
       if (value && *value) {
-        cmSystemTools::ExpandListArgument(value, f->Args, true);
+        cmSystemTools::ExpandListArgument(value, fb->Args, true);
       }
     } else {
       std::ostringstream e;
@@ -214,7 +214,7 @@ bool cmForEachCommand::HandleInMode(std::vector<std::string> const& args)
     }
   }
 
-  this->Makefile->AddFunctionBlocker(f.release()); // TODO: pass unique_ptr
+  this->Makefile->AddFunctionBlocker(std::move(fb));
 
   return true;
 }

Source/cmFunctionCommand.cxx

@@ -179,8 +179,10 @@ bool cmFunctionCommand::InitialPass(std::vector<std::string> const& args,
   }
 
   // create a function blocker
-  cmFunctionFunctionBlocker* f = new cmFunctionFunctionBlocker();
-  cmAppend(f->Args, args);
-  this->Makefile->AddFunctionBlocker(f);
+  {
+    auto fb = cm::make_unique<cmFunctionFunctionBlocker>();
+    cmAppend(fb->Args, args);
+    this->Makefile->AddFunctionBlocker(std::move(fb));
+  }
   return true;
 }

Source/cmIfCommand.cxx

@@ -2,6 +2,8 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmIfCommand.h"
 
+#include "cm_memory.hxx"
+
 #include "cmConditionEvaluator.h"
 #include "cmExecutionStatus.h"
 #include "cmExpandedCommandArgument.h"
@@ -11,6 +13,8 @@
 #include "cmSystemTools.h"
 #include "cmake.h"
 
+#include <utility>
+
 static std::string cmIfCommandError(
   std::vector<cmExpandedCommandArgument> const& args)
 {
@@ -200,15 +204,17 @@ bool cmIfCommand::InvokeInitialPass(
     this->Makefile->IssueMessage(status, err);
   }
 
-  cmIfFunctionBlocker* f = new cmIfFunctionBlocker();
-  // if is isn't true block the commands
-  f->ScopeDepth = 1;
-  f->IsBlocking = !isTrue;
-  if (isTrue) {
-    f->HasRun = true;
+  {
+    auto fb = cm::make_unique<cmIfFunctionBlocker>();
+    // if is isn't true block the commands
+    fb->ScopeDepth = 1;
+    fb->IsBlocking = !isTrue;
+    if (isTrue) {
+      fb->HasRun = true;
+    }
+    fb->Args = args;
+    this->Makefile->AddFunctionBlocker(std::move(fb));
   }
-  f->Args = args;
-  this->Makefile->AddFunctionBlocker(f);
 
   return true;
 }

Source/cmMacroCommand.cxx

@@ -213,8 +213,10 @@ bool cmMacroCommand::InitialPass(std::vector<std::string> const& args,
   }
 
   // create a function blocker
-  cmMacroFunctionBlocker* f = new cmMacroFunctionBlocker();
-  cmAppend(f->Args, args);
-  this->Makefile->AddFunctionBlocker(f);
+  {
+    auto fb = cm::make_unique<cmMacroFunctionBlocker>();
+    cmAppend(fb->Args, args);
+    this->Makefile->AddFunctionBlocker(std::move(fb));
+  }
   return true;
 }

Source/cmMakefile.cxx

@@ -118,7 +118,6 @@ cmMakefile::~cmMakefile()
   cmDeleteAll(this->SourceFiles);
   cmDeleteAll(this->Tests);
   cmDeleteAll(this->ImportedTargetsOwned);
-  cmDeleteAll(this->FunctionBlockers);
   cmDeleteAll(this->EvaluationFiles);
 }
 
@@ -3076,13 +3075,13 @@ bool cmMakefile::IsFunctionBlocked(const cmListFileFunction& lff,
                                    cmExecutionStatus& status)
 {
   // if there are no blockers get out of here
-  if (this->FunctionBlockers.begin() == this->FunctionBlockers.end()) {
+  if (this->FunctionBlockers.empty()) {
     return false;
   }
 
   // loop over all function blockers to see if any block this command
   // evaluate in reverse, this is critical for balanced IF statements etc
-  for (cmFunctionBlocker* pos : cmReverseRange(this->FunctionBlockers)) {
+  for (auto const& pos : cmReverseRange(this->FunctionBlockers)) {
     if (pos->IsFunctionBlocked(lff, *this, status)) {
       return true;
     }
@@ -3102,7 +3101,8 @@ void cmMakefile::PopFunctionBlockerBarrier(bool reportError)
   FunctionBlockersType::size_type barrier =
     this->FunctionBlockerBarriers.back();
   while (this->FunctionBlockers.size() > barrier) {
-    std::unique_ptr<cmFunctionBlocker> fb(this->FunctionBlockers.back());
+    std::unique_ptr<cmFunctionBlocker> fb(
+      std::move(this->FunctionBlockers.back()));
     this->FunctionBlockers.pop_back();
     if (reportError) {
       // Report the context in which the unclosed block was opened.
@@ -3227,14 +3227,14 @@ bool cmMakefile::ExpandArguments(
   return !cmSystemTools::GetFatalErrorOccured();
 }
 
-void cmMakefile::AddFunctionBlocker(cmFunctionBlocker* fb)
+void cmMakefile::AddFunctionBlocker(std::unique_ptr<cmFunctionBlocker> fb)
 {
   if (!this->ExecutionStatusStack.empty()) {
     // Record the context in which the blocker is created.
     fb->SetStartingContext(this->GetExecutionContext());
   }
 
-  this->FunctionBlockers.push_back(fb);
+  this->FunctionBlockers.push_back(std::move(fb));
 }
 
 std::unique_ptr<cmFunctionBlocker> cmMakefile::RemoveFunctionBlocker(
@@ -3250,9 +3250,8 @@ std::unique_ptr<cmFunctionBlocker> cmMakefile::RemoveFunctionBlocker(
   // Search for the function blocker whose scope this command ends.
   for (FunctionBlockersType::size_type i = this->FunctionBlockers.size();
        i > barrier; --i) {
-    std::vector<cmFunctionBlocker*>::iterator pos =
-      this->FunctionBlockers.begin() + (i - 1);
-    if (*pos == fb) {
+    auto pos = this->FunctionBlockers.begin() + (i - 1);
+    if (pos->get() == fb) {
       // Warn if the arguments do not match, but always remove.
       if (!(*pos)->ShouldRemove(lff, *this)) {
         cmListFileContext const& lfc = fb->GetStartingContext();
@@ -3268,9 +3267,9 @@ std::unique_ptr<cmFunctionBlocker> cmMakefile::RemoveFunctionBlocker(
         /* clang-format on */
         this->IssueMessage(MessageType::AUTHOR_WARNING, e.str());
       }
-      cmFunctionBlocker* b = *pos;
+      std::unique_ptr<cmFunctionBlocker> b = std::move(*pos);
       this->FunctionBlockers.erase(pos);
-      return std::unique_ptr<cmFunctionBlocker>(b);
+      return b;
     }
   }
 

Source/cmMakefile.h

@@ -17,6 +17,7 @@
 #include <vector>
 
 #include "cmAlgorithms.h"
+#include "cmFunctionBlocker.h"
 #include "cmListFileCache.h"
 #include "cmMessageType.h"
 #include "cmNewLineStyle.h"
@@ -36,7 +37,6 @@ class cmCustomCommandLines;
 class cmExecutionStatus;
 class cmExpandedCommandArgument;
 class cmExportBuildFileGenerator;
-class cmFunctionBlocker;
 class cmGeneratorExpressionEvaluationFile;
 class cmGlobalGenerator;
 class cmInstallGenerator;
@@ -95,7 +95,7 @@ public:
   /**
    * Add a function blocker to this makefile
    */
-  void AddFunctionBlocker(cmFunctionBlocker* fb);
+  void AddFunctionBlocker(std::unique_ptr<cmFunctionBlocker> fb);
 
   /// @return whether we are processing the top CMakeLists.txt file.
   bool IsRootMakefile() const;
@@ -955,7 +955,7 @@ private:
   bool EnforceUniqueDir(const std::string& srcPath,
                         const std::string& binPath) const;
 
-  typedef std::vector<cmFunctionBlocker*> FunctionBlockersType;
+  typedef std::vector<std::unique_ptr<cmFunctionBlocker>> FunctionBlockersType;
   FunctionBlockersType FunctionBlockers;
   std::vector<FunctionBlockersType::size_type> FunctionBlockerBarriers;
   void PushFunctionBlockerBarrier();

Source/cmWhileCommand.cxx

@@ -2,6 +2,8 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmWhileCommand.h"
 
+#include "cm_memory.hxx"
+
 #include "cmConditionEvaluator.h"
 #include "cmExecutionStatus.h"
 #include "cmExpandedCommandArgument.h"
@@ -9,6 +11,8 @@
 #include "cmMessageType.h"
 #include "cmSystemTools.h"
 
+#include <utility>
+
 cmWhileFunctionBlocker::cmWhileFunctionBlocker(cmMakefile* mf)
   : Makefile(mf)
   , Depth(0)
@@ -134,9 +138,10 @@ bool cmWhileCommand::InvokeInitialPass(
   }
 
   // create a function blocker
-  cmWhileFunctionBlocker* f = new cmWhileFunctionBlocker(this->Makefile);
-  f->Args = args;
-  this->Makefile->AddFunctionBlocker(f);
-
+  {
+    auto fb = cm::make_unique<cmWhileFunctionBlocker>(this->Makefile);
+    fb->Args = args;
+    this->Makefile->AddFunctionBlocker(std::move(fb));
+  }
   return true;
 }