cmGccDepfileReader: Rework helper code

Fix some of the semantics of the depfile, add error handling, and
refactor cmGccDepfileLexerHelper.

13 files changed, 73 insertions, 28 deletions

diff --git a/Source/LexerParser/cmGccDepfileLexer.cxx b/Source/LexerParser/cmGccDepfileLexer.cxx
index a98969d591..3630f4ef8e 100644
--- a/Source/LexerParser/cmGccDepfileLexer.cxx
+++ b/Source/LexerParser/cmGccDepfileLexer.cxx
@@ -994,7 +994,7 @@ case 5:
 YY_RULE_SETUP
 {
                          // A line continuation ends the current file name.
-                         yyextra->newDependency();
+                         yyextra->newRuleOrDependency();
                        }
 	YY_BREAK
 case 6:
diff --git a/Source/LexerParser/cmGccDepfileLexer.in.l b/Source/LexerParser/cmGccDepfileLexer.in.l
index 08f8577fc2..c83cb75e81 100644
--- a/Source/LexerParser/cmGccDepfileLexer.in.l
+++ b/Source/LexerParser/cmGccDepfileLexer.in.l
@@ -42,7 +42,7 @@ NEWLINE \r?\n
                        }
 {WSPACE}*\\{NEWLINE}   {
                          // A line continuation ends the current file name.
-                         yyextra->newDependency();
+                         yyextra->newRuleOrDependency();
                        }
 {NEWLINE}              {
                          // A newline ends the current file name and the current rule.
diff --git a/Source/cmGccDepfileLexerHelper.cxx b/Source/cmGccDepfileLexerHelper.cxx
index 957896f521..c782bcd0ae 100644
--- a/Source/cmGccDepfileLexerHelper.cxx
+++ b/Source/cmGccDepfileLexerHelper.cxx
@@ -27,23 +27,30 @@ bool cmGccDepfileLexerHelper::readFile(const char* filePath)
   if (!file) {
     return false;
   }
-  newEntry();
+  this->newEntry();
   yyscan_t scanner;
   cmGccDepfile_yylex_init(&scanner);
   cmGccDepfile_yyset_extra(this, scanner);
   cmGccDepfile_yyrestart(file, scanner);
   cmGccDepfile_yylex(scanner);
   cmGccDepfile_yylex_destroy(scanner);
-  sanitizeContent();
+  this->sanitizeContent();
   fclose(file);
-  return true;
+  return this->HelperState != State::Failed;
 }
 
 void cmGccDepfileLexerHelper::newEntry()
 {
+  if (this->HelperState == State::Rule && !this->Content.empty()) {
+    if (!this->Content.back().rules.empty() &&
+        !this->Content.back().rules.back().empty()) {
+      this->HelperState = State::Failed;
+    }
+    return;
+  }
   this->HelperState = State::Rule;
   this->Content.emplace_back();
-  newRule();
+  this->newRule();
 }
 
 void cmGccDepfileLexerHelper::newRule()
@@ -56,20 +63,22 @@ void cmGccDepfileLexerHelper::newRule()
 
 void cmGccDepfileLexerHelper::newDependency()
 {
-  // printf("NEW DEP\n");
+  if (this->HelperState == State::Failed) {
+    return;
+  }
   this->HelperState = State::Dependency;
-  if (this->Content.back().paths.empty() ||
-      !this->Content.back().paths.back().empty()) {
-    this->Content.back().paths.emplace_back();
+  auto& entry = this->Content.back();
+  if (entry.paths.empty() || !entry.paths.back().empty()) {
+    entry.paths.emplace_back();
   }
 }
 
 void cmGccDepfileLexerHelper::newRuleOrDependency()
 {
   if (this->HelperState == State::Rule) {
-    newRule();
-  } else {
-    newDependency();
+    this->newRule();
+  } else if (this->HelperState == State::Dependency) {
+    this->newDependency();
   }
 }
 
@@ -93,6 +102,8 @@ void cmGccDepfileLexerHelper::addToCurrentPath(const char* s)
       }
       dst = &dep->paths.back();
     } break;
