Merge topic 'remove-warn-unused-vars'

df6b077625 cmake: Remove broken '--warn-unused-vars' option

Acked-by: Kitware Robot <[email protected]>
Acked-by: Ben Boeckel <[email protected]>
Merge-request: !4953

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.

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.

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()) {

Source/QtDialog/CMakeSetupDialog.h

@@ -111,7 +111,6 @@ protected:
   QAction* ConfigureAction;
   QAction* GenerateAction;
   QAction* WarnUninitializedAction;
-  QAction* WarnUnusedAction;
   QAction* InstallForCommandLineAction;
   State CurrentState;
 

Source/QtDialog/QCMake.cxx

@@ -21,7 +21,6 @@ QCMake::QCMake(QObject* p)
   : QObject(p)
 {
   this->WarnUninitializedMode = false;
-  this->WarnUnusedMode = false;
   qRegisterMetaType<QCMakeProperty>();
   qRegisterMetaType<QCMakePropertyList>();
 
@@ -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();

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;

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<std::string> cmDefinitions::UnusedKeys() const
-{
-  std::vector<std::string> 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;
-}

Source/cmDefinitions.h

@@ -48,9 +48,6 @@ public:
   /** Unset a definition.  */
   void Unset(const std::string& key);
 
-  /** List of unused keys.  */
-  std::vector<std::string> UnusedKeys() const;
-
 private:
   /** String with existence boolean.  */
   struct Def
@@ -62,7 +59,6 @@ private:
     {
     }
     cm::String Value;
-    bool Used = false;
   };
   static Def NoDef;
 

Source/cmMakefile.cxx

@@ -86,7 +86,6 @@ cmMakefile::cmMakefile(cmGlobalGenerator* globalGenerator,
 {
   this->IsSourceFileTryCompile = false;
 
-  this->WarnUnused = this->GetCMakeInstance()->GetWarnUnused();
   this->CheckSystemVars = this->GetCMakeInstance()->GetCheckSystemVars();
 
   this->SuppressSideEffects = false;
@@ -754,7 +753,6 @@ void cmMakefile::ReadListFile(cmListFile const& listFile,
       break;
     }
   }
-  this->CheckForUnusedVariables();
 
   this->AddDefinition("CMAKE_PARENT_LIST_FILE", currentParentFile);
   this->AddDefinition("CMAKE_CURRENT_LIST_FILE", currentFile);
@@ -1514,8 +1512,6 @@ void cmMakefile::PopFunctionScope(bool reportError)
 #endif
 
   this->PopLoopBlockBarrier();
-
-  this->CheckForUnusedVariables();
 }
 
 void cmMakefile::PushMacroScope(std::string const& fileName,
@@ -1862,9 +1858,6 @@ void cmMakefile::AddSystemIncludeDirectories(const std::set<std::string>& 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
@@ -1925,16 +1918,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);
@@ -1962,29 +1945,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();
@@ -3727,7 +3689,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
@@ -3746,7 +3708,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);
 
@@ -4262,8 +4224,6 @@ void cmMakefile::PopScope()
 
   this->PopLoopBlockBarrier();
 
-  this->CheckForUnusedVariables();
-
   this->PopSnapshot();
 }
 

Source/cmMakefile.h

@@ -996,9 +996,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<cmListFileContext> CMP0054ReportedIds;
 
   // libraries, classes, and executables
@@ -1234,10 +1231,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<std::string> WarnedCMP0074;

Source/cmServerProtocol.cxx

@@ -136,6 +136,7 @@ bool cmServerProtocol::Activate(cmServer* server,
   this->m_Server = server;
   this->m_CMakeInstance =
     cm::make_unique<cmake>(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,

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<cmake> m_CMakeInstance;

Source/cmStateSnapshot.cxx

@@ -232,11 +232,6 @@ void cmStateSnapshot::RemoveDefinition(std::string const& name)
   this->Position->Vars->Unset(name);
 }
 
-std::vector<std::string> cmStateSnapshot::UnusedKeys() const
-{
-  return this->Position->Vars->UnusedKeys();
-}
-
 std::vector<std::string> cmStateSnapshot::ClosureKeys() const
 {
   return cmDefinitions::ClosureKeys(this->Position->Vars,

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<std::string> UnusedKeys() const;
   std::vector<std::string> ClosureKeys() const;
   bool RaiseScope(std::string const& var, const char* varDef);
 

Source/cmake.cxx

@@ -773,8 +773,7 @@ void cmake::SetArgs(const std::vector<std::string>& 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";

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<std::string, bool> UsedCliVariables;

Source/cmakemain.cxx

@@ -93,7 +93,6 @@ const char* cmDocumentationOptions[][2] = {
   { "--trace-redirect=<file>",
     "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 "

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"

Tests/VariableUnusedViaSet/CMakeLists.txt

@@ -1,4 +0,0 @@
-set(UNUSED_VARIABLE)
-# Warning should occur here
-set(UNUSED_VARIABLE "Usage")
-message(STATUS "${UNUSED_VARIABLE}")

Tests/VariableUnusedViaUnset/CMakeLists.txt

@@ -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}")