From df6b077625f86e4ec0d534f6cd88f8610c5b8f8a Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 29 Jun 2020 16:45:43 -0400 Subject: cmake: Remove broken '--warn-unused-vars' option This option has been broken since commit b9f9915516 (cmMakefile: Remove VarUsageStack., 2015-05-17, v3.3.0-rc1~52^2). That commit removed the check that an initialized variable has actually been used and caused the option to warn on every variable ever set. This was not caught by the test suite because the test for the feature only checked that warnings appear when needed and not that they do not appear when not needed. The option was never very practical to use. Remove it to avoid the runtime cost of usage tracking and checks for every variable (which we were doing even when the option was not used). --- Help/manual/cmake.1.rst | 6 ++-- Help/release/dev/remove-warn-unused-vars.rst | 6 ++++ Source/QtDialog/CMakeSetupDialog.cxx | 6 ---- Source/QtDialog/CMakeSetupDialog.h | 1 - Source/QtDialog/QCMake.cxx | 7 ----- Source/QtDialog/QCMake.h | 4 --- Source/cmDefinitions.cxx | 14 --------- Source/cmDefinitions.h | 4 --- Source/cmMakefile.cxx | 44 ++-------------------------- Source/cmMakefile.h | 7 ----- Source/cmServerProtocol.cxx | 5 ++-- Source/cmServerProtocol.h | 1 + Source/cmStateSnapshot.cxx | 5 ---- Source/cmStateSnapshot.h | 1 - Source/cmake.cxx | 3 +- Source/cmake.h | 3 -- Source/cmakemain.cxx | 1 - Tests/CMakeLists.txt | 30 ------------------- Tests/VariableUnusedViaSet/CMakeLists.txt | 4 --- Tests/VariableUnusedViaUnset/CMakeLists.txt | 8 ----- 20 files changed, 16 insertions(+), 144 deletions(-) create mode 100644 Help/release/dev/remove-warn-unused-vars.rst delete mode 100644 Tests/VariableUnusedViaSet/CMakeLists.txt delete mode 100644 Tests/VariableUnusedViaUnset/CMakeLists.txt diff --git a/Help/manual/cmake.1.rst b/Help/manual/cmake.1.rst index 72ecfc7..d5fe34c 100644 --- a/Help/manual/cmake.1.rst +++ b/Help/manual/cmake.1.rst @@ -341,9 +341,9 @@ Options Print a warning when an uninitialized variable is used. ``--warn-unused-vars`` - Warn about unused variables. - - Find variables that are declared or set, but not used. + Does nothing. In CMake versions 3.2 and below this enabled warnings about + unused variables. In CMake versions 3.3 through 3.18 the option was broken. + In CMake 3.19 and above the option has been removed. ``--no-warn-unused-cli`` Don't warn about command line options. diff --git a/Help/release/dev/remove-warn-unused-vars.rst b/Help/release/dev/remove-warn-unused-vars.rst new file mode 100644 index 0000000..7a06e91 --- /dev/null +++ b/Help/release/dev/remove-warn-unused-vars.rst @@ -0,0 +1,6 @@ +remove-warn-unused-vars +----------------------- + +* The :manual:`cmake(1)` command-line option ``--warn-unused-vars`` has + been removed and is now silently ignored. The option has not worked + correctly since CMake 3.3. diff --git a/Source/QtDialog/CMakeSetupDialog.cxx b/Source/QtDialog/CMakeSetupDialog.cxx index 6dbfe11..fcc8408 100644 --- a/Source/QtDialog/CMakeSetupDialog.cxx +++ b/Source/QtDialog/CMakeSetupDialog.cxx @@ -152,9 +152,6 @@ CMakeSetupDialog::CMakeSetupDialog() this->WarnUninitializedAction = OptionsMenu->addAction(tr("&Warn Uninitialized (--warn-uninitialized)")); this->WarnUninitializedAction->setCheckable(true); - this->WarnUnusedAction = - OptionsMenu->addAction(tr("&Warn Unused (--warn-unused-vars)")); - this->WarnUnusedAction->setCheckable(true); QAction* debugAction = OptionsMenu->addAction(tr("&Debug Output")); debugAction->setCheckable(true); @@ -290,9 +287,6 @@ void CMakeSetupDialog::initialize() QObject::connect(this->WarnUninitializedAction, SIGNAL(triggered(bool)), this->CMakeThread->cmakeInstance(), SLOT(setWarnUninitializedMode(bool))); - QObject::connect(this->WarnUnusedAction, SIGNAL(triggered(bool)), - this->CMakeThread->cmakeInstance(), - SLOT(setWarnUnusedMode(bool))); if (!this->SourceDirectory->text().isEmpty() || !this->BinaryDirectory->lineEdit()->text().isEmpty()) { diff --git a/Source/QtDialog/CMakeSetupDialog.h b/Source/QtDialog/CMakeSetupDialog.h index d1e2035..914be12 100644 --- a/Source/QtDialog/CMakeSetupDialog.h +++ b/Source/QtDialog/CMakeSetupDialog.h @@ -111,7 +111,6 @@ protected: QAction* ConfigureAction; QAction* GenerateAction; QAction* WarnUninitializedAction; - QAction* WarnUnusedAction; QAction* InstallForCommandLineAction; State CurrentState; diff --git a/Source/QtDialog/QCMake.cxx b/Source/QtDialog/QCMake.cxx index f0b967b..6090256 100644 --- a/Source/QtDialog/QCMake.cxx +++ b/Source/QtDialog/QCMake.cxx @@ -21,7 +21,6 @@ QCMake::QCMake(QObject* p) : QObject(p) { this->WarnUninitializedMode = false; - this->WarnUnusedMode = false; qRegisterMetaType(); qRegisterMetaType(); @@ -170,7 +169,6 @@ void QCMake::configure() this->CMakeInstance->SetGeneratorToolset(this->Toolset.toLocal8Bit().data()); this->CMakeInstance->LoadCache(); this->CMakeInstance->SetWarnUninitialized(this->WarnUninitializedMode); - this->CMakeInstance->SetWarnUnused(this->WarnUnusedMode); this->CMakeInstance->PreLoadCMakeFiles(); InterruptFlag = 0; @@ -478,11 +476,6 @@ void QCMake::setWarnUninitializedMode(bool value) this->WarnUninitializedMode = value; } -void QCMake::setWarnUnusedMode(bool value) -{ - this->WarnUnusedMode = value; -} - void QCMake::checkOpenPossible() { std::string data = this->BinaryDirectory.toLocal8Bit().data(); diff --git a/Source/QtDialog/QCMake.h b/Source/QtDialog/QCMake.h index 110a971..39555d6 100644 --- a/Source/QtDialog/QCMake.h +++ b/Source/QtDialog/QCMake.h @@ -114,8 +114,6 @@ public slots: void setDeprecatedWarningsAsErrors(bool value); /// set whether to run cmake with warnings about uninitialized variables void setWarnUninitializedMode(bool value); - /// set whether to run cmake with warnings about unused variables - void setWarnUnusedMode(bool value); /// check if project IDE open is possible and emit openPossible signal void checkOpenPossible(); @@ -175,8 +173,6 @@ protected: void stderrCallback(std::string const& msg); bool WarnUninitializedMode; - bool WarnUnusedMode; - bool WarnUnusedAllMode; QString SourceDirectory; QString BinaryDirectory; QString Generator; diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 69a6427..4a4f87d 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -19,7 +19,6 @@ cmDefinitions::Def const& cmDefinitions::GetInternal(const std::string& key, { auto it = begin->Map.find(cm::String::borrow(key)); if (it != begin->Map.end()) { - it->second.Used = true; return it->second; } } @@ -108,16 +107,3 @@ void cmDefinitions::Unset(const std::string& key) { this->Map[key] = Def(); } - -std::vector cmDefinitions::UnusedKeys() const -{ - std::vector keys; - keys.reserve(this->Map.size()); - // Consider local definitions. - for (auto const& mi : this->Map) { - if (!mi.second.Used) { - keys.push_back(*mi.first.str_if_stable()); - } - } - return keys; -} diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index 0e38fb1..d57750a 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -48,9 +48,6 @@ public: /** Unset a definition. */ void Unset(const std::string& key); - /** List of unused keys. */ - std::vector UnusedKeys() const; - private: /** String with existence boolean. */ struct Def @@ -62,7 +59,6 @@ private: { } cm::String Value; - bool Used = false; }; static Def NoDef; diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index d34259f..25013df 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -83,7 +83,6 @@ cmMakefile::cmMakefile(cmGlobalGenerator* globalGenerator, { this->IsSourceFileTryCompile = false; - this->WarnUnused = this->GetCMakeInstance()->GetWarnUnused(); this->CheckSystemVars = this->GetCMakeInstance()->GetCheckSystemVars(); this->SuppressSideEffects = false; @@ -751,7 +750,6 @@ void cmMakefile::ReadListFile(cmListFile const& listFile, break; } } - this->CheckForUnusedVariables(); this->AddDefinition("CMAKE_PARENT_LIST_FILE", currentParentFile); this->AddDefinition("CMAKE_CURRENT_LIST_FILE", currentFile); @@ -1511,8 +1509,6 @@ void cmMakefile::PopFunctionScope(bool reportError) #endif this->PopLoopBlockBarrier(); - - this->CheckForUnusedVariables(); } void cmMakefile::PushMacroScope(std::string const& fileName, @@ -1859,9 +1855,6 @@ void cmMakefile::AddSystemIncludeDirectories(const std::set& incs) void cmMakefile::AddDefinition(const std::string& name, cm::string_view value) { - if (this->VariableInitialized(name)) { - this->LogUnused("changing definition", name); - } this->StateSnapshot.SetDefinition(name, value); #ifndef CMAKE_BOOTSTRAP @@ -1922,16 +1915,6 @@ void cmMakefile::AddCacheDefinition(const std::string& name, const char* value, this->StateSnapshot.RemoveDefinition(name); } -void cmMakefile::CheckForUnusedVariables() const -{ - if (!this->WarnUnused) { - return; - } - for (const std::string& key : this->StateSnapshot.UnusedKeys()) { - this->LogUnused("out of scope", key); - } -} - void cmMakefile::MarkVariableAsUsed(const std::string& var) { this->StateSnapshot.GetDefinition(var); @@ -1959,29 +1942,8 @@ void cmMakefile::MaybeWarnUninitialized(std::string const& variable, } } -void cmMakefile::LogUnused(const char* reason, const std::string& name) const -{ - if (this->WarnUnused) { - std::string path; - if (!this->ExecutionStatusStack.empty()) { - path = this->GetExecutionContext().FilePath; - } else { - path = cmStrCat(this->GetCurrentSourceDirectory(), "/CMakeLists.txt"); - } - - if (this->CheckSystemVars || this->IsProjectFile(path.c_str())) { - std::ostringstream msg; - msg << "unused variable (" << reason << ") \'" << name << "\'"; - this->IssueMessage(MessageType::AUTHOR_WARNING, msg.str()); - } - } -} - void cmMakefile::RemoveDefinition(const std::string& name) { - if (this->VariableInitialized(name)) { - this->LogUnused("unsetting", name); - } this->StateSnapshot.RemoveDefinition(name); #ifndef CMAKE_BOOTSTRAP cmVariableWatch* vv = this->GetVariableWatch(); @@ -3724,7 +3686,7 @@ int cmMakefile::TryCompile(const std::string& srcdir, if (cmakeArgs) { // FIXME: Workaround to ignore unused CLI variables in try-compile. // - // Ideally we should use SetArgs to honor options like --warn-unused-vars. + // Ideally we should use SetArgs for options like --no-warn-unused-cli. // However, there is a subtle problem when certain arguments are passed to // a macro wrapping around try_compile or try_run that does not escape // semicolons in its parameters but just passes ${ARGV} or ${ARGN}. In @@ -3743,7 +3705,7 @@ int cmMakefile::TryCompile(const std::string& srcdir, // the value VAR=a is sufficient for the try_compile or try_run to get the // correct result. Calling SetArgs here would break such projects that // previously built. Instead we work around the issue by never reporting - // unused arguments and ignoring options such as --warn-unused-vars. + // unused arguments and ignoring options such as --no-warn-unused-cli. cm.SetWarnUnusedCli(false); // cm.SetArgs(*cmakeArgs, true); @@ -4247,8 +4209,6 @@ void cmMakefile::PopScope() this->PopLoopBlockBarrier(); - this->CheckForUnusedVariables(); - this->PopSnapshot(); } diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 332554e..045ded4 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -995,9 +995,6 @@ protected: // add link libraries and directories to the target void AddGlobalLinkInformation(cmTarget& target); - // Check for a an unused variable - void LogUnused(const char* reason, const std::string& name) const; - mutable std::set CMP0054ReportedIds; // libraries, classes, and executables @@ -1233,10 +1230,6 @@ private: std::string const& config, const std::string& feature) const; - void CheckForUnusedVariables() const; - - // Unused variable flags - bool WarnUnused; bool CheckSystemVars; bool CheckCMP0000; std::set WarnedCMP0074; diff --git a/Source/cmServerProtocol.cxx b/Source/cmServerProtocol.cxx index 4f7131f..e586fd9 100644 --- a/Source/cmServerProtocol.cxx +++ b/Source/cmServerProtocol.cxx @@ -136,6 +136,7 @@ bool cmServerProtocol::Activate(cmServer* server, this->m_Server = server; this->m_CMakeInstance = cm::make_unique(cmake::RoleProject, cmState::Project); + this->m_WarnUnused = false; const bool result = this->DoActivate(request, errorMessage); if (!result) { this->m_CMakeInstance = nullptr; @@ -636,7 +637,7 @@ cmServerResponse cmServerProtocol1::ProcessGlobalSettings( obj[kTRACE_KEY] = cm->GetTrace(); obj[kTRACE_EXPAND_KEY] = cm->GetTraceExpand(); obj[kWARN_UNINITIALIZED_KEY] = cm->GetWarnUninitialized(); - obj[kWARN_UNUSED_KEY] = cm->GetWarnUnused(); + obj[kWARN_UNUSED_KEY] = m_WarnUnused; obj[kWARN_UNUSED_CLI_KEY] = cm->GetWarnUnusedCli(); obj[kCHECK_SYSTEM_VARS_KEY] = cm->GetCheckSystemVars(); @@ -682,7 +683,7 @@ cmServerResponse cmServerProtocol1::ProcessSetGlobalSettings( setBool(request, kTRACE_EXPAND_KEY, [cm](bool e) { cm->SetTraceExpand(e); }); setBool(request, kWARN_UNINITIALIZED_KEY, [cm](bool e) { cm->SetWarnUninitialized(e); }); - setBool(request, kWARN_UNUSED_KEY, [cm](bool e) { cm->SetWarnUnused(e); }); + setBool(request, kWARN_UNUSED_KEY, [this](bool e) { m_WarnUnused = e; }); setBool(request, kWARN_UNUSED_CLI_KEY, [cm](bool e) { cm->SetWarnUnusedCli(e); }); setBool(request, kCHECK_SYSTEM_VARS_KEY, diff --git a/Source/cmServerProtocol.h b/Source/cmServerProtocol.h index c71b7bf..6009e23 100644 --- a/Source/cmServerProtocol.h +++ b/Source/cmServerProtocol.h @@ -94,6 +94,7 @@ protected: // Implement protocol specific activation tasks here. Called from Activate(). virtual bool DoActivate(const cmServerRequest& request, std::string* errorMessage); + bool m_WarnUnused = false; // storage for legacy option private: std::unique_ptr m_CMakeInstance; diff --git a/Source/cmStateSnapshot.cxx b/Source/cmStateSnapshot.cxx index 15a98af..cfabc3f 100644 --- a/Source/cmStateSnapshot.cxx +++ b/Source/cmStateSnapshot.cxx @@ -232,11 +232,6 @@ void cmStateSnapshot::RemoveDefinition(std::string const& name) this->Position->Vars->Unset(name); } -std::vector cmStateSnapshot::UnusedKeys() const -{ - return this->Position->Vars->UnusedKeys(); -} - std::vector cmStateSnapshot::ClosureKeys() const { return cmDefinitions::ClosureKeys(this->Position->Vars, diff --git a/Source/cmStateSnapshot.h b/Source/cmStateSnapshot.h index 021fd53..c19f174 100644 --- a/Source/cmStateSnapshot.h +++ b/Source/cmStateSnapshot.h @@ -28,7 +28,6 @@ public: bool IsInitialized(std::string const& name) const; void SetDefinition(std::string const& name, cm::string_view value); void RemoveDefinition(std::string const& name); - std::vector UnusedKeys() const; std::vector ClosureKeys() const; bool RaiseScope(std::string const& var, const char* varDef); diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 27ecb4d..638f10e 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -773,8 +773,7 @@ void cmake::SetArgs(const std::vector& args) std::cout << "Warn about uninitialized values.\n"; this->SetWarnUninitialized(true); } else if (cmHasLiteralPrefix(arg, "--warn-unused-vars")) { - std::cout << "Finding unused variables.\n"; - this->SetWarnUnused(true); + // Option was removed. } else if (cmHasLiteralPrefix(arg, "--no-warn-unused-cli")) { std::cout << "Not searching for unused variables given on the " << "command line.\n"; diff --git a/Source/cmake.h b/Source/cmake.h index c5d608f..0c4f429 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -450,8 +450,6 @@ public: bool GetWarnUninitialized() { return this->WarnUninitialized; } void SetWarnUninitialized(bool b) { this->WarnUninitialized = b; } - bool GetWarnUnused() { return this->WarnUnused; } - void SetWarnUnused(bool b) { this->WarnUnused = b; } bool GetWarnUnusedCli() { return this->WarnUnusedCli; } void SetWarnUnusedCli(bool b) { this->WarnUnusedCli = b; } bool GetCheckSystemVars() { return this->CheckSystemVars; } @@ -605,7 +603,6 @@ private: TraceFormat TraceFormatVar = TRACE_HUMAN; cmGeneratedFileStream TraceFile; bool WarnUninitialized = false; - bool WarnUnused = false; bool WarnUnusedCli = true; bool CheckSystemVars = false; std::map UsedCliVariables; diff --git a/Source/cmakemain.cxx b/Source/cmakemain.cxx index 9fa6705..cebf34e 100644 --- a/Source/cmakemain.cxx +++ b/Source/cmakemain.cxx @@ -93,7 +93,6 @@ const char* cmDocumentationOptions[][2] = { { "--trace-redirect=", "Redirect trace output to a file instead of stderr." }, { "--warn-uninitialized", "Warn about uninitialized values." }, - { "--warn-unused-vars", "Warn about unused variables." }, { "--no-warn-unused-cli", "Don't warn about command line options." }, { "--check-system-vars", "Find problems with variable usage in system " diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 067ffdb..19aa4c4 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -2417,36 +2417,6 @@ ${CMake_SOURCE_DIR}/Utilities/Release/push.bash --dir dev -- '${CMake_BUILD_NIGH list(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/BundleGeneratorTest") endif() - add_test(WarnUnusedUnusedViaSet ${CMAKE_CTEST_COMMAND} - --build-and-test - "${CMake_SOURCE_DIR}/Tests/VariableUnusedViaSet" - "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaSet" - ${build_generator_args} - --build-noclean - --build-project WarnUnusedUnusedViaSet - --build-options - "--warn-unused-vars") - set_tests_properties(WarnUnusedUnusedViaSet PROPERTIES - PASS_REGULAR_EXPRESSION "unused variable \\(changing definition\\) 'UNUSED_VARIABLE'") - set_tests_properties(WarnUnusedUnusedViaSet PROPERTIES - FAIL_REGULAR_EXPRESSION "unused variable \\(unsetting\\) 'UNUSED_VARIABLE'") - list(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaSet") - - add_test(WarnUnusedUnusedViaUnset ${CMAKE_CTEST_COMMAND} - --build-and-test - "${CMake_SOURCE_DIR}/Tests/VariableUnusedViaUnset" - "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaUnset" - ${build_generator_args} - --build-noclean - --build-project WarnUnusedUnusedViaUnset - --build-options - "--warn-unused-vars") - set_tests_properties(WarnUnusedUnusedViaUnset PROPERTIES - PASS_REGULAR_EXPRESSION "CMake Warning \\(dev\\) at CMakeLists.txt:7 \\(set\\):") - set_tests_properties(WarnUnusedUnusedViaUnset PROPERTIES - FAIL_REGULAR_EXPRESSION "CMakeLists.txt:5 \\(set\\):") - list(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaUnset") - add_test(WarnUnusedCliUnused ${CMAKE_CTEST_COMMAND} --build-and-test "${CMake_SOURCE_DIR}/Tests/WarnUnusedCliUnused" diff --git a/Tests/VariableUnusedViaSet/CMakeLists.txt b/Tests/VariableUnusedViaSet/CMakeLists.txt deleted file mode 100644 index 0123ab2..0000000 --- a/Tests/VariableUnusedViaSet/CMakeLists.txt +++ /dev/null @@ -1,4 +0,0 @@ -set(UNUSED_VARIABLE) -# Warning should occur here -set(UNUSED_VARIABLE "Usage") -message(STATUS "${UNUSED_VARIABLE}") diff --git a/Tests/VariableUnusedViaUnset/CMakeLists.txt b/Tests/VariableUnusedViaUnset/CMakeLists.txt deleted file mode 100644 index 4b4031d..0000000 --- a/Tests/VariableUnusedViaUnset/CMakeLists.txt +++ /dev/null @@ -1,8 +0,0 @@ -# NOTE: Changing lines in here changes the test results since the first -# instance shouldn't warn, but the second should and they have the same message - -# A warning should NOT be issued for this line: -set(UNUSED_VARIABLE) -# Warning should occur here: -set(UNUSED_VARIABLE) -message(STATUS "${UNUSED_VARIABLE}") -- cgit v0.12