Merge topic 'string_scopes'

3039fd7a29 cmDependsC: Use faster cmSystemTools::FileTimeCompare
b96c3c74dd cmDependsC: Use auto for long type names
e81b425019 cmDependsC: Remove useless string preallocation artifact
d1a54ee26a cmDepends: Reduce temporary object lifetime with local scopes
ae416a6b5c cmLocalUnixMakefileGenerator3: Use std::unique_ptr instead of new/delete
60a407b01c cmLocalUnixMakefileGenerator3: Pass strings instead of recomputing them
addd1ce402 cmLocalUnixMakefileGenerator3: Move local strings into local brace scopes

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Brad King <brad.king@kitware.com>
Merge-request: !3088

4 files changed, 56 insertions, 68 deletions

diff --git a/Source/cmDepends.cxx b/Source/cmDepends.cxx
index 7d1d316..c128b02 100644
--- a/Source/cmDepends.cxx
+++ b/Source/cmDepends.cxx
@@ -29,27 +29,27 @@ cmDepends::~cmDepends()
 
 bool cmDepends::Write(std::ostream& makeDepends, std::ostream& internalDepends)
 {
-  // Lookup the set of sources to scan.
-  std::string srcLang = "CMAKE_DEPENDS_CHECK_";
-  srcLang += this->Language;
-  cmMakefile* mf = this->LocalGenerator->GetMakefile();
-  std::string const& srcStr = mf->GetSafeDefinition(srcLang);
-  std::vector<std::string> pairs;
-  cmSystemTools::ExpandListArgument(srcStr, pairs);
-
   std::map<std::string, std::set<std::string>> dependencies;
-  for (std::vector<std::string>::iterator si = pairs.begin();
-       si != pairs.end();) {
-    // Get the source and object file.
-    std::string const& src = *si++;
-    if (si == pairs.end()) {
-      break;
+  {
+    // Lookup the set of sources to scan.
+    std::vector<std::string> pairs;
+    {
+      std::string const srcLang = "CMAKE_DEPENDS_CHECK_" + this->Language;
+      cmMakefile* mf = this->LocalGenerator->GetMakefile();
+      cmSystemTools::ExpandListArgument(mf->GetSafeDefinition(srcLang), pairs);
+    }
+    for (std::vector<std::string>::iterator si = pairs.begin();
+         si != pairs.end();) {
+      // Get the source and object file.
+      std::string const& src = *si++;
+      if (si == pairs.end()) {
+        break;
+      }
+      std::string const& obj = *si++;
+      dependencies[obj].insert(src);
     }
-    std::string const& obj = *si++;
-    dependencies[obj].insert(src);
   }
   for (auto const& d : dependencies) {
-
     // Write the dependencies for this pair.
     if (!this->WriteDependencies(d.second, d.first, makeDepends,
                                  internalDepends)) {
diff --git a/Source/cmDependsC.cxx b/Source/cmDependsC.cxx
index f5c106c..a85f5ee 100644
--- a/Source/cmDependsC.cxx
+++ b/Source/cmDependsC.cxx
@@ -6,7 +6,6 @@
 #include <utility>
 
 #include "cmAlgorithms.h"
-#include "cmFileTimeComparison.h"
 #include "cmLocalGenerator.h"
 #include "cmMakefile.h"
 #include "cmSystemTools.h"
@@ -123,12 +122,6 @@ bool cmDependsC::WriteDependencies(const std::set<std::string>& sources,
     }
 
     std::set<std::string> scanned;
-
-    // Use reserve to allocate enough memory for tempPathStr
-    // so that during the loops no memory is allocated or freed
-    std::string tempPathStr;
-    tempPathStr.reserve(4 * 1024);
-
     while (!this->Unscanned.empty()) {
       // Get the next file to scan.
       UnscannedEntry current = this->Unscanned.front();
@@ -147,22 +140,21 @@ bool cmDependsC::WriteDependencies(const std::set<std::string>& sources,
         // the source containing the include statement.
         fullName = current.QuotedLocation;
       } else {
-        std::map<std::string, std::string>::iterator headerLocationIt =
+        auto headerLocationIt =
           this->HeaderLocationCache.find(current.FileName);
         if (headerLocationIt != this->HeaderLocationCache.end()) {
           fullName = headerLocationIt->second;
         } else {
-          for (std::string const& i : this->IncludePath) {
+          for (std::string const& iPath : this->IncludePath) {
             // Construct the name of the file as if it were in the current
             // include directory.  Avoid using a leading "./".
-
-            tempPathStr =
-              cmSystemTools::CollapseCombinedPath(i, current.FileName);
+            std::string tmpPath =
+              cmSystemTools::CollapseCombinedPath(iPath, current.FileName);
 
             // Look for the file in this location.
-            if (cmSystemTools::FileExists(tempPathStr, true)) {
-              fullName = tempPathStr;
-              HeaderLocationCache[current.FileName] = fullName;
+            if (cmSystemTools::FileExists(tmpPath, true)) {
+              fullName = tmpPath;
+              this->HeaderLocationCache[current.FileName] = std::move(tmpPath);
               break;
             }
           }
@@ -183,8 +175,7 @@ bool cmDependsC::WriteDependencies(const std::set<std::string>& sources,
         scanned.insert(fullName);
 
         // Check whether this file is already in the cache
-        std::map<std::string, cmIncludeLines*>::iterator fileIt =
-          this->FileCache.find(fullName);
+        auto fileIt = this->FileCache.find(fullName);
         if (fileIt != this->FileCache.end()) {
           fileIt->second->Used = true;
           dependencies.insert(fullName);
@@ -266,8 +257,8 @@ void cmDependsC::ReadCacheFile()
     if (!haveFileName) {
       haveFileName = true;
       int newer = 0;
-      cmFileTimeComparison comp;
-      bool res = comp.FileTimeCompare(this->CacheFileName, line, &newer);
+      bool res =
+        cmSystemTools::FileTimeCompare(this->CacheFileName, line, &newer);
 
       if (res && newer == 1) // cache is newer than the parsed file
       {
diff --git a/Source/cmLocalUnixMakefileGenerator3.cxx b/Source/cmLocalUnixMakefileGenerator3.cxx
index 33a31dd..d1093be 100644
--- a/Source/cmLocalUnixMakefileGenerator3.cxx
+++ b/Source/cmLocalUnixMakefileGenerator3.cxx
@@ -1264,9 +1264,9 @@ bool cmLocalUnixMakefileGenerator3::UpdateDependencies(
   // Check if any multiple output pairs have a missing file.
   this->CheckMultipleOutputs(verbose);
 
-  std::string dir = cmSystemTools::GetFilenamePath(tgtInfo);
-  std::string internalDependFile = dir + "/depend.internal";
-  std::string dependFile = dir + "/depend.make";
+  std::string const targetDir = cmSystemTools::GetFilenamePath(tgtInfo);
+  std::string const internalDependFile = targetDir + "/depend.internal";
+  std::string const dependFile = targetDir + "/depend.make";
 
   // If the target DependInfo.cmake file has changed since the last
   // time dependencies were scanned then force rescanning.  This may
@@ -1292,10 +1292,10 @@ bool cmLocalUnixMakefileGenerator3::UpdateDependencies(
   // If the directory information is newer than depend.internal, include dirs
   // may have changed. In this case discard all old dependencies.
   bool needRescanDirInfo = false;
-  std::string dirInfoFile = this->GetCurrentBinaryDirectory();
-  dirInfoFile += "/CMakeFiles";
-  dirInfoFile += "/CMakeDirectoryInformation.cmake";
   {
+    std::string dirInfoFile = this->GetCurrentBinaryDirectory();
+    dirInfoFile += "/CMakeFiles";
+    dirInfoFile += "/CMakeDirectoryInformation.cmake";
     int result;
     if (!ftc->FileTimeCompare(internalDependFile, dirInfoFile, &result) ||
         result < 0) {
@@ -1335,7 +1335,7 @@ bool cmLocalUnixMakefileGenerator3::UpdateDependencies(
 
   if (needRescanDependInfo || needRescanDirInfo || needRescanDependencies) {
     // The dependencies must be regenerated.
-    std::string targetName = cmSystemTools::GetFilenameName(dir);
+    std::string targetName = cmSystemTools::GetFilenameName(targetDir);
     targetName = targetName.substr(0, targetName.length() - 4);
     std::string message = "Scanning dependencies of target ";
     message += targetName;
@@ -1343,7 +1343,8 @@ bool cmLocalUnixMakefileGenerator3::UpdateDependencies(
                                        cmsysTerminal_Color_ForegroundBold,
                                      message.c_str(), true, color);
 
-    return this->ScanDependencies(dir, validDependencies);
+    return this->ScanDependencies(targetDir, dependFile, internalDependFile,
+                                  validDependencies);
   }
 
   // The dependencies are already up-to-date.
@@ -1351,17 +1352,21 @@ bool cmLocalUnixMakefileGenerator3::UpdateDependencies(
 }
 
 bool cmLocalUnixMakefileGenerator3::ScanDependencies(
-  const std::string& targetDir,
+  std::string const& targetDir, std::string const& dependFile,
+  std::string const& internalDependFile,
   std::map<std::string, cmDepends::DependencyVector>& validDeps)
 {
   // Read the directory information file.
   cmMakefile* mf = this->Makefile;
   bool haveDirectoryInfo = false;
-  std::string dirInfoFile = this->GetCurrentBinaryDirectory();
-  dirInfoFile += "/CMakeFiles";
-  dirInfoFile += "/CMakeDirectoryInformation.cmake";
-  if (mf->ReadListFile(dirInfoFile) && !cmSystemTools::GetErrorOccuredFlag()) {
-    haveDirectoryInfo = true;
+  {
+    std::string dirInfoFile = this->GetCurrentBinaryDirectory();
+    dirInfoFile += "/CMakeFiles";
+    dirInfoFile += "/CMakeDirectoryInformation.cmake";
+    if (mf->ReadListFile(dirInfoFile) &&
+        !cmSystemTools::GetErrorOccuredFlag()) {
+      haveDirectoryInfo = true;
+    }
   }
 
   // Lookup useful directory information.
@@ -1390,10 +1395,8 @@ bool cmLocalUnixMakefileGenerator3::ScanDependencies(
 
   // Open the make depends file.  This should be copy-if-different
   // because the make tool may try to reload it needlessly otherwise.
-  std::string ruleFileNameFull = targetDir;
-  ruleFileNameFull += "/depend.make";
   cmGeneratedFileStream ruleFileStream(
-    ruleFileNameFull, false, this->GlobalGenerator->GetMakefileEncoding());
+    dependFile, false, this->GlobalGenerator->GetMakefileEncoding());
   ruleFileStream.SetCopyIfDifferent(true);
   if (!ruleFileStream) {
     return false;
@@ -1402,11 +1405,8 @@ bool cmLocalUnixMakefileGenerator3::ScanDependencies(
   // Open the cmake dependency tracking file.  This should not be
   // copy-if-different because dependencies are re-scanned when it is
   // older than the DependInfo.cmake.
-  std::string internalRuleFileNameFull = targetDir;
-  internalRuleFileNameFull += "/depend.internal";
   cmGeneratedFileStream internalRuleFileStream(
-    internalRuleFileNameFull, false,
-    this->GlobalGenerator->GetMakefileEncoding());
+    internalDependFile, false, this->GlobalGenerator->GetMakefileEncoding());
   if (!internalRuleFileStream) {
     return false;
   }
@@ -1415,26 +1415,25 @@ bool cmLocalUnixMakefileGenerator3::ScanDependencies(
   this->WriteDisclaimer(internalRuleFileStream);
 
   // for each language we need to scan, scan it
-  std::string const& langStr =
-    mf->GetSafeDefinition("CMAKE_DEPENDS_LANGUAGES");
   std::vector<std::string> langs;
-  cmSystemTools::ExpandListArgument(langStr, langs);
+  cmSystemTools::ExpandListArgument(
+    mf->GetSafeDefinition("CMAKE_DEPENDS_LANGUAGES"), langs);
   for (std::string const& lang : langs) {
     // construct the checker
     // Create the scanner for this language
-    cmDepends* scanner = nullptr;
+    std::unique_ptr<cmDepends> scanner;
     if (lang == "C" || lang == "CXX" || lang == "RC" || lang == "ASM" ||
         lang == "CUDA") {
       // TODO: Handle RC (resource files) dependencies correctly.
-      scanner = new cmDependsC(this, targetDir, lang, &validDeps);
+      scanner = cm::make_unique<cmDependsC>(this, targetDir, lang, &validDeps);
     }
 #ifdef CMAKE_BUILD_WITH_CMAKE
     else if (lang == "Fortran") {
       ruleFileStream << "# Note that incremental build could trigger "
                      << "a call to cmake_copy_f90_mod on each re-build\n";
-      scanner = new cmDependsFortran(this);
+      scanner = cm::make_unique<cmDependsFortran>(this);
     } else if (lang == "Java") {
-      scanner = new cmDependsJava();
+      scanner = cm::make_unique<cmDependsJava>();
     }
 #endif
 
@@ -1445,9 +1444,6 @@ bool cmLocalUnixMakefileGenerator3::ScanDependencies(
       scanner->SetLanguage(lang);
       scanner->SetTargetDirectory(targetDir);
       scanner->Write(ruleFileStream, internalRuleFileStream);
-
-      // free the scanner for this language
-      delete scanner;
     }
   }
 
diff --git a/Source/cmLocalUnixMakefileGenerator3.h b/Source/cmLocalUnixMakefileGenerator3.h
index ced2dbd..ba882da 100644
--- a/Source/cmLocalUnixMakefileGenerator3.h
+++ b/Source/cmLocalUnixMakefileGenerator3.h
@@ -231,7 +231,8 @@ protected:
 
   // Helper methods for dependency updates.
   bool ScanDependencies(
-    const std::string& targetDir,
+    std::string const& targetDir, std::string const& dependFile,
+    std::string const& internalDependFile,
     std::map<std::string, cmDepends::DependencyVector>& validDeps);
   void CheckMultipleOutputs(bool verbose);