Merge topic 'source-cleanup'

b86b6aaa4a Source: Cleanup and simplify some code

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

Source/CPack/IFW/cmCPackIFWGenerator.h

@@ -9,13 +9,15 @@
 #include <string>
 #include <vector>
 
-#include "cmCPackComponentGroup.h"
 #include "cmCPackGenerator.h"
 #include "cmCPackIFWCommon.h"
 #include "cmCPackIFWInstaller.h"
 #include "cmCPackIFWPackage.h"
 #include "cmCPackIFWRepository.h"
 
+class cmCPackComponent;
+class cmCPackComponentGroup;
+
 /** \class cmCPackIFWGenerator
  * \brief A generator for Qt Installer Framework tools
  *
@@ -30,8 +32,6 @@ public:
 
   using PackagesMap = std::map<std::string, cmCPackIFWPackage>;
   using RepositoriesMap = std::map<std::string, cmCPackIFWRepository>;
-  using ComponentsMap = std::map<std::string, cmCPackComponent>;
-  using ComponentGoupsMap = std::map<std::string, cmCPackComponentGroup>;
   using DependenceMap =
     std::map<std::string, cmCPackIFWPackage::DependenceStruct>;
 
@@ -154,8 +154,8 @@ private:
   std::string ArchiveFormat;
   std::string ArchiveCompression;
 
-  bool OnlineOnly;
-  bool ResolveDuplicateNames;
+  bool OnlineOnly{};
+  bool ResolveDuplicateNames{};
   std::vector<std::string> PkgsDirsVector;
   std::vector<std::string> RepoDirsVector;
 };

Source/CPack/IFW/cmCPackIFWPackage.h

@@ -44,7 +44,7 @@ public:
   struct DependenceStruct
   {
     DependenceStruct();
-    DependenceStruct(const std::string& dependence);
+    explicit DependenceStruct(const std::string& dependence);
 
     std::string Name;
     CompareStruct Compare;

Source/CPack/cmCPackComponentGroup.h

@@ -35,12 +35,10 @@ class cmCPackComponent
 {
 public:
   cmCPackComponent()
-    : Group(nullptr)
-    , IsRequired(true)
+    : IsRequired(true)
     , IsHidden(false)
     , IsDisabledByDefault(false)
     , IsDownloaded(false)
-    , TotalSize(0)
   {
   }
 
@@ -51,7 +49,7 @@ public:
   std::string DisplayName;
 
   /// The component group that contains this component (if any).
-  cmCPackComponentGroup* Group;
+  cmCPackComponentGroup* Group = nullptr;
 
   /// Whether this component group must always be installed.
   bool IsRequired : 1;
@@ -103,7 +101,7 @@ public:
   unsigned long GetInstalledSizeInKbytes(const std::string& installDir) const;
 
 private:
-  mutable unsigned long TotalSize;
+  mutable unsigned long TotalSize = 0;
 };
 
 /** \class cmCPackComponentGroup
@@ -113,7 +111,8 @@ class cmCPackComponentGroup
 {
 public:
   cmCPackComponentGroup()
-    : ParentGroup(nullptr)
+    : IsBold(false)
+    , IsExpandedByDefault(false)
   {
   }
 
@@ -136,7 +135,7 @@ public:
   std::vector<cmCPackComponent*> Components;
 
   /// The parent group of this component group (if any).
-  cmCPackComponentGroup* ParentGroup;
+  cmCPackComponentGroup* ParentGroup = nullptr;
 
   /// The subgroups of this group.
   std::vector<cmCPackComponentGroup*> Subgroups;

Source/CPack/cmCPackCygwinBinaryGenerator.cxx

@@ -17,9 +17,7 @@ cmCPackCygwinBinaryGenerator::cmCPackCygwinBinaryGenerator()
 {
 }
 
-cmCPackCygwinBinaryGenerator::~cmCPackCygwinBinaryGenerator()
-{
-}
+cmCPackCygwinBinaryGenerator::~cmCPackCygwinBinaryGenerator() = default;
 
 int cmCPackCygwinBinaryGenerator::InitializeInternal()
 {
@@ -43,7 +41,7 @@ int cmCPackCygwinBinaryGenerator::PackageFiles()
   // create an extra scope to force the stream
   // to create the file before the super class is called
   {
-    cmGeneratedFileStream ofs(manifestFile.c_str());
+    cmGeneratedFileStream ofs(manifestFile);
     for (std::string const& file : files) {
       // remove the temp dir and replace with /usr
       ofs << file.substr(tempdir.size()) << "\n";

Source/CPack/cmCPackCygwinBinaryGenerator.h

@@ -19,8 +19,8 @@ public:
   ~cmCPackCygwinBinaryGenerator() override;
 
 protected:
-  virtual int InitializeInternal();
-  int PackageFiles();
-  virtual const char* GetOutputExtension();
+  int InitializeInternal() override;
+  int PackageFiles() override;
+  const char* GetOutputExtension() override;
   std::string OutputExtension;
 };

Source/CPack/cmCPackCygwinSourceGenerator.cxx

@@ -26,9 +26,7 @@ cmCPackCygwinSourceGenerator::cmCPackCygwinSourceGenerator()
 {
 }
 
-cmCPackCygwinSourceGenerator::~cmCPackCygwinSourceGenerator()
-{
-}
+cmCPackCygwinSourceGenerator::~cmCPackCygwinSourceGenerator() = default;
 
 int cmCPackCygwinSourceGenerator::InitializeInternal()
 {
@@ -50,7 +48,7 @@ int cmCPackCygwinSourceGenerator::PackageFiles()
   // Now create a tar file that contains the above .tar.bz2 file
   // and the CPACK_CYGWIN_PATCH_FILE and CPACK_TOPLEVEL_DIRECTORY
   // files
-  std::string compressOutFile = packageDirFileName;
+  const std::string& compressOutFile = packageDirFileName;
   // at this point compressOutFile is the full path to
   // _CPack_Package/.../package-2.5.0.tar.bz2
   // we want to create a tar _CPack_Package/.../package-2.5.0-1-src.tar.bz2

Source/CPack/cmCPackCygwinSourceGenerator.h

@@ -19,10 +19,10 @@ public:
   ~cmCPackCygwinSourceGenerator() override;
 
 protected:
-  const char* GetPackagingInstallPrefix();
-  virtual int InitializeInternal();
-  int PackageFiles();
-  virtual const char* GetOutputExtension();
+  const char* GetPackagingInstallPrefix() override;
+  int InitializeInternal() override;
+  int PackageFiles() override;
+  const char* GetOutputExtension() override;
   std::string InstallPrefix;
   std::string OutputExtension;
 };

Source/CPack/cmCPackDebGenerator.cxx

@@ -203,7 +203,7 @@ bool DebGenerator::generateDataTar() const
 
   // uid/gid should be the one of the root user, and this root user has
   // always uid/gid equal to 0.
-  data_tar.SetUIDAndGID(0u, 0u);
+  data_tar.SetUIDAndGID(0U, 0U);
   data_tar.SetUNAMEAndGNAME("root", "root");
 
   // now add all directories which have to be compressed
@@ -902,7 +902,7 @@ std::string cmCPackDebGenerator::GetComponentInstallDirNameSuffix(
   }
 
   if (this->componentPackageMethod == ONE_PACKAGE) {
-    return std::string("ALL_COMPONENTS_IN_ONE");
+    return { "ALL_COMPONENTS_IN_ONE" };
   }
   // We have to find the name of the COMPONENT GROUP
   // the current COMPONENT belongs to.

Source/CPack/cmCPackDebGenerator.h

@@ -29,9 +29,9 @@ public:
 #ifdef __APPLE__
     // on MacOS enable CPackDeb iff dpkg is found
     std::vector<std::string> locations;
-    locations.push_back("/sw/bin");        // Fink
-    locations.push_back("/opt/local/bin"); // MacPorts
-    return cmSystemTools::FindProgram("dpkg", locations) != "" ? true : false;
+    locations.emplace_back("/sw/bin");        // Fink
+    locations.emplace_back("/opt/local/bin"); // MacPorts
+    return !cmSystemTools::FindProgram("dpkg", locations).empty();
 #else
     // legacy behavior on other systems
     return true;

Source/CPack/cmCPackDragNDropGenerator.h

@@ -33,7 +33,7 @@ protected:
 
   bool CopyFile(std::ostringstream& source, std::ostringstream& target);
   bool CreateEmptyFile(std::ostringstream& target, size_t size);
-  bool RunCommand(std::ostringstream& command, std::string* output = 0);
+  bool RunCommand(std::ostringstream& command, std::string* output = nullptr);
 
   std::string GetComponentInstallDirNameSuffix(
     const std::string& componentName) override;

Source/CPack/cmCPackFreeBSDGenerator.cxx

@@ -205,7 +205,7 @@ std::string cmCPackFreeBSDGenerator::var_lookup(const char* var_name)
 {
   cmValue pv = this->GetOption(var_name);
   if (!pv) {
-    return std::string();
+    return {};
   }
   return *pv;
 }

Source/CPack/cmCPackGeneratorFactory.h

@@ -41,5 +41,5 @@ private:
   using t_GeneratorCreatorsMap = std::map<std::string, CreateGeneratorCall*>;
   t_GeneratorCreatorsMap GeneratorCreators;
   DescriptionsMap GeneratorDescriptions;
-  cmCPackLog* Logger;
+  cmCPackLog* Logger{};
 };

Source/CPack/cmCPackLog.h

@@ -128,7 +128,7 @@ public:
   }
 
   const char* Data;
-  size_t Length;
+  std::streamsize Length;
 };
 
 inline std::ostream& operator<<(std::ostream& os, const cmCPackLogWrite& c)

Source/CPack/cmCPackNSISGenerator.cxx

@@ -883,7 +883,7 @@ std::string cmCPackNSISGenerator::CreateSelectionDependenciesDescription(
 {
   // Don't visit a component twice
   if (visited.count(component)) {
-    return std::string();
+    return {};
   }
   visited.insert(component);
 
@@ -907,7 +907,7 @@ std::string cmCPackNSISGenerator::CreateDeselectionDependenciesDescription(
 {
   // Don't visit a component twice
   if (visited.count(component)) {
-    return std::string();
+    return {};
   }
   visited.insert(component);
 
@@ -933,7 +933,7 @@ std::string cmCPackNSISGenerator::CreateComponentGroupDescription(
 {
   if (group->Components.empty() && group->Subgroups.empty()) {
     // Silently skip empty groups. NSIS doesn't support them.
-    return std::string();
+    return {};
   }
 
   std::string code = "SectionGroup ";

Source/CPack/cmCPackOSXX11Generator.cxx

@@ -20,8 +20,6 @@ cmCPackOSXX11Generator::~cmCPackOSXX11Generator() = default;
 
 int cmCPackOSXX11Generator::PackageFiles()
 {
-  // TODO: Use toplevel ?
-  //       It is used! Is this an obsolete comment?
 
   cmValue cpackPackageExecutables =
     this->GetOption("CPACK_PACKAGE_EXECUTABLES");
@@ -147,33 +145,26 @@ int cmCPackOSXX11Generator::PackageFiles()
   // since we get random dashboard failures with this one
   // try running it more than once
   int retVal = 1;
-  int numTries = 10;
   bool res = false;
-  while (numTries > 0) {
+  for (unsigned numTries = 10; numTries > 0; numTries--) {
     res = cmSystemTools::RunSingleCommand(
       dmgCmd.str(), &output, &output, &retVal, nullptr, this->GeneratorVerbose,
       cmDuration::zero());
     if (res && !retVal) {
-      numTries = -1;
-      break;
+      return 1;
     }
     cmSystemTools::Delay(500);
-    numTries--;
-  }
-  if (!res || retVal) {
-    cmGeneratedFileStream ofs(tmpFile);
-    ofs << "# Run command: " << dmgCmd.str() << std::endl
-        << "# Output:" << std::endl
-        << output << std::endl;
-    cmCPackLogger(cmCPackLog::LOG_ERROR,
-                  "Problem running hdiutil command: "
-                    << dmgCmd.str() << std::endl
-                    << "Please check " << tmpFile << " for errors"
-                    << std::endl);
-    return 0;
   }
 
-  return 1;
+  cmGeneratedFileStream ofs(tmpFile);
+  ofs << "# Run command: " << dmgCmd.str() << std::endl
+      << "# Output:" << std::endl
+      << output << std::endl;
+  cmCPackLogger(cmCPackLog::LOG_ERROR,
+                "Problem running hdiutil command: "
+                  << dmgCmd.str() << std::endl
+                  << "Please check " << tmpFile << " for errors" << std::endl);
+  return 0;
 }
 
 int cmCPackOSXX11Generator::InitializeInternal()

Source/CPack/cmCPackOSXX11Generator.h

@@ -25,7 +25,7 @@ public:
   ~cmCPackOSXX11Generator() override;
 
 protected:
-  virtual int InitializeInternal() override;
+  int InitializeInternal() override;
   int PackageFiles() override;
   const char* GetPackagingInstallPrefix() override;
   const char* GetOutputExtension() override { return ".dmg"; }
@@ -33,7 +33,7 @@ protected:
   // bool CopyCreateResourceFile(const std::string& name,
   //                            const std::string& dir);
   bool CopyResourcePlistFile(const std::string& name, const std::string& dir,
-                             const char* outputFileName = 0,
+                             const char* outputFileName = nullptr,
                              bool copyOnly = false);
   std::string InstallPrefix;
 };

Source/CPack/cmCPackPKGGenerator.h

@@ -43,7 +43,8 @@ protected:
   // which will be configured via ConfigureFile.
   bool CopyCreateResourceFile(const std::string& name,
                               const std::string& dirName);
-  bool CopyResourcePlistFile(const std::string& name, const char* outName = 0);
+  bool CopyResourcePlistFile(const std::string& name,
+                             const char* outName = nullptr);
 
   int CopyInstallScript(const std::string& resdir, const std::string& script,
                         const std::string& name);

Source/CPack/cmCPackRPMGenerator.cxx

@@ -329,9 +329,10 @@ int cmCPackRPMGenerator::PackageComponents(bool ignoreGroup)
 
   if (retval) {
     this->AddGeneratedPackageNames();
+    return retval;
   }
 
-  return retval;
+  return 0;
 }
 
 int cmCPackRPMGenerator::PackageComponentsAllInOne(
@@ -424,7 +425,7 @@ std::string cmCPackRPMGenerator::GetComponentInstallDirNameSuffix(
   }
 
   if (this->componentPackageMethod == ONE_PACKAGE) {
-    return std::string("ALL_COMPONENTS_IN_ONE");
+    return { "ALL_COMPONENTS_IN_ONE" };
   }
   // We have to find the name of the COMPONENT GROUP
   // the current COMPONENT belongs to.

Source/CPack/cmCPackRPMGenerator.h

@@ -32,9 +32,9 @@ public:
 #ifdef __APPLE__
     // on MacOS enable CPackRPM iff rpmbuild is found
     std::vector<std::string> locations;
-    locations.push_back("/sw/bin");        // Fink
-    locations.push_back("/opt/local/bin"); // MacPorts
-    return cmSystemTools::FindProgram("rpmbuild") != "" ? true : false;
+    locations.emplace_back("/sw/bin");        // Fink
+    locations.emplace_back("/opt/local/bin"); // MacPorts
+    return !cmSystemTools::FindProgram("rpmbuild").empty();
 #else
     // legacy behavior on other systems
     return true;

Source/CPack/cpack.cxx

@@ -67,7 +67,7 @@ struct cpackDefinitions
 {
   using MapType = std::map<std::string, std::string>;
   MapType Map;
-  cmCPackLog* Log;
+  cmCPackLog* Log{};
 };
 
 int cpackDefinitionArgument(const char* argument, const char* cValue,

Source/CTest/cmCTestGIT.cxx

@@ -24,7 +24,7 @@ static unsigned int cmCTestGITVersion(unsigned int epic, unsigned int major,
                                       unsigned int minor, unsigned int fix)
 {
   // 1.6.5.0 maps to 10605000
-  return fix + minor * 1000 + major * 100000 + epic * 10000000;
+  return epic * 10000000 + major * 100000 + minor * 1000 + fix;
 }
 
 cmCTestGIT::cmCTestGIT(cmCTest* ct, std::ostream& log)

Source/CTest/cmCTestLaunch.h

@@ -24,14 +24,14 @@ public:
   /** Entry point from ctest executable main().  */
   static int Main(int argc, const char* const argv[]);
 
