From 4bedf6c9fa3f55a19b09d5ab26cfd149ee2e13e6 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Sun, 6 Oct 2019 00:23:17 +0200 Subject: Refactor: Modernize `cmVariableWatchCommand` a little --- Source/cmVariableWatch.cxx | 14 ++--- Source/cmVariableWatch.h | 4 +- Source/cmVariableWatchCommand.cxx | 65 +++++++++++----------- .../variable_watch-default-script-stderr.txt | 4 +- .../variable_watch-default-stderr.txt | 4 +- .../variable_watch-invalid-var-script-stderr.txt | 4 +- .../variable_watch-invalid-var-stderr.txt | 4 +- .../variable_watch-var-script-stderr.txt | 10 ++-- .../variable_watch-var-stderr.txt | 10 ++-- 9 files changed, 58 insertions(+), 61 deletions(-) diff --git a/Source/cmVariableWatch.cxx b/Source/cmVariableWatch.cxx index 4995da9..35e1c8c 100644 --- a/Source/cmVariableWatch.cxx +++ b/Source/cmVariableWatch.cxx @@ -2,19 +2,19 @@ file Copyright.txt or https://cmake.org/licensing for details. */ #include "cmVariableWatch.h" +#include #include #include #include -static const char* const cmVariableWatchAccessStrings[] = { - "READ_ACCESS", "UNKNOWN_READ_ACCESS", "UNKNOWN_DEFINED_ACCESS", - "MODIFIED_ACCESS", "REMOVED_ACCESS", "NO_ACCESS" -}; - -const char* cmVariableWatch::GetAccessAsString(int access_type) +const std::string& cmVariableWatch::GetAccessAsString(int access_type) { + static const std::array cmVariableWatchAccessStrings = { + { "READ_ACCESS", "UNKNOWN_READ_ACCESS", "UNKNOWN_DEFINED_ACCESS", + "MODIFIED_ACCESS", "REMOVED_ACCESS", "NO_ACCESS" } + }; if (access_type < 0 || access_type >= cmVariableWatch::NO_ACCESS) { - return "NO_ACCESS"; + access_type = cmVariableWatch::NO_ACCESS; } return cmVariableWatchAccessStrings[access_type]; } diff --git a/Source/cmVariableWatch.h b/Source/cmVariableWatch.h index e4b3b7c..6c418ed 100644 --- a/Source/cmVariableWatch.h +++ b/Source/cmVariableWatch.h @@ -46,7 +46,7 @@ public: */ enum { - VARIABLE_READ_ACCESS = 0, + VARIABLE_READ_ACCESS, UNKNOWN_VARIABLE_READ_ACCESS, UNKNOWN_VARIABLE_DEFINED_ACCESS, VARIABLE_MODIFIED_ACCESS, @@ -57,7 +57,7 @@ public: /** * Return the access as string */ - static const char* GetAccessAsString(int access_type); + static const std::string& GetAccessAsString(int access_type); protected: struct Pair diff --git a/Source/cmVariableWatchCommand.cxx b/Source/cmVariableWatchCommand.cxx index f2c8f3c..039f1ba 100644 --- a/Source/cmVariableWatchCommand.cxx +++ b/Source/cmVariableWatchCommand.cxx @@ -2,6 +2,7 @@ file Copyright.txt or https://cmake.org/licensing for details. */ #include "cmVariableWatchCommand.h" +#include #include #include @@ -14,17 +15,17 @@ #include "cmVariableWatch.h" #include "cmake.h" +namespace { struct cmVariableWatchCallbackData { bool InCallback; std::string Command; }; -static void cmVariableWatchCommandVariableAccessed(const std::string& variable, - int access_type, - void* client_data, - const char* newValue, - const cmMakefile* mf) +void cmVariableWatchCommandVariableAccessed(const std::string& variable, + int access_type, void* client_data, + const char* newValue, + const cmMakefile* mf) { cmVariableWatchCallbackData* data = static_cast(client_data); @@ -34,40 +35,35 @@ static void cmVariableWatchCommandVariableAccessed(const std::string& variable, } data->InCallback = true; - cmListFileFunction newLFF; - cmListFileArgument arg; - bool processed = false; - const char* accessString = cmVariableWatch::GetAccessAsString(access_type); - const char* currentListFile = mf->GetDefinition("CMAKE_CURRENT_LIST_FILE"); + auto accessString = cmVariableWatch::GetAccessAsString(access_type); /// Ultra bad!! cmMakefile* makefile = const_cast(mf); std::string stack = makefile->GetProperty("LISTFILE_STACK"); if (!data->Command.empty()) { - newLFF.Arguments.clear(); - newLFF.Arguments.emplace_back(variable, cmListFileArgument::Quoted, 9999); - newLFF.Arguments.emplace_back(accessString, cmListFileArgument::Quoted, - 9999); - newLFF.Arguments.emplace_back(newValue ? newValue : "", - cmListFileArgument::Quoted, 9999); - newLFF.Arguments.emplace_back(currentListFile, cmListFileArgument::Quoted, - 9999); - newLFF.Arguments.emplace_back(stack, cmListFileArgument::Quoted, 9999); + cmListFileFunction newLFF; + const char* const currentListFile = + mf->GetDefinition("CMAKE_CURRENT_LIST_FILE"); + const auto fakeLineNo = + std::numeric_limits::max(); + newLFF.Arguments = { + { variable, cmListFileArgument::Quoted, fakeLineNo }, + { accessString, cmListFileArgument::Quoted, fakeLineNo }, + { newValue ? newValue : "", cmListFileArgument::Quoted, fakeLineNo }, + { currentListFile, cmListFileArgument::Quoted, fakeLineNo }, + { stack, cmListFileArgument::Quoted, fakeLineNo } + }; newLFF.Name = data->Command; - newLFF.Line = 9999; + newLFF.Line = fakeLineNo; cmExecutionStatus status(*makefile); if (!makefile->ExecuteCommand(newLFF, status)) { cmSystemTools::Error( cmStrCat("Error in cmake code at\nUnknown:0:\nA command failed " "during the invocation of callback \"", data->Command, "\".")); - data->InCallback = false; - return; } - processed = true; - } - if (!processed) { + } else { makefile->IssueMessage( MessageType::LOG, cmStrCat("Variable \"", variable, "\" was accessed using ", accessString, @@ -77,7 +73,7 @@ static void cmVariableWatchCommandVariableAccessed(const std::string& variable, data->InCallback = false; } -static void deleteVariableWatchCallbackData(void* client_data) +void deleteVariableWatchCallbackData(void* client_data) { cmVariableWatchCallbackData* data = static_cast(client_data); @@ -91,7 +87,7 @@ class FinalAction public: /* NOLINTNEXTLINE(performance-unnecessary-value-param) */ FinalAction(cmMakefile* makefile, std::string variable) - : Action(std::make_shared(makefile, std::move(variable))) + : Action{ std::make_shared(makefile, std::move(variable)) } { } @@ -101,8 +97,8 @@ private: struct Impl { Impl(cmMakefile* makefile, std::string variable) - : Makefile(makefile) - , Variable(std::move(variable)) + : Makefile{ makefile } + , Variable{ std::move(variable) } { } @@ -112,12 +108,13 @@ private: this->Variable, cmVariableWatchCommandVariableAccessed); } - cmMakefile* Makefile; - std::string Variable; + cmMakefile* const Makefile; + std::string const Variable; }; std::shared_ptr Action; }; +} // anonymous namespace bool cmVariableWatchCommand(std::vector const& args, cmExecutionStatus& status) @@ -136,10 +133,10 @@ bool cmVariableWatchCommand(std::vector const& args, return false; } - cmVariableWatchCallbackData* data = new cmVariableWatchCallbackData; + auto* const data = new cmVariableWatchCallbackData; data->InCallback = false; - data->Command = command; + data->Command = std::move(command); if (!status.GetMakefile().GetCMakeInstance()->GetVariableWatch()->AddWatch( variable, cmVariableWatchCommandVariableAccessed, data, @@ -149,6 +146,6 @@ bool cmVariableWatchCommand(std::vector const& args, } status.GetMakefile().AddFinalAction( - FinalAction(&status.GetMakefile(), variable)); + FinalAction{ &status.GetMakefile(), variable }); return true; } diff --git a/Tests/RunCMake/MaxRecursionDepth/variable_watch-default-script-stderr.txt b/Tests/RunCMake/MaxRecursionDepth/variable_watch-default-script-stderr.txt index 4dddc96..07deee2 100644 --- a/Tests/RunCMake/MaxRecursionDepth/variable_watch-default-script-stderr.txt +++ b/Tests/RunCMake/MaxRecursionDepth/variable_watch-default-script-stderr.txt @@ -1,6 +1,6 @@ [0-9]+ -CMake Error at .*/variable_watch\.cmake:9999 \(update_x\): +CMake Error at .*/variable_watch\.cmake:[0-9]+ \(update_x\): Maximum recursion depth of [0-9]+ exceeded Call Stack \(most recent call first\): .*/variable_watch\.cmake:5 \(set\) - .*/variable_watch\.cmake:9999 \(update_x\) + .*/variable_watch\.cmake:[0-9]+ \(update_x\) diff --git a/Tests/RunCMake/MaxRecursionDepth/variable_watch-default-stderr.txt b/Tests/RunCMake/MaxRecursionDepth/variable_watch-default-stderr.txt index a8b4756..b2395b3 100644 --- a/Tests/RunCMake/MaxRecursionDepth/variable_watch-default-stderr.txt +++ b/Tests/RunCMake/MaxRecursionDepth/variable_watch-default-stderr.txt @@ -1,6 +1,6 @@ [0-9]+ -CMake Error at variable_watch\.cmake:9999 \(update_x\): +CMake Error at variable_watch\.cmake:[0-9]+ \(update_x\): Maximum recursion depth of [0-9]+ exceeded Call Stack \(most recent call first\): variable_watch\.cmake:5 \(set\) - variable_watch\.cmake:9999 \(update_x\) + variable_watch\.cmake:[0-9]+ \(update_x\) diff --git a/Tests/RunCMake/MaxRecursionDepth/variable_watch-invalid-var-script-stderr.txt b/Tests/RunCMake/MaxRecursionDepth/variable_watch-invalid-var-script-stderr.txt index 4dddc96..07deee2 100644 --- a/Tests/RunCMake/MaxRecursionDepth/variable_watch-invalid-var-script-stderr.txt +++ b/Tests/RunCMake/MaxRecursionDepth/variable_watch-invalid-var-script-stderr.txt @@ -1,6 +1,6 @@ [0-9]+ -CMake Error at .*/variable_watch\.cmake:9999 \(update_x\): +CMake Error at .*/variable_watch\.cmake:[0-9]+ \(update_x\): Maximum recursion depth of [0-9]+ exceeded Call Stack \(most recent call first\): .*/variable_watch\.cmake:5 \(set\) - .*/variable_watch\.cmake:9999 \(update_x\) + .*/variable_watch\.cmake:[0-9]+ \(update_x\) diff --git a/Tests/RunCMake/MaxRecursionDepth/variable_watch-invalid-var-stderr.txt b/Tests/RunCMake/MaxRecursionDepth/variable_watch-invalid-var-stderr.txt index a8b4756..b2395b3 100644 --- a/Tests/RunCMake/MaxRecursionDepth/variable_watch-invalid-var-stderr.txt +++ b/Tests/RunCMake/MaxRecursionDepth/variable_watch-invalid-var-stderr.txt @@ -1,6 +1,6 @@ [0-9]+ -CMake Error at variable_watch\.cmake:9999 \(update_x\): +CMake Error at variable_watch\.cmake:[0-9]+ \(update_x\): Maximum recursion depth of [0-9]+ exceeded Call Stack \(most recent call first\): variable_watch\.cmake:5 \(set\) - variable_watch\.cmake:9999 \(update_x\) + variable_watch\.cmake:[0-9]+ \(update_x\) diff --git a/Tests/RunCMake/MaxRecursionDepth/variable_watch-var-script-stderr.txt b/Tests/RunCMake/MaxRecursionDepth/variable_watch-var-script-stderr.txt index 00b2b3c..52fedd3 100644 --- a/Tests/RunCMake/MaxRecursionDepth/variable_watch-var-script-stderr.txt +++ b/Tests/RunCMake/MaxRecursionDepth/variable_watch-var-script-stderr.txt @@ -2,17 +2,17 @@ 6 8 10 -CMake Error at .*/variable_watch\.cmake:9999 \(update_x\): +CMake Error at .*/variable_watch\.cmake:[0-9]+ \(update_x\): Maximum recursion depth of 10 exceeded Call Stack \(most recent call first\): .*/variable_watch\.cmake:5 \(set\) - .*/variable_watch\.cmake:9999 \(update_x\) + .*/variable_watch\.cmake:[0-9]+ \(update_x\) .*/variable_watch\.cmake:5 \(set\) - .*/variable_watch\.cmake:9999 \(update_x\) + .*/variable_watch\.cmake:[0-9]+ \(update_x\) .*/variable_watch\.cmake:5 \(set\) - .*/variable_watch\.cmake:9999 \(update_x\) + .*/variable_watch\.cmake:[0-9]+ \(update_x\) .*/variable_watch\.cmake:5 \(set\) - .*/variable_watch\.cmake:9999 \(update_x\) + .*/variable_watch\.cmake:[0-9]+ \(update_x\) .*/variable_watch\.cmake:9 \(set\) .*/CMakeLists\.txt:5 \(include\) diff --git a/Tests/RunCMake/MaxRecursionDepth/variable_watch-var-stderr.txt b/Tests/RunCMake/MaxRecursionDepth/variable_watch-var-stderr.txt index 8f27bf1..1427f1d 100644 --- a/Tests/RunCMake/MaxRecursionDepth/variable_watch-var-stderr.txt +++ b/Tests/RunCMake/MaxRecursionDepth/variable_watch-var-stderr.txt @@ -2,17 +2,17 @@ 6 8 10 -CMake Error at variable_watch\.cmake:9999 \(update_x\): +CMake Error at variable_watch\.cmake:[0-9]+ \(update_x\): Maximum recursion depth of 10 exceeded Call Stack \(most recent call first\): variable_watch\.cmake:5 \(set\) - variable_watch\.cmake:9999 \(update_x\) + variable_watch\.cmake:[0-9]+ \(update_x\) variable_watch\.cmake:5 \(set\) - variable_watch\.cmake:9999 \(update_x\) + variable_watch\.cmake:[0-9]+ \(update_x\) variable_watch\.cmake:5 \(set\) - variable_watch\.cmake:9999 \(update_x\) + variable_watch\.cmake:[0-9]+ \(update_x\) variable_watch\.cmake:5 \(set\) - variable_watch\.cmake:9999 \(update_x\) + variable_watch\.cmake:[0-9]+ \(update_x\) variable_watch\.cmake:9 \(set\) CMakeLists\.txt:5 \(include\) -- cgit v0.12