+    case State::Failed:
+      return;
   }
   dst->append(s);
 }
diff --git a/Source/cmGccDepfileLexerHelper.h b/Source/cmGccDepfileLexerHelper.h
index 07ca61d181..91132f5d06 100644
--- a/Source/cmGccDepfileLexerHelper.h
+++ b/Source/cmGccDepfileLexerHelper.h
@@ -29,7 +29,8 @@ private:
   enum class State
   {
     Rule,
-    Dependency
+    Dependency,
+    Failed,
   };
   State HelperState = State::Rule;
 };
diff --git a/Source/cmGccDepfileReader.cxx b/Source/cmGccDepfileReader.cxx
index 9d70edec54..eb3511a53b 100644
--- a/Source/cmGccDepfileReader.cxx
+++ b/Source/cmGccDepfileReader.cxx
@@ -5,14 +5,15 @@
 #include <type_traits>
 #include <utility>
 
+#include <cm/optional>
+
 #include "cmGccDepfileLexerHelper.h"
 
-cmGccDepfileContent cmReadGccDepfile(const char* filePath)
+cm::optional<cmGccDepfileContent> cmReadGccDepfile(const char* filePath)
 {
-  cmGccDepfileContent result;
   cmGccDepfileLexerHelper helper;
   if (helper.readFile(filePath)) {
-    result = std::move(helper).extractContent();
+    return cm::make_optional(std::move(helper).extractContent());
   }
-  return result;
+  return cm::nullopt;
 }
diff --git a/Source/cmGccDepfileReader.h b/Source/cmGccDepfileReader.h
index 395dd779a3..59ed7fd73a 100644
--- a/Source/cmGccDepfileReader.h
+++ b/Source/cmGccDepfileReader.h
@@ -2,6 +2,8 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #pragma once
 
+#include <cm/optional>
+
 #include "cmGccDepfileReaderTypes.h"
 
-cmGccDepfileContent cmReadGccDepfile(const char* filePath);
+cm::optional<cmGccDepfileContent> cmReadGccDepfile(const char* filePath);
diff --git a/Source/cmQtAutoMocUic.cxx b/Source/cmQtAutoMocUic.cxx
index 9cb172bd35..b27bb88f7e 100644
--- a/Source/cmQtAutoMocUic.cxx
+++ b/Source/cmQtAutoMocUic.cxx
@@ -15,6 +15,7 @@
 #include <vector>
 
 #include <cm/memory>
+#include <cm/optional>
 #include <cm/string_view>
 #include <cmext/algorithm>
 
@@ -26,7 +27,6 @@
 #include "cmCryptoHash.h"
 #include "cmFileTime.h"
 #include "cmGccDepfileReader.h"
-#include "cmGccDepfileReaderTypes.h"
 #include "cmGeneratedFileStream.h"
 #include "cmQtAutoGen.h"
 #include "cmQtAutoGenerator.h"