+  cmCTestLaunch(const cmCTestLaunch&) = delete;
+  cmCTestLaunch& operator=(const cmCTestLaunch&) = delete;
+
 private:
   // Initialize the launcher from its command line.
   cmCTestLaunch(int argc, const char* const* argv);
   ~cmCTestLaunch();
 
-  cmCTestLaunch(const cmCTestLaunch&) = delete;
-  cmCTestLaunch& operator=(const cmCTestLaunch&) = delete;
-
   // Run the real command.
   int Run();
   void RunChild();

Source/CTest/cmCTestP4.h

@@ -34,14 +34,6 @@ private:
     std::string Name;
     std::string EMail;
     std::string AccessTime;
-
-    User()
-      : UserName()
-      , Name()
-      , EMail()
-      , AccessTime()
-    {
-    }
   };
   std::map<std::string, User> Users;
   std::vector<std::string> P4Options;

Source/CTest/cmCTestVC.cxx

@@ -126,7 +126,7 @@ std::string cmCTestVC::GetNightlyTime()
   snprintf(current_time, sizeof(current_time), "%04d-%02d-%02d %02d:%02d:%02d",
            t->tm_year + 1900, t->tm_mon + 1, t->tm_mday, t->tm_hour, t->tm_min,
            t->tm_sec);
