From 946adadd404a160b708e96e668be9b84c949de69 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Fri, 2 Oct 2020 12:11:19 -0400 Subject: cmGccDepfileReader: Rework helper code Fix some of the semantics of the depfile, add error handling, and refactor cmGccDepfileLexerHelper. --- Source/LexerParser/cmGccDepfileLexer.cxx | 2 +- Source/LexerParser/cmGccDepfileLexer.in.l | 2 +- Source/cmGccDepfileLexerHelper.cxx | 33 ++++++++++++++-------- Source/cmGccDepfileLexerHelper.h | 3 +- Source/cmGccDepfileReader.cxx | 9 +++--- Source/cmGccDepfileReader.h | 4 ++- Source/cmQtAutoMocUic.cxx | 8 +++--- Tests/CMakeLib/testGccDepfileReader.cxx | 21 ++++++++++---- Tests/CMakeLib/testGccDepfileReader_data/deps4.d | 1 + Tests/CMakeLib/testGccDepfileReader_data/deps5.d | 0 Tests/CMakeLib/testGccDepfileReader_data/deps5.txt | 2 ++ Tests/CMakeLib/testGccDepfileReader_data/deps7.d | 6 ++++ Tests/CMakeLib/testGccDepfileReader_data/deps7.txt | 10 +++++++ 13 files changed, 73 insertions(+), 28 deletions(-) create mode 100644 Tests/CMakeLib/testGccDepfileReader_data/deps4.d create mode 100644 Tests/CMakeLib/testGccDepfileReader_data/deps5.d create mode 100644 Tests/CMakeLib/testGccDepfileReader_data/deps5.txt create mode 100644 Tests/CMakeLib/testGccDepfileReader_data/deps7.d create mode 100644 Tests/CMakeLib/testGccDepfileReader_data/deps7.txt diff --git a/Source/LexerParser/cmGccDepfileLexer.cxx b/Source/LexerParser/cmGccDepfileLexer.cxx index a98969d..3630f4e 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 08f8577..c83cb75 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 957896f..c782bcd 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 07ca61d..91132f5 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 9d70ede..eb3511a 100644 --- a/Source/cmGccDepfileReader.cxx +++ b/Source/cmGccDepfileReader.cxx @@ -5,14 +5,15 @@ #include #include +#include + #include "cmGccDepfileLexerHelper.h" -cmGccDepfileContent cmReadGccDepfile(const char* filePath) +cm::optional 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 395dd77..59ed7fd 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 + #include "cmGccDepfileReaderTypes.h" -cmGccDepfileContent cmReadGccDepfile(const char* filePath); +cm::optional cmReadGccDepfile(const char* filePath); diff --git a/Source/cmQtAutoMocUic.cxx b/Source/cmQtAutoMocUic.cxx index 9cb172b..b27bb88 100644 --- a/Source/cmQtAutoMocUic.cxx +++ b/Source/cmQtAutoMocUic.cxx @@ -15,6 +15,7 @@ #include #include +#include #include #include @@ -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 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 e79f047..d46e8f3 100644 --- a/Tests/CMakeLib/testGccDepfileReader.cxx +++ b/Tests/CMakeLib/testGccDepfileReader.cxx @@ -5,6 +5,8 @@ #include #include +#include + #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 0000000..9977a28 --- /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 0000000..e69de29 diff --git a/Tests/CMakeLib/testGccDepfileReader_data/deps5.txt b/Tests/CMakeLib/testGccDepfileReader_data/deps5.txt new file mode 100644 index 0000000..6c4a75b --- /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 0000000..92280cf --- /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 0000000..86b6600 --- /dev/null +++ b/Tests/CMakeLib/testGccDepfileReader_data/deps7.txt @@ -0,0 +1,10 @@ +--RULES-- +out1 +out2 +--DEPENDENCIES-- +in1 +in2 +--RULES-- +out3 +--DEPENDENCIES-- +in3 -- cgit v0.12