@@ -2841,14 +2841,14 @@ bool cmQtAutoMocUicT::CreateDirectories()
 std::vector<std::string> cmQtAutoMocUicT::dependenciesFromDepFile(
   const char* filePath)
 {
-  cmGccDepfileContent content = cmReadGccDepfile(filePath);
-  if (content.empty()) {
+  auto const content = cmReadGccDepfile(filePath);
+  if (!content || content->empty()) {
     return {};
   }
 
   // Moc outputs a depfile with exactly one rule.
   // Discard the rule and return the dependencies.
-  return content.front().paths;
+  return content->front().paths;
 }
 
 void cmQtAutoMocUicT::Abort(bool error)
diff --git a/Tests/CMakeLib/testGccDepfileReader.cxx b/Tests/CMakeLib/testGccDepfileReader.cxx
index e79f047fd6..d46e8f3fe0 100644
--- a/Tests/CMakeLib/testGccDepfileReader.cxx
+++ b/Tests/CMakeLib/testGccDepfileReader.cxx
@@ -5,6 +5,8 @@
 #include <utility>
 #include <vector>
 
+#include <cm/optional>
+
 #include "cmsys/FStream.hxx"
 
 #include "cmGccDepfileReader.h"
@@ -112,17 +114,26 @@ int testGccDepfileReader(int argc, char* argv[])
 
   std::string dataDirPath = argv[1];
   dataDirPath += "/testGccDepfileReader_data";
-  const int numberOfTestFiles = 3;
+  const int numberOfTestFiles = 7; // 6th file doesn't exist
   for (int i = 1; i <= numberOfTestFiles; ++i) {
     const std::string base = dataDirPath + "/deps" + std::to_string(i);
     const std::string depfile = base + ".d";
     const std::string plainDepfile = base + ".txt";
     std::cout << "Comparing " << base << " with " << plainDepfile << std::endl;
     const auto actual = cmReadGccDepfile(depfile.c_str());
-    const auto expected = readPlainDepfile(plainDepfile.c_str());
-    if (!compare(actual, expected)) {
-      dump("actual", actual);
-      dump("expected", expected);
+    if (cmSystemTools::FileExists(plainDepfile)) {
+      if (!actual) {
+        std::cerr << "Reading " << depfile << " should have succeeded\n";
+        return 1;
+      }
+      const auto expected = readPlainDepfile(plainDepfile.c_str());
+      if (!compare(*actual, expected)) {
+        dump("actual", *actual);
+        dump("expected", expected);
+        return 1;
+      }
+    } else if (actual) {
+      std::cerr << "Reading " << depfile << " should have failed\n";
       return 1;
     }
   }
diff --git a/Tests/CMakeLib/testGccDepfileReader_data/deps4.d b/Tests/CMakeLib/testGccDepfileReader_data/deps4.d
new file mode 100644
index 0000000000..9977a2836c
--- /dev/null
+++ b/Tests/CMakeLib/testGccDepfileReader_data/deps4.d
@@ -0,0 +1 @@
+invalid
diff --git a/Tests/CMakeLib/testGccDepfileReader_data/deps5.d b/Tests/CMakeLib/testGccDepfileReader_data/deps5.d
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/Tests/CMakeLib/testGccDepfileReader_data/deps5.d
diff --git a/Tests/CMakeLib/testGccDepfileReader_data/deps5.txt b/Tests/CMakeLib/testGccDepfileReader_data/deps5.txt
new file mode 100644
index 0000000000..6c4a75bc9d
--- /dev/null
+++ b/Tests/CMakeLib/testGccDepfileReader_data/deps5.txt
@@ -0,0 +1,2 @@
+--RULES--
+--DEPENDENCIES--
diff --git a/Tests/CMakeLib/testGccDepfileReader_data/deps7.d b/Tests/CMakeLib/testGccDepfileReader_data/deps7.d
new file mode 100644
index 0000000000..92280cf822
--- /dev/null
+++ b/Tests/CMakeLib/testGccDepfileReader_data/deps7.d
@@ -0,0 +1,6 @@
+out1 \
+  out2: \
+  in1 \
+  in2
+
+out3: in3
diff --git a/Tests/CMakeLib/testGccDepfileReader_data/deps7.txt b/Tests/CMakeLib/testGccDepfileReader_data/deps7.txt
new file mode 100644
index 0000000000..86b6600d0c
--- /dev/null
+++ b/Tests/CMakeLib/testGccDepfileReader_data/deps7.txt
@@ -0,0 +1,10 @@
+--RULES--
+out1
+out2
+--DEPENDENCIES--
+in1
+in2
+--RULES--
+out3
+--DEPENDENCIES--
+in3