-  return std::string(current_time);
+  return { current_time };
 }
 
 void cmCTestVC::Cleanup()

Source/CTest/cmCTestVC.h

@@ -95,15 +95,10 @@ protected:
   /** Represent change to one file.  */
   struct File
   {
-    PathStatus Status;
-    Revision const* Rev;
-    Revision const* PriorRev;
-    File()
-      : Status(PathUpdated)
-      , Rev(nullptr)
-      , PriorRev(nullptr)
-    {
-    }
+    PathStatus Status = PathUpdated;
+    Revision const* Rev = nullptr;
+    Revision const* PriorRev = nullptr;
+    File() = default;
     File(PathStatus status, Revision const* rev, Revision const* priorRev)
       : Status(status)
       , Rev(rev)

Source/CTest/cmProcess.h

@@ -52,9 +52,9 @@ public:
   };
 
   State GetProcessStatus();
-  int GetId() { return this->Id; }
+  int GetId() const { return this->Id; }
   void SetId(int id) { this->Id = id; }
-  int64_t GetExitValue() { return this->ExitValue; }
+  int64_t GetExitValue() const { return this->ExitValue; }
   cmDuration GetTotalTime() { return this->TotalTime; }
 
   enum class Exception
@@ -111,15 +111,11 @@ private:
   class Buffer : public std::vector<char>
   {
     // Half-open index range of partial line already scanned.
-    size_type First;
-    size_type Last;
+    size_type First = 0;
+    size_type Last = 0;
 
   public:
-    Buffer()
-      : First(0)
-      , Last(0)
-    {
-    }
+    Buffer() = default;
     bool GetLine(std::string& line);
     bool GetLast(std::string& line);
   };

