cmDepends: Refactor cmDepends::CheckDependencies method

This patch changes the following issues in `cmDepends::CheckDependencies`:

- Use the `std::string` based `std::getline` interface to read lines from a
  file instead of using raw reads into raw buffers.
- To reduce the file system access, we load file times only once from
  `cmFileTimeCache` and keep them on the stack for later comparison.
- When a file is removed from the file system we remove it from the
  `cmFileTimeCache` as well.

1 files changed, 71 insertions, 73 deletions

diff --git a/Source/cmDepends.cxx b/Source/cmDepends.cxx
index efadaf1123..334f1e56dc 100644
--- a/Source/cmDepends.cxx
+++ b/Source/cmDepends.cxx
@@ -2,6 +2,7 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmDepends.h"
 
+#include "cmFileTime.h"
 #include "cmFileTimeCache.h"
 #include "cmGeneratedFileStream.h"
 #include "cmLocalGenerator.h"
@@ -10,22 +11,15 @@
 
 #include "cmsys/FStream.hxx"
 #include <sstream>
-#include <string.h>
 #include <utility>
 
 cmDepends::cmDepends(cmLocalGenerator* lg, std::string targetDir)
   : LocalGenerator(lg)
   , TargetDirectory(std::move(targetDir))
-  , Dependee(new char[MaxPath])
-  , Depender(new char[MaxPath])
 {
 }
 
-cmDepends::~cmDepends()
-{
-  delete[] this->Dependee;
-  delete[] this->Depender;
-}
+cmDepends::~cmDepends() = default;
 
 bool cmDepends::Write(std::ostream& makeDepends, std::ostream& internalDepends)
 {
@@ -76,9 +70,9 @@ bool cmDepends::Check(const std::string& makeFile,
     // Clear all dependencies so they will be regenerated.
     this->Clear(makeFile);
     cmSystemTools::RemoveFile(internalFile);
+    this->FileTimeCache->Remove(internalFile);
     okay = false;
   }
-
   return okay;
 }
 