Source/cmMessenger.cxx

@@ -155,19 +155,16 @@ void cmMessenger::IssueMessage(MessageType t, const std::string& text,
                                const cmListFileBacktrace& backtrace) const
 {
   bool force = false;
-  if (!force) {
-    // override the message type, if needed, for warnings and errors
-    MessageType override = this->ConvertMessageType(t);
-    if (override != t) {
-      t = override;
-      force = true;
-    }
+  // override the message type, if needed, for warnings and errors
+  MessageType override = this->ConvertMessageType(t);
+  if (override != t) {
+    t = override;
+    force = true;
   }
 
-  if (!force && !this->IsMessageTypeVisible(t)) {
-    return;
+  if (force || this->IsMessageTypeVisible(t)) {
+    this->DisplayMessage(t, text, backtrace);
   }
-  this->DisplayMessage(t, text, backtrace);
 }
 
 void cmMessenger::DisplayMessage(MessageType t, const std::string& text,

Source/cmXCOFF.cxx

@@ -61,20 +61,20 @@ namespace {
 
 struct XCOFF32
 {
-  typedef struct filehdr filehdr;
-  typedef struct aouthdr aouthdr;
-  typedef struct scnhdr scnhdr;
-  typedef struct ldhdr ldhdr;
+  using filehdr = struct filehdr;
+  using aouthdr = struct aouthdr;
+  using scnhdr = struct scnhdr;
+  using ldhdr = struct ldhdr;
   static const std::size_t aouthdr_size = _AOUTHSZ_EXEC;
 };
 const unsigned char xcoff32_magic[] = { 0x01, 0xDF };
 
 struct XCOFF64
 {
-  typedef struct filehdr_64 filehdr;
-  typedef struct aouthdr_64 aouthdr;
-  typedef struct scnhdr_64 scnhdr;
-  typedef struct ldhdr_64 ldhdr;
+  using filehdr = struct filehdr_64;
+  using aouthdr = struct aouthdr_64;
+  using scnhdr = struct scnhdr_64;
+  using ldhdr = struct ldhdr_64;
   static const std::size_t aouthdr_size = _AOUTHSZ_EXEC_64;
 };
 const unsigned char xcoff64_magic[] = { 0x01, 0xF7 };
@@ -326,8 +326,8 @@ cmXCOFF::cmXCOFF(const char* fname, Mode mode)
 
 cmXCOFF::~cmXCOFF() = default;
 
-cmXCOFF::cmXCOFF(cmXCOFF&&) = default;
-cmXCOFF& cmXCOFF::operator=(cmXCOFF&&) = default;
+cmXCOFF::cmXCOFF(cmXCOFF&&) noexcept = default;
+cmXCOFF& cmXCOFF::operator=(cmXCOFF&&) noexcept = default;
 
 bool cmXCOFF::Valid() const
 {

Source/cmXCOFF.h

@@ -35,9 +35,9 @@ public:
   /** Destruct.   */
   ~cmXCOFF();
 
-  cmXCOFF(cmXCOFF&&);
+  cmXCOFF(cmXCOFF&&) noexcept;
   cmXCOFF(cmXCOFF const&) = delete;
-  cmXCOFF& operator=(cmXCOFF&&);
+  cmXCOFF& operator=(cmXCOFF&&) noexcept;
   cmXCOFF& operator=(cmXCOFF const&) = delete;
 
   /** Get the error message if any.  */