@@ -111,44 +105,58 @@ bool cmDepends::CheckDependencies(
   std::istream& internalDepends, const std::string& internalDependsFileName,
   std::map<std::string, DependencyVector>& validDeps)
 {
+  // Read internal depends file time
+  cmFileTime internalDependsTime;
+  if (!this->FileTimeCache->Load(internalDependsFileName,
+                                 internalDependsTime)) {
+    return false;
+  }
+
   // Parse dependencies from the stream.  If any dependee is missing
   // or newer than the depender then dependencies should be
   // regenerated.
   bool okay = true;
   bool dependerExists = false;
+
+  std::string line;
+  line.reserve(1024);
+  std::string depender;
+  std::string dependee;
+  cmFileTime dependerTime;
+  cmFileTime dependeeTime;
   DependencyVector* currentDependencies = nullptr;
 
-  while (internalDepends.getline(this->Dependee, this->MaxPath)) {
-    if (this->Dependee[0] == 0 || this->Dependee[0] == '#' ||
-        this->Dependee[0] == '\r') {
+  while (std::getline(internalDepends, line)) {
+    // Check if this an empty or a comment line
+    if (line.empty() || line.front() == '#') {
       continue;
     }
-    size_t len = internalDepends.gcount() - 1;
-    if (this->Dependee[len - 1] == '\r') {
-      len--;
-      this->Dependee[len] = 0;
+    // Drop carriage return character at the end
+    if (line.back() == '\r') {
+      line.pop_back();
+      if (line.empty()) {
+        continue;
+      }
     }
-    if (this->Dependee[0] != ' ') {
-      memcpy(this->Depender, this->Dependee, len + 1);
-      // Calling FileExists() for the depender here saves in many cases 50%
-      // of the calls to FileExists() further down in the loop. E.g. for
-      // kdelibs/khtml this reduces the number of calls from 184k down to 92k,
-      // or the time for cmake -E cmake_depends from 0.3 s down to 0.21 s.
-      dependerExists = cmSystemTools::FileExists(this->Depender);
+    // Check if this a depender line
+    if (line.front() != ' ') {
+      depender = line;
+      dependerExists = this->FileTimeCache->Load(depender, dependerTime);
       // If we erase validDeps[this->Depender] by overwriting it with an empty
       // vector, we lose dependencies for dependers that have multiple
       // entries. No need to initialize the entry, std::map will do so on first
       // access.
-      currentDependencies = &validDeps[this->Depender];
+      currentDependencies = &validDeps[depender];
       continue;
     }
-    /*
-    // Parse the dependency line.
-    if(!this->ParseDependency(line.c_str()))
-      {
-      continue;
-      }
-      */
+
+    // This is a dependee line
+    dependee = line.substr(1);
+
+    // Add dependee to depender's list
+    if (currentDependencies != nullptr) {
+      currentDependencies->push_back(dependee);
+    }
 
     // Dependencies must be regenerated
     // * if the dependee does not exist
@@ -156,13 +164,8 @@ bool cmDepends::CheckDependencies(
     // * if the depender does not exist, but the dependee is newer than the
     //   depends file
     bool regenerate = false;
-    const std::string dependee(this->Dependee + 1);
-    const std::string depender(this->Depender);
-    if (currentDependencies != nullptr) {
-      currentDependencies->push_back(dependee);
-    }
-
-    if (!cmSystemTools::FileExists(dependee)) {
+    bool dependeeExists = this->FileTimeCache->Load(dependee, dependeeTime);
+    if (!dependeeExists) {
       // The dependee does not exist.
       regenerate = true;
 
@@ -173,44 +176,38 @@ bool cmDepends::CheckDependencies(
             << depender << "\"." << std::endl;
         cmSystemTools::Stdout(msg.str());
       }
-    } else {
-      if (dependerExists) {
-        // The dependee and depender both exist.  Compare file times.
-        int result = 0;
-        if ((!this->FileTimeCache->Compare(depender, dependee, &result) ||
-             result < 0)) {
-          // The depender is older than the dependee.
-          regenerate = true;
-
-          // Print verbose output.
-          if (this->Verbose) {
-            std::ostringstream msg;
-            msg << "Dependee \"" << dependee << "\" is newer than depender \""
-                << depender << "\"." << std::endl;
-            cmSystemTools::Stdout(msg.str());
-          }
+    } else if (dependerExists) {
+      // The dependee and depender both exist.  Compare file times.
+      if (dependerTime.Older(dependeeTime)) {
+        // The depender is older than the dependee.
+        regenerate = true;
+
+        // Print verbose output.
+        if (this->Verbose) {
+          std::ostringstream msg;
+          msg << "Dependee \"" << dependee << "\" is newer than depender \""
+              << depender << "\"." << std::endl;
+          cmSystemTools::Stdout(msg.str());
         }
-      } else {
-        // The dependee exists, but the depender doesn't. Regenerate if the
-        // internalDepends file is older than the dependee.
-        int result = 0;
-        if ((!this->FileTimeCache->Compare(internalDependsFileName, dependee,
-                                           &result) ||
-             result < 0)) {
-          // The depends-file is older than the dependee.
-          regenerate = true;
-
-          // Print verbose output.
-          if (this->Verbose) {
-            std::ostringstream msg;
-            msg << "Dependee \"" << dependee
-                << "\" is newer than depends file \""
-                << internalDependsFileName << "\"." << std::endl;
-            cmSystemTools::Stdout(msg.str());
-          }
+      }
+    } else {
+      // The dependee exists, but the depender doesn't. Regenerate if the
+      // internalDepends file is older than the dependee.
+      if (internalDependsTime.Older(dependeeTime)) {
+        // The depends-file is older than the dependee.
+        regenerate = true;
+
+        // Print verbose output.
+        if (this->Verbose) {
+          std::ostringstream msg;
+          msg << "Dependee \"" << dependee
+              << "\" is newer than depends file \"" << internalDependsFileName
+              << "\"." << std::endl;
+          cmSystemTools::Stdout(msg.str());
         }
       }
     }
+
     if (regenerate) {
       // Dependencies must be regenerated.
       okay = false;
@@ -218,13 +215,14 @@ bool cmDepends::CheckDependencies(
       // Remove the information of this depender from the map, it needs
       // to be rescanned
       if (currentDependencies != nullptr) {
-        validDeps.erase(this->Depender);
+        validDeps.erase(depender);
         currentDependencies = nullptr;
       }
 
       // Remove the depender to be sure it is rebuilt.
       if (dependerExists) {
         cmSystemTools::RemoveFile(depender);
+        this->FileTimeCache->Remove(depender);
         dependerExists = false;
       }
     }