From cd626ea66ed114736ddf5d6a4c989ef6c9b8d248 Mon Sep 17 00:00:00 2001 From: Bill Hoffman Date: Tue, 13 Apr 2010 08:56:11 -0400 Subject: [PATCH 01/87] For macros make sure the FilePath points to a valid pointer in the args. --- Source/cmMacroCommand.cxx | 12 ++++++++++-- Source/cmMacroCommand.h | 1 - 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index 497949aed..774f32b44 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -31,6 +31,7 @@ public: // we must copy when we clone newC->Args = this->Args; newC->Functions = this->Functions; + newC->FilePath = this->FilePath; newC->Policies = this->Policies; return newC; } @@ -78,6 +79,7 @@ public: std::vector Args; std::vector Functions; cmPolicies::PolicyMap Policies; + std::string FilePath; }; @@ -121,7 +123,10 @@ bool cmMacroHelperCommand::InvokeInitialPass std::string argnDef; bool argnDefInitialized = false; bool argvDefInitialized = false; - + if( this->Functions.size()) + { + this->FilePath = this->Functions[0].FilePath; + } // Invoke all the functions that were collected in the block. cmListFileFunction newLFF; // for each function @@ -135,10 +140,13 @@ bool cmMacroHelperCommand::InvokeInitialPass newLFF.Line = this->Functions[c].Line; // for each argument of the current function - for (std::vector::const_iterator k = + for (std::vector::iterator k = this->Functions[c].Arguments.begin(); k != this->Functions[c].Arguments.end(); ++k) { + // Set the FilePath on the arguments to match the function since it is + // not stored and the original values may be freed + k->FilePath = this->FilePath.c_str(); tmps = k->Value; // replace formal arguments for (unsigned int j = 1; j < this->Args.size(); ++j) diff --git a/Source/cmMacroCommand.h b/Source/cmMacroCommand.h index 3457da2d9..93e10b271 100644 --- a/Source/cmMacroCommand.h +++ b/Source/cmMacroCommand.h @@ -112,7 +112,6 @@ public: "policies inside macros." ; } - cmTypeMacro(cmMacroCommand, cmCommand); }; From 48b5b855934be341c02139c0bed88c35c1b40d8f Mon Sep 17 00:00:00 2001 From: Bill Hoffman Date: Tue, 13 Apr 2010 09:21:31 -0400 Subject: [PATCH 02/87] Add a warning when variables are used uninitialized. --- Source/cmCommandArgumentParserHelper.cxx | 7 ++++++- Source/cmMakefile.cxx | 11 +++++++++++ Source/cmMakefile.h | 4 ++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/Source/cmCommandArgumentParserHelper.cxx b/Source/cmCommandArgumentParserHelper.cxx index 234c37e56..027a2ba78 100644 --- a/Source/cmCommandArgumentParserHelper.cxx +++ b/Source/cmCommandArgumentParserHelper.cxx @@ -119,10 +119,15 @@ char* cmCommandArgumentParserHelper::ExpandVariable(const char* var) cmOStringStream ostr; ostr << this->FileLine; return this->AddString(ostr.str().c_str()); - } + } const char* value = this->Makefile->GetDefinition(var); if(!value && !this->RemoveEmpty) { + if(!this->Makefile->VariableCleared(var)) + { + std::cerr << this->FileName << ":" << this->FileLine << ":" << + " warning: uninitialized variable \'" << var << "\'\n"; + } return 0; } if (this->EscapeQuotes && value) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 8eece6b5a..c56641c91 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -43,6 +43,7 @@ class cmMakefile::Internals { public: std::stack > VarStack; + std::set VarRemoved; }; // default is not to be building executables @@ -1694,9 +1695,19 @@ void cmMakefile::AddDefinition(const char* name, bool value) #endif } +bool cmMakefile::VariableCleared(const char* var) const +{ + if(this->Internal->VarRemoved.find(var) != this->Internal->VarRemoved.end()) + { + return true; + } + return false; +} + void cmMakefile::RemoveDefinition(const char* name) { this->Internal->VarStack.top().Set(name, 0); + this->Internal->VarRemoved.insert(name); #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); if ( vv ) diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 4fae7eee2..daeab8392 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -61,6 +61,10 @@ public: unsigned int GetCacheMajorVersion(); unsigned int GetCacheMinorVersion(); + /* return true if a variable has been set with + set(foo ) + */ + bool VariableCleared(const char* ) const; /** Return whether compatibility features needed for a version of the cache or lower should be enabled. */ bool NeedCacheCompatibility(int major, int minor); From f794d589a44918c905911eb7688d69350922c6b3 Mon Sep 17 00:00:00 2001 From: Bill Hoffman Date: Mon, 12 Jul 2010 15:48:51 -0400 Subject: [PATCH 03/87] Make --strict-mode option, and integrate with cmake-gui --- Modules/CMakeDetermineCompilerABI.cmake | 14 ++++++++++++-- Modules/CMakeDetermineCompilerId.cmake | 4 +++- Source/QtDialog/CMakeSetupDialog.cxx | 9 ++++++++- Source/QtDialog/CMakeSetupDialog.h | 1 + Source/QtDialog/QCMake.cxx | 8 ++++++++ Source/QtDialog/QCMake.h | 3 +++ Source/cmCommandArgumentParserHelper.cxx | 13 ++++++++++--- Source/cmCommandArgumentParserHelper.h | 1 + Source/cmake.cxx | 6 ++++++ Source/cmake.h | 3 +++ Source/cmakemain.cxx | 3 +++ 11 files changed, 58 insertions(+), 7 deletions(-) diff --git a/Modules/CMakeDetermineCompilerABI.cmake b/Modules/CMakeDetermineCompilerABI.cmake index d6df30590..aa21c3127 100644 --- a/Modules/CMakeDetermineCompilerABI.cmake +++ b/Modules/CMakeDetermineCompilerABI.cmake @@ -24,9 +24,13 @@ FUNCTION(CMAKE_DETERMINE_COMPILER_ABI lang src) # Compile the ABI identification source. SET(BIN "${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeDetermineCompilerABI_${lang}.bin") + SET(CMAKE_FLAGS ) + IF(DEFINED CMAKE_${lang}_VERBOSE_FLAG) + SET(CMAKE_FLAGS "-DCMAKE_EXE_LINKER_FLAGS=${CMAKE_${lang}_VERBOSE_FLAG}") + ENDIF() TRY_COMPILE(CMAKE_DETERMINE_${lang}_ABI_COMPILED ${CMAKE_BINARY_DIR} ${src} - CMAKE_FLAGS "-DCMAKE_EXE_LINKER_FLAGS=${CMAKE_${lang}_VERBOSE_FLAG}" + CMAKE_FLAGS "${CMAKE_FLAGS}" "-DCMAKE_${lang}_STANDARD_LIBRARIES=" OUTPUT_VARIABLE OUTPUT COPY_FILE "${BIN}" @@ -58,10 +62,16 @@ FUNCTION(CMAKE_DETERMINE_COMPILER_ABI lang src) # Parse implicit linker information for this language, if available. SET(implicit_dirs "") SET(implicit_libs "") + SET(MULTI_ARCH FALSE) + IF(DEFINED CMAKE_OSX_ARCHITECTURES) + IF( "${CMAKE_OSX_ARCHITECTURES}" MATCHES ";" ) + SET(MULTI_ARCH TRUE) + ENDIF() + ENDIF() IF(CMAKE_${lang}_VERBOSE_FLAG # Implicit link information cannot be used explicitly for # multiple OS X architectures, so we skip it. - AND NOT "${CMAKE_OSX_ARCHITECTURES}" MATCHES ";" + AND NOT MULTI_ARCH # Skip this with Xcode for now. AND NOT "${CMAKE_GENERATOR}" MATCHES Xcode) CMAKE_PARSE_IMPLICIT_LINK_INFO("${OUTPUT}" implicit_libs implicit_dirs log) diff --git a/Modules/CMakeDetermineCompilerId.cmake b/Modules/CMakeDetermineCompilerId.cmake index bf78a5ba3..9a3884af9 100644 --- a/Modules/CMakeDetermineCompilerId.cmake +++ b/Modules/CMakeDetermineCompilerId.cmake @@ -243,7 +243,9 @@ FUNCTION(CMAKE_DETERMINE_COMPILER_ID_CHECK lang file) # ENDIF("${CMAKE_EXECUTABLE_MAGIC}" MATCHES "feedface") ENDIF(NOT CMAKE_EXECUTABLE_FORMAT) - + IF(NOT DEFINED CMAKE_EXECUTABLE_FORMAT) + SET(CMAKE_EXECUTABLE_FORMAT) + ENDIF() # Return the information extracted. SET(CMAKE_${lang}_COMPILER_ID "${CMAKE_${lang}_COMPILER_ID}" PARENT_SCOPE) SET(CMAKE_${lang}_PLATFORM_ID "${CMAKE_${lang}_PLATFORM_ID}" PARENT_SCOPE) diff --git a/Source/QtDialog/CMakeSetupDialog.cxx b/Source/QtDialog/CMakeSetupDialog.cxx index 74a3d3594..7600897bd 100644 --- a/Source/QtDialog/CMakeSetupDialog.cxx +++ b/Source/QtDialog/CMakeSetupDialog.cxx @@ -114,8 +114,12 @@ CMakeSetupDialog::CMakeSetupDialog() this, SLOT(doInstallForCommandLine())); #endif QMenu* OptionsMenu = this->menuBar()->addMenu(tr("&Options")); - this->SuppressDevWarningsAction = OptionsMenu->addAction(tr("&Suppress dev Warnings (-Wno-dev)")); + this->SuppressDevWarningsAction = + OptionsMenu->addAction(tr("&Suppress dev Warnings (-Wno-dev)")); this->SuppressDevWarningsAction->setCheckable(true); + this->StrictModeAction = + OptionsMenu->addAction(tr("&Strict Mode (--strict-mode)")); + this->StrictModeAction->setCheckable(true); QAction* debugAction = OptionsMenu->addAction(tr("&Debug Output")); debugAction->setCheckable(true); @@ -240,6 +244,9 @@ void CMakeSetupDialog::initialize() QObject::connect(this->SuppressDevWarningsAction, SIGNAL(triggered(bool)), this->CMakeThread->cmakeInstance(), SLOT(setSuppressDevWarnings(bool))); + QObject::connect(this->StrictModeAction, SIGNAL(triggered(bool)), + this->CMakeThread->cmakeInstance(), + SLOT(setStrictMode(bool))); if(!this->SourceDirectory->text().isEmpty() || !this->BinaryDirectory->lineEdit()->text().isEmpty()) diff --git a/Source/QtDialog/CMakeSetupDialog.h b/Source/QtDialog/CMakeSetupDialog.h index 0e3caec44..cd2be6e5f 100644 --- a/Source/QtDialog/CMakeSetupDialog.h +++ b/Source/QtDialog/CMakeSetupDialog.h @@ -93,6 +93,7 @@ protected: QAction* ConfigureAction; QAction* GenerateAction; QAction* SuppressDevWarningsAction; + QAction* StrictModeAction; QAction* InstallForCommandLineAction; State CurrentState; diff --git a/Source/QtDialog/QCMake.cxx b/Source/QtDialog/QCMake.cxx index dc31fad44..31daf3c86 100644 --- a/Source/QtDialog/QCMake.cxx +++ b/Source/QtDialog/QCMake.cxx @@ -28,6 +28,7 @@ QCMake::QCMake(QObject* p) : QObject(p) { this->SuppressDevWarnings = false; + this->StrictMode = false; qRegisterMetaType(); qRegisterMetaType(); @@ -164,6 +165,8 @@ void QCMake::configure() this->CMakeInstance->CreateGlobalGenerator(this->Generator.toAscii().data())); this->CMakeInstance->LoadCache(); this->CMakeInstance->SetSuppressDevWarnings(this->SuppressDevWarnings); + std::cerr << "set strict " << this->StrictMode << "\n"; + this->CMakeInstance->SetStrictMode(this->StrictMode); this->CMakeInstance->PreLoadCMakeFiles(); cmSystemTools::ResetErrorOccuredFlag(); @@ -417,3 +420,8 @@ void QCMake::setSuppressDevWarnings(bool value) { this->SuppressDevWarnings = value; } + +void QCMake::setStrictMode(bool value) +{ + this->StrictMode = value; +} diff --git a/Source/QtDialog/QCMake.h b/Source/QtDialog/QCMake.h index bbfb3d7c5..f20893f22 100644 --- a/Source/QtDialog/QCMake.h +++ b/Source/QtDialog/QCMake.h @@ -88,6 +88,8 @@ public slots: void setDebugOutput(bool); /// set whether to do suppress dev warnings void setSuppressDevWarnings(bool value); + /// set whether to run cmake in strict mode + void setStrictMode(bool value); public: /// get the list of cache properties @@ -133,6 +135,7 @@ protected: static void errorCallback(const char* msg, const char* title, bool&, void* cd); bool SuppressDevWarnings; + bool StrictMode; QString SourceDirectory; QString BinaryDirectory; QString Generator; diff --git a/Source/cmCommandArgumentParserHelper.cxx b/Source/cmCommandArgumentParserHelper.cxx index 027a2ba78..410058f1c 100644 --- a/Source/cmCommandArgumentParserHelper.cxx +++ b/Source/cmCommandArgumentParserHelper.cxx @@ -20,6 +20,7 @@ int cmCommandArgument_yyparse( yyscan_t yyscanner ); // cmCommandArgumentParserHelper::cmCommandArgumentParserHelper() { + this->StrictMode = false; this->FileLine = -1; this->FileName = 0; this->RemoveEmpty = true; @@ -123,10 +124,15 @@ char* cmCommandArgumentParserHelper::ExpandVariable(const char* var) const char* value = this->Makefile->GetDefinition(var); if(!value && !this->RemoveEmpty) { - if(!this->Makefile->VariableCleared(var)) + // check to see if we need to print a warning + // if strict mode is on and the variable has + // not been "cleared"/initialized with a set(foo ) call + if(this->StrictMode && !this->Makefile->VariableCleared(var)) { - std::cerr << this->FileName << ":" << this->FileLine << ":" << - " warning: uninitialized variable \'" << var << "\'\n"; + cmOStringStream msg; + msg << this->FileName << ":" << this->FileLine << ":" << + " warning: uninitialized variable \'" << var << "\'"; + cmSystemTools::Message(msg.str().c_str()); } return 0; } @@ -324,6 +330,7 @@ void cmCommandArgumentParserHelper::Error(const char* str) void cmCommandArgumentParserHelper::SetMakefile(const cmMakefile* mf) { this->Makefile = mf; + this->StrictMode = mf->GetCMakeInstance()->GetStrictMode(); } void cmCommandArgumentParserHelper::SetResult(const char* value) diff --git a/Source/cmCommandArgumentParserHelper.h b/Source/cmCommandArgumentParserHelper.h index 62cbc80ba..d7206ee97 100644 --- a/Source/cmCommandArgumentParserHelper.h +++ b/Source/cmCommandArgumentParserHelper.h @@ -96,6 +96,7 @@ private: const cmMakefile* Makefile; std::string Result; const char* FileName; + bool StrictMode; long FileLine; bool EscapeQuotes; std::string ErrorString; diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 0f9ef1bb3..2e777483d 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -140,6 +140,7 @@ void cmNeedBackwardsCompatibility(const std::string& variable, cmake::cmake() { this->Trace = false; + this->StrictMode = false; this->SuppressDevWarnings = false; this->DoSuppressDevWarnings = false; this->DebugOutput = false; @@ -613,6 +614,11 @@ void cmake::SetArgs(const std::vector& args) std::cout << "Running with trace output on.\n"; this->SetTrace(true); } + else if(arg.find("--strict-mode",0) == 0) + { + std::cout << "Running in strict mode.\n"; + this->SetStrictMode(true); + } else if(arg.find("-G",0) == 0) { std::string value = arg.substr(2); diff --git a/Source/cmake.h b/Source/cmake.h index 8312795eb..5c2f17ac4 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -306,6 +306,8 @@ class cmake // Do we want trace output during the cmake run. bool GetTrace() { return this->Trace;} void SetTrace(bool b) { this->Trace = b;} + bool GetStrictMode() { return this->StrictMode;} + void SetStrictMode(bool b) { this->StrictMode = b;} // Define a property void DefineProperty(const char *name, cmProperty::ScopeType scope, const char *ShortDescription, @@ -443,6 +445,7 @@ private: bool ScriptMode; bool DebugOutput; bool Trace; + bool StrictMode; std::string CMakeEditCommand; std::string CMakeCommand; std::string CXXEnvironment; diff --git a/Source/cmakemain.cxx b/Source/cmakemain.cxx index ddff71d9e..ac2a33847 100644 --- a/Source/cmakemain.cxx +++ b/Source/cmakemain.cxx @@ -120,6 +120,9 @@ static const char * cmDocumentationOptions[][3] = {"--trace", "Put cmake in trace mode.", "Print a trace of all calls made and from where with " "message(send_error ) calls."}, + {"--strict-mode", "Put cmake in strict mode.", + "In strict mode cmake will print a warning when an uninitialized variable " + "is used."}, {"--help-command cmd [file]", "Print help for a single command and exit.", "Full documentation specific to the given command is displayed. " "If a file is specified, the documentation is written into and the output " From 52f9637174242752721dfb322908adb40c8244c2 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 24 Aug 2010 14:40:51 -0400 Subject: [PATCH 04/87] Add method to get the local scope variables --- Source/cmDefinitions.cxx | 13 +++++++++++++ Source/cmDefinitions.h | 3 +++ 2 files changed, 16 insertions(+) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 2d407189b..34ca68d4a 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -84,6 +84,19 @@ const char* cmDefinitions::Set(const char* key, const char* value) return def.Exists? def.c_str() : 0; } +//---------------------------------------------------------------------------- +std::set cmDefinitions::LocalKeys() const +{ + std::set keys; + // Consider local definitions. + for(MapType::const_iterator mi = this->Map.begin(); + mi != this->Map.end(); ++mi) + { + keys.insert(mi->first); + } + return keys; +} + //---------------------------------------------------------------------------- cmDefinitions cmDefinitions::Closure() const { diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index e385e8887..4834d8443 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -40,6 +40,9 @@ public: /** Set (or unset if null) a value associated with a key. */ const char* Set(const char* key, const char* value); + /** Get the set of all local keys. */ + std::set LocalKeys() const; + /** Compute the closure of all defined keys with values. This flattens the scope. The result has no parent. */ cmDefinitions Closure() const; From f332e14ff2035e33bced0915373296a1f4cf0876 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 24 Aug 2010 14:38:06 -0400 Subject: [PATCH 05/87] Complete strict-mode checks for uninitialized vars --- Source/cmCommandArgumentParserHelper.cxx | 2 +- Source/cmMakefile.cxx | 35 +++++++++++++++++++++++- Source/cmMakefile.h | 2 ++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/Source/cmCommandArgumentParserHelper.cxx b/Source/cmCommandArgumentParserHelper.cxx index 410058f1c..d955ff7e0 100644 --- a/Source/cmCommandArgumentParserHelper.cxx +++ b/Source/cmCommandArgumentParserHelper.cxx @@ -127,7 +127,7 @@ char* cmCommandArgumentParserHelper::ExpandVariable(const char* var) // check to see if we need to print a warning // if strict mode is on and the variable has // not been "cleared"/initialized with a set(foo ) call - if(this->StrictMode && !this->Makefile->VariableCleared(var)) + if(this->StrictMode && !this->Makefile->VariableInitialized(var)) { cmOStringStream msg; msg << this->FileName << ":" << this->FileLine << ":" << diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index c56641c91..d89168df0 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -43,13 +43,17 @@ class cmMakefile::Internals { public: std::stack > VarStack; + std::stack > VarInitStack; std::set VarRemoved; }; // default is not to be building executables cmMakefile::cmMakefile(): Internal(new Internals) { - this->Internal->VarStack.push(cmDefinitions()); + const cmDefinitions& defs = cmDefinitions(); + const std::set globalKeys = defs.LocalKeys(); + this->Internal->VarStack.push(defs); + this->Internal->VarInitStack.push(globalKeys); // Setup the default include file regular expression (match everything). this->IncludeFileRegularExpression = "^.*$"; @@ -1685,6 +1689,7 @@ void cmMakefile::AddCacheDefinition(const char* name, const char* value, void cmMakefile::AddDefinition(const char* name, bool value) { this->Internal->VarStack.top().Set(name, value? "ON" : "OFF"); + this->Internal->VarInitStack.top().insert(name); #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); if ( vv ) @@ -1695,6 +1700,15 @@ void cmMakefile::AddDefinition(const char* name, bool value) #endif } +bool cmMakefile::VariableInitialized(const char* var) const +{ + if(this->Internal->VarInitStack.top().find(var) != this->Internal->VarInitStack.top().end()) + { + return true; + } + return false; +} + bool cmMakefile::VariableCleared(const char* var) const { if(this->Internal->VarRemoved.find(var) != this->Internal->VarRemoved.end()) @@ -1708,6 +1722,7 @@ void cmMakefile::RemoveDefinition(const char* name) { this->Internal->VarStack.top().Set(name, 0); this->Internal->VarRemoved.insert(name); + this->Internal->VarInitStack.top().insert(name); #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); if ( vv ) @@ -3316,12 +3331,30 @@ std::string cmMakefile::GetListFileStack() void cmMakefile::PushScope() { cmDefinitions* parent = &this->Internal->VarStack.top(); + const std::set& init = this->Internal->VarInitStack.top(); this->Internal->VarStack.push(cmDefinitions(parent)); + this->Internal->VarInitStack.push(init); } void cmMakefile::PopScope() { + cmDefinitions* current = &this->Internal->VarStack.top(); + std::set init = this->Internal->VarInitStack.top(); + const std::set& locals = current->LocalKeys(); + // Remove initialization information for variables in the local scope. + std::set::const_iterator it = locals.begin(); + for (; it != locals.end(); ++it) + { + init.erase(*it); + } this->Internal->VarStack.pop(); + this->Internal->VarInitStack.pop(); + // Push initialization up to the parent scope. + it = init.begin(); + for (; it != init.end(); ++it) + { + this->Internal->VarInitStack.top().insert(*it); + } } void cmMakefile::RaiseScope(const char *var, const char *varDef) diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index daeab8392..cec273805 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -61,6 +61,8 @@ public: unsigned int GetCacheMajorVersion(); unsigned int GetCacheMinorVersion(); + /* return true if a variable has been initialized */ + bool VariableInitialized(const char* ) const; /* return true if a variable has been set with set(foo ) */ From d3e8eb504137dde90a73f1b46f97f889af46db18 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 24 Aug 2010 16:49:49 -0400 Subject: [PATCH 06/87] Add flags to detect unused variables --- Source/cmake.cxx | 14 ++++++++++++++ Source/cmake.h | 6 ++++++ Source/cmakemain.cxx | 4 ++++ 3 files changed, 24 insertions(+) diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 2e777483d..41f2775ac 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -141,6 +141,8 @@ cmake::cmake() { this->Trace = false; this->StrictMode = false; + this->FindUnused = false; + this->DefaultToUsed = false; this->SuppressDevWarnings = false; this->DoSuppressDevWarnings = false; this->DebugOutput = false; @@ -619,6 +621,18 @@ void cmake::SetArgs(const std::vector& args) std::cout << "Running in strict mode.\n"; this->SetStrictMode(true); } + else if(arg.find("--find-unused",0) == 0) + { + std::cout << "Finding unused command line variables.\n"; + this->SetFindUnused(true); + this->SetDefaultToUsed(true); + } + else if(arg.find("--find-unused-all",0) == 0) + { + std::cout << "Finding unused variables.\n"; + this->SetFindUnused(true); + this->SetDefaultToUsed(false); + } else if(arg.find("-G",0) == 0) { std::string value = arg.substr(2); diff --git a/Source/cmake.h b/Source/cmake.h index 5c2f17ac4..7b612ea70 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -308,6 +308,10 @@ class cmake void SetTrace(bool b) { this->Trace = b;} bool GetStrictMode() { return this->StrictMode;} void SetStrictMode(bool b) { this->StrictMode = b;} + bool GetFindUnused() { return this->FindUnused;} + void SetFindUnused(bool b) { this->FindUnused = b;} + bool GetDefaultToUsed() { return this->DefaultToUsed;} + void SetDefaultToUsed(bool b) { this->DefaultToUsed = b;} // Define a property void DefineProperty(const char *name, cmProperty::ScopeType scope, const char *ShortDescription, @@ -446,6 +450,8 @@ private: bool DebugOutput; bool Trace; bool StrictMode; + bool FindUnused; + bool DefaultToUsed; std::string CMakeEditCommand; std::string CMakeCommand; std::string CXXEnvironment; diff --git a/Source/cmakemain.cxx b/Source/cmakemain.cxx index ac2a33847..235aaf527 100644 --- a/Source/cmakemain.cxx +++ b/Source/cmakemain.cxx @@ -123,6 +123,10 @@ static const char * cmDocumentationOptions[][3] = {"--strict-mode", "Put cmake in strict mode.", "In strict mode cmake will print a warning when an uninitialized variable " "is used."}, + {"--find-unused", "Find unused variables.", + "Find variables that are declared on the command line, but not used."}, + {"--find-unused-all", "Find all unused variables.", + "Find variables that are declared, but not used."}, {"--help-command cmd [file]", "Print help for a single command and exit.", "Full documentation specific to the given command is displayed. " "If a file is specified, the documentation is written into and the output " From e141bc950a1970c6bc96fa5f55fd60c6aedbb2d0 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 24 Aug 2010 16:50:15 -0400 Subject: [PATCH 07/87] Detect unused variables --- Source/cmMakefile.cxx | 65 +++++++++++++++++++++++++++++++++++++++++-- Source/cmMakefile.h | 6 ++++ 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index d89168df0..e4973cb5b 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -44,6 +44,7 @@ class cmMakefile::Internals public: std::stack > VarStack; std::stack > VarInitStack; + std::stack > VarUsageStack; std::set VarRemoved; }; @@ -91,6 +92,8 @@ cmMakefile::cmMakefile(): Internal(new Internals) this->AddDefaultDefinitions(); this->Initialize(); this->PreOrder = false; + this->FindUnused = false; + this->DefaultToUsed = false; } cmMakefile::cmMakefile(const cmMakefile& mf): Internal(new Internals) @@ -133,6 +136,8 @@ cmMakefile::cmMakefile(const cmMakefile& mf): Internal(new Internals) this->SubDirectoryOrder = mf.SubDirectoryOrder; this->Properties = mf.Properties; this->PreOrder = mf.PreOrder; + this->FindUnused = mf.FindUnused; + this->DefaultToUsed = mf.DefaultToUsed; this->ListFileStack = mf.ListFileStack; this->Initialize(); } @@ -757,6 +762,21 @@ void cmMakefile::SetLocalGenerator(cmLocalGenerator* lg) this->AddSourceGroup("Resources", "\\.plist$"); #endif + if (this->Internal->VarUsageStack.empty()) + { + const cmDefinitions& defs = cmDefinitions(); + const std::set globalKeys = defs.LocalKeys(); + this->FindUnused = this->GetCMakeInstance()->GetFindUnused(); + this->DefaultToUsed = this->GetCMakeInstance()->GetDefaultToUsed(); + if (this->FindUnused) + { + this->Internal->VarUsageStack.push(globalKeys); + } + else + { + this->Internal->VarUsageStack.push(std::set()); + } + } } bool cmMakefile::NeedBackwardsCompatibility(unsigned int major, @@ -1690,6 +1710,10 @@ void cmMakefile::AddDefinition(const char* name, bool value) { this->Internal->VarStack.top().Set(name, value? "ON" : "OFF"); this->Internal->VarInitStack.top().insert(name); + if (this->FindUnused && this->DefaultToUsed) + { + this->Internal->VarUsageStack.top().insert(name); + } #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); if ( vv ) @@ -1709,6 +1733,15 @@ bool cmMakefile::VariableInitialized(const char* var) const return false; } +bool cmMakefile::VariableUsed(const char* var) const +{ + if(this->Internal->VarUsageStack.top().find(var) != this->Internal->VarUsageStack.top().end()) + { + return true; + } + return false; +} + bool cmMakefile::VariableCleared(const char* var) const { if(this->Internal->VarRemoved.find(var) != this->Internal->VarRemoved.end()) @@ -1723,6 +1756,10 @@ void cmMakefile::RemoveDefinition(const char* name) this->Internal->VarStack.top().Set(name, 0); this->Internal->VarRemoved.insert(name); this->Internal->VarInitStack.top().insert(name); + if (this->FindUnused) + { + this->Internal->VarUsageStack.top().insert(name); + } #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); if ( vv ) @@ -2101,6 +2138,10 @@ const char* cmMakefile::GetDefinition(const char* name) const RecordPropertyAccess(name,cmProperty::VARIABLE); } #endif + if (this->FindUnused) + { + this->Internal->VarUsageStack.top().insert(name); + } const char* def = this->Internal->VarStack.top().Get(name); if(!def) { @@ -3332,29 +3373,49 @@ void cmMakefile::PushScope() { cmDefinitions* parent = &this->Internal->VarStack.top(); const std::set& init = this->Internal->VarInitStack.top(); + const std::set& usage = this->Internal->VarUsageStack.top(); this->Internal->VarStack.push(cmDefinitions(parent)); this->Internal->VarInitStack.push(init); + this->Internal->VarUsageStack.push(usage); } void cmMakefile::PopScope() { cmDefinitions* current = &this->Internal->VarStack.top(); std::set init = this->Internal->VarInitStack.top(); + std::set usage = this->Internal->VarUsageStack.top(); const std::set& locals = current->LocalKeys(); - // Remove initialization information for variables in the local scope. + // Remove initialization and usage information for variables in the local + // scope. std::set::const_iterator it = locals.begin(); for (; it != locals.end(); ++it) { init.erase(*it); + if (this->FindUnused && usage.find(*it) == usage.end()) + { + cmOStringStream m; + m << "unused variable \'" << *it << "\'"; + this->IssueMessage(cmake::AUTHOR_WARNING, m.str()); + } + else + { + usage.erase(*it); + } } this->Internal->VarStack.pop(); this->Internal->VarInitStack.pop(); - // Push initialization up to the parent scope. + this->Internal->VarUsageStack.pop(); + // Push initialization and usage up to the parent scope. it = init.begin(); for (; it != init.end(); ++it) { this->Internal->VarInitStack.top().insert(*it); } + it = usage.begin(); + for (; it != usage.end(); ++it) + { + this->Internal->VarUsageStack.top().insert(*it); + } } void cmMakefile::RaiseScope(const char *var, const char *varDef) diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index cec273805..184253a66 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -63,6 +63,8 @@ public: /* return true if a variable has been initialized */ bool VariableInitialized(const char* ) const; + /* return true if a variable has been used */ + bool VariableUsed(const char* ) const; /* return true if a variable has been set with set(foo ) */ @@ -931,6 +933,10 @@ private: // should this makefile be processed before or after processing the parent bool PreOrder; + // Unused variable flags + bool FindUnused; + bool DefaultToUsed; + // stack of list files being read std::deque ListFileStack; From d7999e9b294f93f68b5ec9e3efd8017fad3f05d9 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 25 Aug 2010 12:35:40 -0400 Subject: [PATCH 08/87] Rename strict-mode to warn-uninitialized --- Source/QtDialog/CMakeSetupDialog.cxx | 10 +++++----- Source/QtDialog/CMakeSetupDialog.h | 2 +- Source/QtDialog/QCMake.cxx | 10 +++++----- Source/QtDialog/QCMake.h | 6 +++--- Source/cmCommandArgumentParserHelper.cxx | 6 +++--- Source/cmCommandArgumentParserHelper.h | 2 +- Source/cmake.cxx | 8 ++++---- Source/cmake.h | 6 +++--- Source/cmakemain.cxx | 5 ++--- 9 files changed, 27 insertions(+), 28 deletions(-) diff --git a/Source/QtDialog/CMakeSetupDialog.cxx b/Source/QtDialog/CMakeSetupDialog.cxx index 7600897bd..a5510afc0 100644 --- a/Source/QtDialog/CMakeSetupDialog.cxx +++ b/Source/QtDialog/CMakeSetupDialog.cxx @@ -117,9 +117,9 @@ CMakeSetupDialog::CMakeSetupDialog() this->SuppressDevWarningsAction = OptionsMenu->addAction(tr("&Suppress dev Warnings (-Wno-dev)")); this->SuppressDevWarningsAction->setCheckable(true); - this->StrictModeAction = - OptionsMenu->addAction(tr("&Strict Mode (--strict-mode)")); - this->StrictModeAction->setCheckable(true); + this->WarnUninitializedAction = + OptionsMenu->addAction(tr("&Warn Uninitialized (--warn-uninitialized)")); + this->WarnUninitializedAction->setCheckable(true); QAction* debugAction = OptionsMenu->addAction(tr("&Debug Output")); debugAction->setCheckable(true); @@ -244,9 +244,9 @@ void CMakeSetupDialog::initialize() QObject::connect(this->SuppressDevWarningsAction, SIGNAL(triggered(bool)), this->CMakeThread->cmakeInstance(), SLOT(setSuppressDevWarnings(bool))); - QObject::connect(this->StrictModeAction, SIGNAL(triggered(bool)), + QObject::connect(this->WarnUninitializedAction, SIGNAL(triggered(bool)), this->CMakeThread->cmakeInstance(), - SLOT(setStrictMode(bool))); + SLOT(setWarnUninitializedMode(bool))); if(!this->SourceDirectory->text().isEmpty() || !this->BinaryDirectory->lineEdit()->text().isEmpty()) diff --git a/Source/QtDialog/CMakeSetupDialog.h b/Source/QtDialog/CMakeSetupDialog.h index cd2be6e5f..4c161d9e2 100644 --- a/Source/QtDialog/CMakeSetupDialog.h +++ b/Source/QtDialog/CMakeSetupDialog.h @@ -93,7 +93,7 @@ protected: QAction* ConfigureAction; QAction* GenerateAction; QAction* SuppressDevWarningsAction; - QAction* StrictModeAction; + QAction* WarnUninitializedAction; QAction* InstallForCommandLineAction; State CurrentState; diff --git a/Source/QtDialog/QCMake.cxx b/Source/QtDialog/QCMake.cxx index 31daf3c86..8d24c1c27 100644 --- a/Source/QtDialog/QCMake.cxx +++ b/Source/QtDialog/QCMake.cxx @@ -28,7 +28,7 @@ QCMake::QCMake(QObject* p) : QObject(p) { this->SuppressDevWarnings = false; - this->StrictMode = false; + this->WarnUninitializedMode = false; qRegisterMetaType(); qRegisterMetaType(); @@ -165,8 +165,8 @@ void QCMake::configure() this->CMakeInstance->CreateGlobalGenerator(this->Generator.toAscii().data())); this->CMakeInstance->LoadCache(); this->CMakeInstance->SetSuppressDevWarnings(this->SuppressDevWarnings); - std::cerr << "set strict " << this->StrictMode << "\n"; - this->CMakeInstance->SetStrictMode(this->StrictMode); + std::cerr << "set warn uninitialized " << this->WarnUninitializedMode << "\n"; + this->CMakeInstance->SetWarnUninitialized(this->WarnUninitializedMode); this->CMakeInstance->PreLoadCMakeFiles(); cmSystemTools::ResetErrorOccuredFlag(); @@ -421,7 +421,7 @@ void QCMake::setSuppressDevWarnings(bool value) this->SuppressDevWarnings = value; } -void QCMake::setStrictMode(bool value) +void QCMake::setWarnUninitializedMode(bool value) { - this->StrictMode = value; + this->WarnUninitializedMode = value; } diff --git a/Source/QtDialog/QCMake.h b/Source/QtDialog/QCMake.h index f20893f22..6d2b2fff5 100644 --- a/Source/QtDialog/QCMake.h +++ b/Source/QtDialog/QCMake.h @@ -88,8 +88,8 @@ public slots: void setDebugOutput(bool); /// set whether to do suppress dev warnings void setSuppressDevWarnings(bool value); - /// set whether to run cmake in strict mode - void setStrictMode(bool value); + /// set whether to run cmake with warnings about uninitialized variables + void setWarnUninitializedMode(bool value); public: /// get the list of cache properties @@ -135,7 +135,7 @@ protected: static void errorCallback(const char* msg, const char* title, bool&, void* cd); bool SuppressDevWarnings; - bool StrictMode; + bool WarnUninitializedMode; QString SourceDirectory; QString BinaryDirectory; QString Generator; diff --git a/Source/cmCommandArgumentParserHelper.cxx b/Source/cmCommandArgumentParserHelper.cxx index d955ff7e0..1460e5ddd 100644 --- a/Source/cmCommandArgumentParserHelper.cxx +++ b/Source/cmCommandArgumentParserHelper.cxx @@ -20,7 +20,7 @@ int cmCommandArgument_yyparse( yyscan_t yyscanner ); // cmCommandArgumentParserHelper::cmCommandArgumentParserHelper() { - this->StrictMode = false; + this->WarnUninitialized = false; this->FileLine = -1; this->FileName = 0; this->RemoveEmpty = true; @@ -127,7 +127,7 @@ char* cmCommandArgumentParserHelper::ExpandVariable(const char* var) // check to see if we need to print a warning // if strict mode is on and the variable has // not been "cleared"/initialized with a set(foo ) call - if(this->StrictMode && !this->Makefile->VariableInitialized(var)) + if(this->WarnUninitialized && !this->Makefile->VariableInitialized(var)) { cmOStringStream msg; msg << this->FileName << ":" << this->FileLine << ":" << @@ -330,7 +330,7 @@ void cmCommandArgumentParserHelper::Error(const char* str) void cmCommandArgumentParserHelper::SetMakefile(const cmMakefile* mf) { this->Makefile = mf; - this->StrictMode = mf->GetCMakeInstance()->GetStrictMode(); + this->WarnUninitialized = mf->GetCMakeInstance()->GetWarnUninitialized(); } void cmCommandArgumentParserHelper::SetResult(const char* value) diff --git a/Source/cmCommandArgumentParserHelper.h b/Source/cmCommandArgumentParserHelper.h index d7206ee97..1df004244 100644 --- a/Source/cmCommandArgumentParserHelper.h +++ b/Source/cmCommandArgumentParserHelper.h @@ -96,7 +96,7 @@ private: const cmMakefile* Makefile; std::string Result; const char* FileName; - bool StrictMode; + bool WarnUninitialized; long FileLine; bool EscapeQuotes; std::string ErrorString; diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 41f2775ac..f68ab3ef1 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -140,7 +140,7 @@ void cmNeedBackwardsCompatibility(const std::string& variable, cmake::cmake() { this->Trace = false; - this->StrictMode = false; + this->WarnUninitialized = false; this->FindUnused = false; this->DefaultToUsed = false; this->SuppressDevWarnings = false; @@ -616,10 +616,10 @@ void cmake::SetArgs(const std::vector& args) std::cout << "Running with trace output on.\n"; this->SetTrace(true); } - else if(arg.find("--strict-mode",0) == 0) + else if(arg.find("--warn-uninitialized",0) == 0) { - std::cout << "Running in strict mode.\n"; - this->SetStrictMode(true); + std::cout << "Warn about uninitialized values.\n"; + this->SetWarnUninitialized(true); } else if(arg.find("--find-unused",0) == 0) { diff --git a/Source/cmake.h b/Source/cmake.h index 7b612ea70..3a53d481d 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -306,8 +306,8 @@ class cmake // Do we want trace output during the cmake run. bool GetTrace() { return this->Trace;} void SetTrace(bool b) { this->Trace = b;} - bool GetStrictMode() { return this->StrictMode;} - void SetStrictMode(bool b) { this->StrictMode = b;} + bool GetWarnUninitialized() { return this->WarnUninitialized;} + void SetWarnUninitialized(bool b) { this->WarnUninitialized = b;} bool GetFindUnused() { return this->FindUnused;} void SetFindUnused(bool b) { this->FindUnused = b;} bool GetDefaultToUsed() { return this->DefaultToUsed;} @@ -449,7 +449,7 @@ private: bool ScriptMode; bool DebugOutput; bool Trace; - bool StrictMode; + bool WarnUninitialized; bool FindUnused; bool DefaultToUsed; std::string CMakeEditCommand; diff --git a/Source/cmakemain.cxx b/Source/cmakemain.cxx index 235aaf527..af922000b 100644 --- a/Source/cmakemain.cxx +++ b/Source/cmakemain.cxx @@ -120,9 +120,8 @@ static const char * cmDocumentationOptions[][3] = {"--trace", "Put cmake in trace mode.", "Print a trace of all calls made and from where with " "message(send_error ) calls."}, - {"--strict-mode", "Put cmake in strict mode.", - "In strict mode cmake will print a warning when an uninitialized variable " - "is used."}, + {"--warn-uninitialized", "Warn about uninitialized values.", + "Print a warning when an uninitialized variable is used."}, {"--find-unused", "Find unused variables.", "Find variables that are declared on the command line, but not used."}, {"--find-unused-all", "Find all unused variables.", From 4ff03402fc137bf5624d4a71c1ad3b177e5ceb53 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 25 Aug 2010 12:36:21 -0400 Subject: [PATCH 09/87] Rename find-unused to warn-unused --- Source/cmMakefile.cxx | 16 ++++++++-------- Source/cmMakefile.h | 2 +- Source/cmake.cxx | 10 +++++----- Source/cmake.h | 6 +++--- Source/cmakemain.cxx | 4 ++-- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index e4973cb5b..214c8bece 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -92,7 +92,7 @@ cmMakefile::cmMakefile(): Internal(new Internals) this->AddDefaultDefinitions(); this->Initialize(); this->PreOrder = false; - this->FindUnused = false; + this->WarnUnused = false; this->DefaultToUsed = false; } @@ -136,7 +136,7 @@ cmMakefile::cmMakefile(const cmMakefile& mf): Internal(new Internals) this->SubDirectoryOrder = mf.SubDirectoryOrder; this->Properties = mf.Properties; this->PreOrder = mf.PreOrder; - this->FindUnused = mf.FindUnused; + this->WarnUnused = mf.WarnUnused; this->DefaultToUsed = mf.DefaultToUsed; this->ListFileStack = mf.ListFileStack; this->Initialize(); @@ -766,9 +766,9 @@ void cmMakefile::SetLocalGenerator(cmLocalGenerator* lg) { const cmDefinitions& defs = cmDefinitions(); const std::set globalKeys = defs.LocalKeys(); - this->FindUnused = this->GetCMakeInstance()->GetFindUnused(); + this->WarnUnused = this->GetCMakeInstance()->GetWarnUnused(); this->DefaultToUsed = this->GetCMakeInstance()->GetDefaultToUsed(); - if (this->FindUnused) + if (this->WarnUnused) { this->Internal->VarUsageStack.push(globalKeys); } @@ -1710,7 +1710,7 @@ void cmMakefile::AddDefinition(const char* name, bool value) { this->Internal->VarStack.top().Set(name, value? "ON" : "OFF"); this->Internal->VarInitStack.top().insert(name); - if (this->FindUnused && this->DefaultToUsed) + if (this->WarnUnused && this->DefaultToUsed) { this->Internal->VarUsageStack.top().insert(name); } @@ -1756,7 +1756,7 @@ void cmMakefile::RemoveDefinition(const char* name) this->Internal->VarStack.top().Set(name, 0); this->Internal->VarRemoved.insert(name); this->Internal->VarInitStack.top().insert(name); - if (this->FindUnused) + if (this->WarnUnused) { this->Internal->VarUsageStack.top().insert(name); } @@ -2138,7 +2138,7 @@ const char* cmMakefile::GetDefinition(const char* name) const RecordPropertyAccess(name,cmProperty::VARIABLE); } #endif - if (this->FindUnused) + if (this->WarnUnused) { this->Internal->VarUsageStack.top().insert(name); } @@ -3391,7 +3391,7 @@ void cmMakefile::PopScope() for (; it != locals.end(); ++it) { init.erase(*it); - if (this->FindUnused && usage.find(*it) == usage.end()) + if (this->WarnUnused && usage.find(*it) == usage.end()) { cmOStringStream m; m << "unused variable \'" << *it << "\'"; diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 184253a66..714174734 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -934,7 +934,7 @@ private: bool PreOrder; // Unused variable flags - bool FindUnused; + bool WarnUnused; bool DefaultToUsed; // stack of list files being read diff --git a/Source/cmake.cxx b/Source/cmake.cxx index f68ab3ef1..e68f58d4f 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -141,7 +141,7 @@ cmake::cmake() { this->Trace = false; this->WarnUninitialized = false; - this->FindUnused = false; + this->WarnUnused = false; this->DefaultToUsed = false; this->SuppressDevWarnings = false; this->DoSuppressDevWarnings = false; @@ -621,16 +621,16 @@ void cmake::SetArgs(const std::vector& args) std::cout << "Warn about uninitialized values.\n"; this->SetWarnUninitialized(true); } - else if(arg.find("--find-unused",0) == 0) + else if(arg.find("--warn-unused",0) == 0) { std::cout << "Finding unused command line variables.\n"; - this->SetFindUnused(true); + this->SetWarnUnused(true); this->SetDefaultToUsed(true); } - else if(arg.find("--find-unused-all",0) == 0) + else if(arg.find("--warn-unused-all",0) == 0) { std::cout << "Finding unused variables.\n"; - this->SetFindUnused(true); + this->SetWarnUnused(true); this->SetDefaultToUsed(false); } else if(arg.find("-G",0) == 0) diff --git a/Source/cmake.h b/Source/cmake.h index 3a53d481d..7304d94e1 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -308,8 +308,8 @@ class cmake void SetTrace(bool b) { this->Trace = b;} bool GetWarnUninitialized() { return this->WarnUninitialized;} void SetWarnUninitialized(bool b) { this->WarnUninitialized = b;} - bool GetFindUnused() { return this->FindUnused;} - void SetFindUnused(bool b) { this->FindUnused = b;} + bool GetWarnUnused() { return this->WarnUnused;} + void SetWarnUnused(bool b) { this->WarnUnused = b;} bool GetDefaultToUsed() { return this->DefaultToUsed;} void SetDefaultToUsed(bool b) { this->DefaultToUsed = b;} // Define a property @@ -450,7 +450,7 @@ private: bool DebugOutput; bool Trace; bool WarnUninitialized; - bool FindUnused; + bool WarnUnused; bool DefaultToUsed; std::string CMakeEditCommand; std::string CMakeCommand; diff --git a/Source/cmakemain.cxx b/Source/cmakemain.cxx index af922000b..8d4c3344c 100644 --- a/Source/cmakemain.cxx +++ b/Source/cmakemain.cxx @@ -122,9 +122,9 @@ static const char * cmDocumentationOptions[][3] = "message(send_error ) calls."}, {"--warn-uninitialized", "Warn about uninitialized values.", "Print a warning when an uninitialized variable is used."}, - {"--find-unused", "Find unused variables.", + {"--warn-unused", "Warn about unused variables.", "Find variables that are declared on the command line, but not used."}, - {"--find-unused-all", "Find all unused variables.", + {"--warn-unused-all", "Warn about all unused variables.", "Find variables that are declared, but not used."}, {"--help-command cmd [file]", "Print help for a single command and exit.", "Full documentation specific to the given command is displayed. " From 636e6c4ef7e79113802714dbc7ade77d4f04e809 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 25 Aug 2010 12:42:03 -0400 Subject: [PATCH 10/87] Default to marking things as used If we don't then: cmake --warn-unused --warn-unused-all acts differently than: cmake --warn-unused-all --warn-unused --- Source/cmMakefile.cxx | 2 +- Source/cmake.cxx | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 214c8bece..406062d29 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -93,7 +93,7 @@ cmMakefile::cmMakefile(): Internal(new Internals) this->Initialize(); this->PreOrder = false; this->WarnUnused = false; - this->DefaultToUsed = false; + this->DefaultToUsed = true; } cmMakefile::cmMakefile(const cmMakefile& mf): Internal(new Internals) diff --git a/Source/cmake.cxx b/Source/cmake.cxx index e68f58d4f..cd9d10d04 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -142,7 +142,7 @@ cmake::cmake() this->Trace = false; this->WarnUninitialized = false; this->WarnUnused = false; - this->DefaultToUsed = false; + this->DefaultToUsed = true; this->SuppressDevWarnings = false; this->DoSuppressDevWarnings = false; this->DebugOutput = false; @@ -625,7 +625,6 @@ void cmake::SetArgs(const std::vector& args) { std::cout << "Finding unused command line variables.\n"; this->SetWarnUnused(true); - this->SetDefaultToUsed(true); } else if(arg.find("--warn-unused-all",0) == 0) { From 786e2695cb68402d44357002b438c95229c4fb19 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 25 Aug 2010 12:43:02 -0400 Subject: [PATCH 11/87] Add warn-unused to the Qt interface --- Source/QtDialog/CMakeSetupDialog.cxx | 12 ++++++++++++ Source/QtDialog/CMakeSetupDialog.h | 2 ++ Source/QtDialog/QCMake.cxx | 14 ++++++++++++++ Source/QtDialog/QCMake.h | 6 ++++++ 4 files changed, 34 insertions(+) diff --git a/Source/QtDialog/CMakeSetupDialog.cxx b/Source/QtDialog/CMakeSetupDialog.cxx index a5510afc0..a09504df4 100644 --- a/Source/QtDialog/CMakeSetupDialog.cxx +++ b/Source/QtDialog/CMakeSetupDialog.cxx @@ -120,6 +120,12 @@ CMakeSetupDialog::CMakeSetupDialog() this->WarnUninitializedAction = OptionsMenu->addAction(tr("&Warn Uninitialized (--warn-uninitialized)")); this->WarnUninitializedAction->setCheckable(true); + this->WarnUnusedAction = + OptionsMenu->addAction(tr("&Warn Unused (--warn-unused)")); + this->WarnUnusedAction->setCheckable(true); + this->WarnUnusedAllAction = + OptionsMenu->addAction(tr("&Warn Unused All (--warn-unused-all)")); + this->WarnUnusedAllAction->setCheckable(true); QAction* debugAction = OptionsMenu->addAction(tr("&Debug Output")); debugAction->setCheckable(true); @@ -247,6 +253,12 @@ 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))); + QObject::connect(this->WarnUnusedAllAction, SIGNAL(triggered(bool)), + this->CMakeThread->cmakeInstance(), + SLOT(setWarnUnusedAllMode(bool))); if(!this->SourceDirectory->text().isEmpty() || !this->BinaryDirectory->lineEdit()->text().isEmpty()) diff --git a/Source/QtDialog/CMakeSetupDialog.h b/Source/QtDialog/CMakeSetupDialog.h index 4c161d9e2..f9372481c 100644 --- a/Source/QtDialog/CMakeSetupDialog.h +++ b/Source/QtDialog/CMakeSetupDialog.h @@ -94,6 +94,8 @@ protected: QAction* GenerateAction; QAction* SuppressDevWarningsAction; QAction* WarnUninitializedAction; + QAction* WarnUnusedAction; + QAction* WarnUnusedAllAction; QAction* InstallForCommandLineAction; State CurrentState; diff --git a/Source/QtDialog/QCMake.cxx b/Source/QtDialog/QCMake.cxx index 8d24c1c27..1f9fa3dd3 100644 --- a/Source/QtDialog/QCMake.cxx +++ b/Source/QtDialog/QCMake.cxx @@ -29,6 +29,8 @@ QCMake::QCMake(QObject* p) { this->SuppressDevWarnings = false; this->WarnUninitializedMode = false; + this->WarnUnusedMode = false; + this->WarnUnusedAllMode = false; qRegisterMetaType(); qRegisterMetaType(); @@ -167,6 +169,8 @@ void QCMake::configure() this->CMakeInstance->SetSuppressDevWarnings(this->SuppressDevWarnings); std::cerr << "set warn uninitialized " << this->WarnUninitializedMode << "\n"; this->CMakeInstance->SetWarnUninitialized(this->WarnUninitializedMode); + this->CMakeInstance->SetWarnUnused(this->WarnUnusedMode); + this->CMakeInstance->SetDefaultToUsed(!this->WarnUnusedAllMode); this->CMakeInstance->PreLoadCMakeFiles(); cmSystemTools::ResetErrorOccuredFlag(); @@ -425,3 +429,13 @@ void QCMake::setWarnUninitializedMode(bool value) { this->WarnUninitializedMode = value; } + +void QCMake::setWarnUnusedMode(bool value) +{ + this->WarnUnusedMode = value; +} + +void QCMake::setWarnUnusedAllMode(bool value) +{ + this->WarnUnusedAllMode = value; +} diff --git a/Source/QtDialog/QCMake.h b/Source/QtDialog/QCMake.h index 6d2b2fff5..6056a8c8c 100644 --- a/Source/QtDialog/QCMake.h +++ b/Source/QtDialog/QCMake.h @@ -90,6 +90,10 @@ public slots: void setSuppressDevWarnings(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); + /// set whether to run cmake with warnings about all unused variables + void setWarnUnusedAllMode(bool value); public: /// get the list of cache properties @@ -136,6 +140,8 @@ protected: bool&, void* cd); bool SuppressDevWarnings; bool WarnUninitializedMode; + bool WarnUnusedMode; + bool WarnUnusedAllMode; QString SourceDirectory; QString BinaryDirectory; QString Generator; From fff9f6d6f74aa92d0bc4adf3a80a25b1b662458d Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 1 Sep 2010 10:22:08 -0400 Subject: [PATCH 12/87] Rename flags again and use variablewatch for cli --- Source/QtDialog/CMakeSetupDialog.cxx | 8 +---- Source/QtDialog/CMakeSetupDialog.h | 1 - Source/QtDialog/QCMake.cxx | 7 ----- Source/QtDialog/QCMake.h | 2 -- Source/cmMakefile.cxx | 7 ----- Source/cmMakefile.h | 1 - Source/cmake.cxx | 45 +++++++++++++++++++++++----- Source/cmake.h | 10 +++++-- Source/cmakemain.cxx | 6 ++-- 9 files changed, 48 insertions(+), 39 deletions(-) diff --git a/Source/QtDialog/CMakeSetupDialog.cxx b/Source/QtDialog/CMakeSetupDialog.cxx index a09504df4..663753ebc 100644 --- a/Source/QtDialog/CMakeSetupDialog.cxx +++ b/Source/QtDialog/CMakeSetupDialog.cxx @@ -121,11 +121,8 @@ CMakeSetupDialog::CMakeSetupDialog() OptionsMenu->addAction(tr("&Warn Uninitialized (--warn-uninitialized)")); this->WarnUninitializedAction->setCheckable(true); this->WarnUnusedAction = - OptionsMenu->addAction(tr("&Warn Unused (--warn-unused)")); + OptionsMenu->addAction(tr("&Warn Unused (--warn-unused-vars)")); this->WarnUnusedAction->setCheckable(true); - this->WarnUnusedAllAction = - OptionsMenu->addAction(tr("&Warn Unused All (--warn-unused-all)")); - this->WarnUnusedAllAction->setCheckable(true); QAction* debugAction = OptionsMenu->addAction(tr("&Debug Output")); debugAction->setCheckable(true); @@ -256,9 +253,6 @@ void CMakeSetupDialog::initialize() QObject::connect(this->WarnUnusedAction, SIGNAL(triggered(bool)), this->CMakeThread->cmakeInstance(), SLOT(setWarnUnusedMode(bool))); - QObject::connect(this->WarnUnusedAllAction, SIGNAL(triggered(bool)), - this->CMakeThread->cmakeInstance(), - SLOT(setWarnUnusedAllMode(bool))); if(!this->SourceDirectory->text().isEmpty() || !this->BinaryDirectory->lineEdit()->text().isEmpty()) diff --git a/Source/QtDialog/CMakeSetupDialog.h b/Source/QtDialog/CMakeSetupDialog.h index f9372481c..c4d029aa3 100644 --- a/Source/QtDialog/CMakeSetupDialog.h +++ b/Source/QtDialog/CMakeSetupDialog.h @@ -95,7 +95,6 @@ protected: QAction* SuppressDevWarningsAction; QAction* WarnUninitializedAction; QAction* WarnUnusedAction; - QAction* WarnUnusedAllAction; QAction* InstallForCommandLineAction; State CurrentState; diff --git a/Source/QtDialog/QCMake.cxx b/Source/QtDialog/QCMake.cxx index 1f9fa3dd3..c319cb43d 100644 --- a/Source/QtDialog/QCMake.cxx +++ b/Source/QtDialog/QCMake.cxx @@ -30,7 +30,6 @@ QCMake::QCMake(QObject* p) this->SuppressDevWarnings = false; this->WarnUninitializedMode = false; this->WarnUnusedMode = false; - this->WarnUnusedAllMode = false; qRegisterMetaType(); qRegisterMetaType(); @@ -170,7 +169,6 @@ void QCMake::configure() std::cerr << "set warn uninitialized " << this->WarnUninitializedMode << "\n"; this->CMakeInstance->SetWarnUninitialized(this->WarnUninitializedMode); this->CMakeInstance->SetWarnUnused(this->WarnUnusedMode); - this->CMakeInstance->SetDefaultToUsed(!this->WarnUnusedAllMode); this->CMakeInstance->PreLoadCMakeFiles(); cmSystemTools::ResetErrorOccuredFlag(); @@ -434,8 +432,3 @@ void QCMake::setWarnUnusedMode(bool value) { this->WarnUnusedMode = value; } - -void QCMake::setWarnUnusedAllMode(bool value) -{ - this->WarnUnusedAllMode = value; -} diff --git a/Source/QtDialog/QCMake.h b/Source/QtDialog/QCMake.h index 6056a8c8c..0d10823f3 100644 --- a/Source/QtDialog/QCMake.h +++ b/Source/QtDialog/QCMake.h @@ -92,8 +92,6 @@ public slots: void setWarnUninitializedMode(bool value); /// set whether to run cmake with warnings about unused variables void setWarnUnusedMode(bool value); - /// set whether to run cmake with warnings about all unused variables - void setWarnUnusedAllMode(bool value); public: /// get the list of cache properties diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 406062d29..242900ec0 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -93,7 +93,6 @@ cmMakefile::cmMakefile(): Internal(new Internals) this->Initialize(); this->PreOrder = false; this->WarnUnused = false; - this->DefaultToUsed = true; } cmMakefile::cmMakefile(const cmMakefile& mf): Internal(new Internals) @@ -137,7 +136,6 @@ cmMakefile::cmMakefile(const cmMakefile& mf): Internal(new Internals) this->Properties = mf.Properties; this->PreOrder = mf.PreOrder; this->WarnUnused = mf.WarnUnused; - this->DefaultToUsed = mf.DefaultToUsed; this->ListFileStack = mf.ListFileStack; this->Initialize(); } @@ -767,7 +765,6 @@ void cmMakefile::SetLocalGenerator(cmLocalGenerator* lg) const cmDefinitions& defs = cmDefinitions(); const std::set globalKeys = defs.LocalKeys(); this->WarnUnused = this->GetCMakeInstance()->GetWarnUnused(); - this->DefaultToUsed = this->GetCMakeInstance()->GetDefaultToUsed(); if (this->WarnUnused) { this->Internal->VarUsageStack.push(globalKeys); @@ -1710,10 +1707,6 @@ void cmMakefile::AddDefinition(const char* name, bool value) { this->Internal->VarStack.top().Set(name, value? "ON" : "OFF"); this->Internal->VarInitStack.top().insert(name); - if (this->WarnUnused && this->DefaultToUsed) - { - this->Internal->VarUsageStack.top().insert(name); - } #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); if ( vv ) diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 714174734..4ce3b9b67 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -935,7 +935,6 @@ private: // Unused variable flags bool WarnUnused; - bool DefaultToUsed; // stack of list files being read std::deque ListFileStack; diff --git a/Source/cmake.cxx b/Source/cmake.cxx index cd9d10d04..93ca9e3cb 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -137,12 +137,19 @@ void cmNeedBackwardsCompatibility(const std::string& variable, #endif } +void cmWarnUnusedCliWarning(const std::string& variable, + int, void* ctx, const char*, const cmMakefile*) +{ + cmake* cm = reinterpret_cast(ctx); + cm->MarkCliAsUsed(variable); +} + cmake::cmake() { this->Trace = false; this->WarnUninitialized = false; this->WarnUnused = false; - this->DefaultToUsed = true; + this->WarnUnusedCli = true; this->SuppressDevWarnings = false; this->DoSuppressDevWarnings = false; this->DebugOutput = false; @@ -193,6 +200,19 @@ cmake::cmake() cmake::~cmake() { + if(this->WarnUnusedCli) + { + std::map::const_iterator it; + for(it = this->UsedCliVariables.begin(); it != this->UsedCliVariables.end(); ++it) + { + if(!it->second) + { + std::string message = "The variable, \"" + it->first + "\", given " + "on the command line was not used within the build."; + cmSystemTools::Message(message.c_str()); + } + } + } delete this->CacheManager; delete this->Policies; if (this->GlobalGenerator) @@ -370,6 +390,11 @@ bool cmake::SetCacheArgs(const std::vector& args) { this->CacheManager->AddCacheEntry(var.c_str(), value.c_str(), "No help, variable specified on the command line.", type); + if(this->WarnUnusedCli) + { + this->VariableWatch->AddWatch(var, cmWarnUnusedCliWarning, this); + this->UsedCliVariables[var] = false; + } } else { @@ -621,16 +646,15 @@ void cmake::SetArgs(const std::vector& args) std::cout << "Warn about uninitialized values.\n"; this->SetWarnUninitialized(true); } - else if(arg.find("--warn-unused",0) == 0) - { - std::cout << "Finding unused command line variables.\n"; - this->SetWarnUnused(true); - } - else if(arg.find("--warn-unused-all",0) == 0) + else if(arg.find("--warn-unused-vars",0) == 0) { std::cout << "Finding unused variables.\n"; this->SetWarnUnused(true); - this->SetDefaultToUsed(false); + } + else if(arg.find("--warn-unused-cli",0) == 0) + { + std::cout << "Finding unused variables given on the command line.\n"; + this->SetWarnUnusedCli(true); } else if(arg.find("-G",0) == 0) { @@ -2836,6 +2860,11 @@ const char* cmake::GetCPackCommand() return this->CPackCommand.c_str(); } +void cmake::MarkCliAsUsed(const std::string& variable) +{ + this->UsedCliVariables[variable] = true; +} + void cmake::GenerateGraphViz(const char* fileName) const { cmGeneratedFileStream str(fileName); diff --git a/Source/cmake.h b/Source/cmake.h index 7304d94e1..7f7546a12 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -310,8 +310,11 @@ class cmake void SetWarnUninitialized(bool b) { this->WarnUninitialized = b;} bool GetWarnUnused() { return this->WarnUnused;} void SetWarnUnused(bool b) { this->WarnUnused = b;} - bool GetDefaultToUsed() { return this->DefaultToUsed;} - void SetDefaultToUsed(bool b) { this->DefaultToUsed = b;} + bool GetWarnUnusedCli() { return this->WarnUnusedCli;} + void SetWarnUnusedCli(bool b) { this->WarnUnusedCli = b;} + + void MarkCliAsUsed(const std::string& variable); + // Define a property void DefineProperty(const char *name, cmProperty::ScopeType scope, const char *ShortDescription, @@ -451,7 +454,8 @@ private: bool Trace; bool WarnUninitialized; bool WarnUnused; - bool DefaultToUsed; + bool WarnUnusedCli; + std::map UsedCliVariables; std::string CMakeEditCommand; std::string CMakeCommand; std::string CXXEnvironment; diff --git a/Source/cmakemain.cxx b/Source/cmakemain.cxx index 8d4c3344c..cb3fcb04e 100644 --- a/Source/cmakemain.cxx +++ b/Source/cmakemain.cxx @@ -122,10 +122,10 @@ static const char * cmDocumentationOptions[][3] = "message(send_error ) calls."}, {"--warn-uninitialized", "Warn about uninitialized values.", "Print a warning when an uninitialized variable is used."}, - {"--warn-unused", "Warn about unused variables.", + {"--warn-unused-all", "Warn about unused variables.", + "Find variables that are declared or set, but not used."}, + {"--warn-unused-cli", "Warn about command line options.", "Find variables that are declared on the command line, but not used."}, - {"--warn-unused-all", "Warn about all unused variables.", - "Find variables that are declared, but not used."}, {"--help-command cmd [file]", "Print help for a single command and exit.", "Full documentation specific to the given command is displayed. " "If a file is specified, the documentation is written into and the output " From 74997000c89ee3a82d68e4107d4a4264e7e57229 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 1 Sep 2010 11:24:20 -0400 Subject: [PATCH 13/87] Add a flag to warn about system files --- Source/cmCommandArgumentParserHelper.cxx | 14 ++++++++++---- Source/cmCommandArgumentParserHelper.h | 1 + Source/cmMakefile.cxx | 14 +++++++++++--- Source/cmMakefile.h | 1 + Source/cmake.cxx | 6 ++++++ Source/cmake.h | 3 +++ 6 files changed, 32 insertions(+), 7 deletions(-) diff --git a/Source/cmCommandArgumentParserHelper.cxx b/Source/cmCommandArgumentParserHelper.cxx index 1460e5ddd..e9381d4c6 100644 --- a/Source/cmCommandArgumentParserHelper.cxx +++ b/Source/cmCommandArgumentParserHelper.cxx @@ -21,6 +21,7 @@ int cmCommandArgument_yyparse( yyscan_t yyscanner ); cmCommandArgumentParserHelper::cmCommandArgumentParserHelper() { this->WarnUninitialized = false; + this->CheckSystemVars = false; this->FileLine = -1; this->FileName = 0; this->RemoveEmpty = true; @@ -129,10 +130,14 @@ char* cmCommandArgumentParserHelper::ExpandVariable(const char* var) // not been "cleared"/initialized with a set(foo ) call if(this->WarnUninitialized && !this->Makefile->VariableInitialized(var)) { - cmOStringStream msg; - msg << this->FileName << ":" << this->FileLine << ":" << - " warning: uninitialized variable \'" << var << "\'"; - cmSystemTools::Message(msg.str().c_str()); + const char* root = this->Makefile->GetDefinition("CMAKE_ROOT"); + if (this->CheckSystemVars || strstr(this->FileName, root) != this->FileName) + { + cmOStringStream msg; + msg << this->FileName << ":" << this->FileLine << ":" << + " warning: uninitialized variable \'" << var << "\'"; + cmSystemTools::Message(msg.str().c_str()); + } } return 0; } @@ -331,6 +336,7 @@ void cmCommandArgumentParserHelper::SetMakefile(const cmMakefile* mf) { this->Makefile = mf; this->WarnUninitialized = mf->GetCMakeInstance()->GetWarnUninitialized(); + this->CheckSystemVars = mf->GetCMakeInstance()->GetCheckSystemVars(); } void cmCommandArgumentParserHelper::SetResult(const char* value) diff --git a/Source/cmCommandArgumentParserHelper.h b/Source/cmCommandArgumentParserHelper.h index 1df004244..a211e952b 100644 --- a/Source/cmCommandArgumentParserHelper.h +++ b/Source/cmCommandArgumentParserHelper.h @@ -97,6 +97,7 @@ private: std::string Result; const char* FileName; bool WarnUninitialized; + bool CheckSystemVars; long FileLine; bool EscapeQuotes; std::string ErrorString; diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 242900ec0..3a235905b 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -93,6 +93,7 @@ cmMakefile::cmMakefile(): Internal(new Internals) this->Initialize(); this->PreOrder = false; this->WarnUnused = false; + this->CheckSystemVars = false; } cmMakefile::cmMakefile(const cmMakefile& mf): Internal(new Internals) @@ -136,6 +137,7 @@ cmMakefile::cmMakefile(const cmMakefile& mf): Internal(new Internals) this->Properties = mf.Properties; this->PreOrder = mf.PreOrder; this->WarnUnused = mf.WarnUnused; + this->CheckSystemVars = mf.CheckSystemVars; this->ListFileStack = mf.ListFileStack; this->Initialize(); } @@ -774,6 +776,7 @@ void cmMakefile::SetLocalGenerator(cmLocalGenerator* lg) this->Internal->VarUsageStack.push(std::set()); } } + this->CheckSystemVars = this->GetCMakeInstance()->GetCheckSystemVars(); } bool cmMakefile::NeedBackwardsCompatibility(unsigned int major, @@ -3386,9 +3389,14 @@ void cmMakefile::PopScope() init.erase(*it); if (this->WarnUnused && usage.find(*it) == usage.end()) { - cmOStringStream m; - m << "unused variable \'" << *it << "\'"; - this->IssueMessage(cmake::AUTHOR_WARNING, m.str()); + const char* cdir = this->ListFileStack.back().c_str(); + const char* root = this->GetDefinition("CMAKE_ROOT"); + if (this->CheckSystemVars || strstr(cdir, root) != cdir) + { + cmOStringStream m; + m << "unused variable \'" << *it << "\'"; + this->IssueMessage(cmake::AUTHOR_WARNING, m.str()); + } } else { diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 4ce3b9b67..f1ad54d3f 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -935,6 +935,7 @@ private: // Unused variable flags bool WarnUnused; + bool CheckSystemVars; // stack of list files being read std::deque ListFileStack; diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 93ca9e3cb..27bfbeedb 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -150,6 +150,7 @@ cmake::cmake() this->WarnUninitialized = false; this->WarnUnused = false; this->WarnUnusedCli = true; + this->CheckSystemVars = false; this->SuppressDevWarnings = false; this->DoSuppressDevWarnings = false; this->DebugOutput = false; @@ -656,6 +657,11 @@ void cmake::SetArgs(const std::vector& args) std::cout << "Finding unused variables given on the command line.\n"; this->SetWarnUnusedCli(true); } + else if(arg.find("--check-system-vars",0) == 0) + { + std::cout << "Also check system files when warning about unused and uninitialized variables.\n"; + this->SetCheckSystemVars(true); + } else if(arg.find("-G",0) == 0) { std::string value = arg.substr(2); diff --git a/Source/cmake.h b/Source/cmake.h index 7f7546a12..403809fcd 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -312,6 +312,8 @@ class cmake void SetWarnUnused(bool b) { this->WarnUnused = b;} bool GetWarnUnusedCli() { return this->WarnUnusedCli;} void SetWarnUnusedCli(bool b) { this->WarnUnusedCli = b;} + bool GetCheckSystemVars() { return this->CheckSystemVars;} + void SetCheckSystemVars(bool b) { this->CheckSystemVars = b;} void MarkCliAsUsed(const std::string& variable); @@ -455,6 +457,7 @@ private: bool WarnUninitialized; bool WarnUnused; bool WarnUnusedCli; + bool CheckSystemVars; std::map UsedCliVariables; std::string CMakeEditCommand; std::string CMakeCommand; From 2e78224509d6d0bbb7fdf28841f40a6b41c565b3 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 1 Sep 2010 11:26:58 -0400 Subject: [PATCH 14/87] Add a missing comma to the warning message --- Source/cmake.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 27bfbeedb..d4cabdb17 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -208,8 +208,8 @@ cmake::~cmake() { if(!it->second) { - std::string message = "The variable, \"" + it->first + "\", given " - "on the command line was not used within the build."; + std::string message = "warning: The variable, \"" + it->first + "\", given " + "on the command line, was not used within the build."; cmSystemTools::Message(message.c_str()); } } From 9efc05722e9598b8e34062e8f39457b728b7286b Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 2 Sep 2010 09:10:52 -0400 Subject: [PATCH 15/87] VariableWatch is not available when bootstrapping --- Source/cmake.cxx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Source/cmake.cxx b/Source/cmake.cxx index d4cabdb17..cb70f1f91 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -201,6 +201,7 @@ cmake::cmake() cmake::~cmake() { +#ifdef CMAKE_BUILD_WITH_CMAKE if(this->WarnUnusedCli) { std::map::const_iterator it; @@ -214,6 +215,7 @@ cmake::~cmake() } } } +#endif delete this->CacheManager; delete this->Policies; if (this->GlobalGenerator) @@ -393,8 +395,10 @@ bool cmake::SetCacheArgs(const std::vector& args) "No help, variable specified on the command line.", type); if(this->WarnUnusedCli) { +#ifdef CMAKE_BUILD_WITH_CMAKE this->VariableWatch->AddWatch(var, cmWarnUnusedCliWarning, this); this->UsedCliVariables[var] = false; +#endif } } else From d784e6af4d292e5ffee4b43b7ee740d1fa8e2e29 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 2 Sep 2010 11:29:05 -0400 Subject: [PATCH 16/87] Run the unused variables check on the final pass --- Source/cmMakefile.cxx | 2 ++ Source/cmake.cxx | 34 +++++++++++++++++++--------------- Source/cmake.h | 2 ++ 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 3a235905b..288bc531a 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -828,6 +828,8 @@ void cmMakefile::ConfigureFinalPass() { l->second.FinishConfigure(); } + + this->GetCMakeInstance()->RunCheckForUnusedVariables(); } //---------------------------------------------------------------------------- diff --git a/Source/cmake.cxx b/Source/cmake.cxx index cb70f1f91..b3261c59b 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -201,21 +201,6 @@ cmake::cmake() cmake::~cmake() { -#ifdef CMAKE_BUILD_WITH_CMAKE - if(this->WarnUnusedCli) - { - std::map::const_iterator it; - for(it = this->UsedCliVariables.begin(); it != this->UsedCliVariables.end(); ++it) - { - if(!it->second) - { - std::string message = "warning: The variable, \"" + it->first + "\", given " - "on the command line, was not used within the build."; - cmSystemTools::Message(message.c_str()); - } - } - } -#endif delete this->CacheManager; delete this->Policies; if (this->GlobalGenerator) @@ -4500,3 +4485,22 @@ int cmake::Build(const std::string& dir, config.c_str(), clean, false, 0, true, 0, nativeOptions); } + +void cmake::RunCheckForUnusedVariables() const +{ +#ifdef CMAKE_BUILD_WITH_CMAKE + if(this->WarnUnusedCli) + { + std::map::const_iterator it; + for(it = this->UsedCliVariables.begin(); it != this->UsedCliVariables.end(); ++it) + { + if(!it->second) + { + std::string message = "warning: The variable, \"" + it->first + "\", given " + "on the command line, was not used within the build."; + cmSystemTools::Message(message.c_str()); + } + } + } +#endif +} diff --git a/Source/cmake.h b/Source/cmake.h index 403809fcd..4277bdd01 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -362,6 +362,8 @@ class cmake const std::string& config, const std::vector& nativeOptions, bool clean); + + void RunCheckForUnusedVariables() const; protected: void InitializeProperties(); int HandleDeleteCacheVariables(const char* var); From 300fc15779330a1b733c9e1a19d24d682a3b4a91 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 2 Sep 2010 11:30:28 -0400 Subject: [PATCH 17/87] Fix detection of system files Instead of looking to see if the file is under CMAKE_ROOT, check to see if it is instead under the source or binary directories in use. --- Source/cmCommandArgumentParserHelper.cxx | 6 ++++-- Source/cmMakefile.cxx | 5 +++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Source/cmCommandArgumentParserHelper.cxx b/Source/cmCommandArgumentParserHelper.cxx index e9381d4c6..c0a8127f6 100644 --- a/Source/cmCommandArgumentParserHelper.cxx +++ b/Source/cmCommandArgumentParserHelper.cxx @@ -130,8 +130,10 @@ char* cmCommandArgumentParserHelper::ExpandVariable(const char* var) // not been "cleared"/initialized with a set(foo ) call if(this->WarnUninitialized && !this->Makefile->VariableInitialized(var)) { - const char* root = this->Makefile->GetDefinition("CMAKE_ROOT"); - if (this->CheckSystemVars || strstr(this->FileName, root) != this->FileName) + const char* srcRoot = this->Makefile->GetDefinition("CMAKE_SOURCE_DIR"); + const char* binRoot = this->Makefile->GetDefinition("CMAKE_BINARY_DIR"); + if (this->CheckSystemVars || strstr(this->FileName, srcRoot) == this->FileName || + strstr(this->FileName, binRoot) == this->FileName) { cmOStringStream msg; msg << this->FileName << ":" << this->FileLine << ":" << diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 288bc531a..26b9a5867 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -3392,8 +3392,9 @@ void cmMakefile::PopScope() if (this->WarnUnused && usage.find(*it) == usage.end()) { const char* cdir = this->ListFileStack.back().c_str(); - const char* root = this->GetDefinition("CMAKE_ROOT"); - if (this->CheckSystemVars || strstr(cdir, root) != cdir) + const char* srcRoot = this->GetDefinition("CMAKE_SOURCE_DIR"); + const char* binRoot = this->GetDefinition("CMAKE_BINARY_DIR"); + if (this->CheckSystemVars || strstr(cdir, srcRoot) == cdir || strstr(cdir, binRoot) == cdir) { cmOStringStream m; m << "unused variable \'" << *it << "\'"; From 75bda3864ea033f976773df80f22fea2992a165d Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 2 Sep 2010 11:33:57 -0400 Subject: [PATCH 18/87] Add tests for unused command line variables --- Tests/CMakeLists.txt | 28 ++++++++++++++++++++++++++++ Tests/VariableUsage/CMakeLists.txt | 1 + 2 files changed, 29 insertions(+) create mode 100644 Tests/VariableUsage/CMakeLists.txt diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 5383bda97..64154728e 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -1074,6 +1074,34 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/BundleGeneratorTest") ENDIF(APPLE AND CTEST_TEST_CPACK) + ADD_TEST(WarnUnusedCliUnused ${CMAKE_CTEST_COMMAND} + --build-and-test + "${CMake_SOURCE_DIR}/Tests/VariableUsage" + "${CMake_BINARY_DIR}/Tests/WarnUnusedCliUnused" + --build-generator ${CMAKE_TEST_GENERATOR} + --build-makeprogram ${CMAKE_TEST_MAKEPROGRAM} + --build-noclean + --build-project WarnUnusedCliUnused + --build-options "-DUNUSED_VARIABLE=Unused") + SET_TESTS_PROPERTIES(WarnUnusedCliUnused PROPERTIES + PASS_REGULAR_EXPRESSION "warning: The variable, \"UNUSED_VARIABLE\"") + LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedCliUnused") + + ADD_TEST(WarnUnusedCliUsed ${CMAKE_CTEST_COMMAND} + --build-and-test + "${CMake_SOURCE_DIR}/Tests/VariableUsage" + "${CMake_BINARY_DIR}/Tests/WarnUnusedCliUsed" + --build-generator ${CMAKE_TEST_GENERATOR} + --build-makeprogram ${CMAKE_TEST_MAKEPROGRAM} + --build-noclean + --build-project WarnUnusedCliUsed + --build-options "-DUSED_VARIABLE=Usage proven") + SET_TESTS_PROPERTIES(WarnUnusedCliUsed PROPERTIES + PASS_REGULAR_EXPRESSION "Usage proven") + SET_TESTS_PROPERTIES(WarnUnusedCliUsed PROPERTIES + FAIL_REGULAR_EXPRESSION "warning: The variable, \"USED_VARIABLE\"") + LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedCliUsed") + # Make sure CTest can handle a test with no newline in output. ADD_TEST(CTest.NoNewline ${CMAKE_CMAKE_COMMAND} -E echo_append "This line has no newline!") diff --git a/Tests/VariableUsage/CMakeLists.txt b/Tests/VariableUsage/CMakeLists.txt new file mode 100644 index 000000000..4da1f56c7 --- /dev/null +++ b/Tests/VariableUsage/CMakeLists.txt @@ -0,0 +1 @@ +message(STATUS "${USED_VARIABLE}") From f047a17c5910cec1c0b9c20acb74510747ce6ba1 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 2 Sep 2010 11:34:13 -0400 Subject: [PATCH 19/87] Add test for uninitialized variables --- Tests/CMakeLists.txt | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 64154728e..481be09f9 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -1102,6 +1102,19 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ FAIL_REGULAR_EXPRESSION "warning: The variable, \"USED_VARIABLE\"") LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedCliUsed") + ADD_TEST(WarnUninitialized ${CMAKE_CTEST_COMMAND} + --build-and-test + "${CMake_SOURCE_DIR}/Tests/VariableUsage" + "${CMake_BINARY_DIR}/Tests/WarnUninitialized" + --build-generator ${CMAKE_TEST_GENERATOR} + --build-makeprogram ${CMAKE_TEST_MAKEPROGRAM} + --build-noclean + --build-project WarnUninitialized + --build-options "--warn-uninitialized") + SET_TESTS_PROPERTIES(WarnUninitialized PROPERTIES + PASS_REGULAR_EXPRESSION "warning: uninitialized variable 'USED_VARIABLE'") + LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUninitialized") + # Make sure CTest can handle a test with no newline in output. ADD_TEST(CTest.NoNewline ${CMAKE_CMAKE_COMMAND} -E echo_append "This line has no newline!") From b94812072a3e6dbcfc00255d46ea445769092d2f Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 2 Sep 2010 11:38:00 -0400 Subject: [PATCH 20/87] Change logic of flag to turn off cli unused checks Since we default to checking unused cli variables, make the flag turn off the checks. --- Source/cmake.cxx | 6 +++--- Source/cmakemain.cxx | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/cmake.cxx b/Source/cmake.cxx index b3261c59b..f4680ca97 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -641,10 +641,10 @@ void cmake::SetArgs(const std::vector& args) std::cout << "Finding unused variables.\n"; this->SetWarnUnused(true); } - else if(arg.find("--warn-unused-cli",0) == 0) + else if(arg.find("--no-warn-unused-cli",0) == 0) { - std::cout << "Finding unused variables given on the command line.\n"; - this->SetWarnUnusedCli(true); + std::cout << "Not finding unused variables given on the command line.\n"; + this->SetWarnUnusedCli(false); } else if(arg.find("--check-system-vars",0) == 0) { diff --git a/Source/cmakemain.cxx b/Source/cmakemain.cxx index cb3fcb04e..93b844fa1 100644 --- a/Source/cmakemain.cxx +++ b/Source/cmakemain.cxx @@ -124,8 +124,8 @@ static const char * cmDocumentationOptions[][3] = "Print a warning when an uninitialized variable is used."}, {"--warn-unused-all", "Warn about unused variables.", "Find variables that are declared or set, but not used."}, - {"--warn-unused-cli", "Warn about command line options.", - "Find variables that are declared on the command line, but not used."}, + {"--no-warn-unused-cli", "Don't warn about command line options.", + "Don't find variables that are declared on the command line, but not used."}, {"--help-command cmd [file]", "Print help for a single command and exit.", "Full documentation specific to the given command is displayed. " "If a file is specified, the documentation is written into and the output " From b74777fdb21e3632a4adfcd1e15844b632a2765d Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 2 Sep 2010 11:39:01 -0400 Subject: [PATCH 21/87] Fix the spelling of the flag for warn-unused-vars --- Source/cmakemain.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/cmakemain.cxx b/Source/cmakemain.cxx index 93b844fa1..d7ab951b7 100644 --- a/Source/cmakemain.cxx +++ b/Source/cmakemain.cxx @@ -122,7 +122,7 @@ static const char * cmDocumentationOptions[][3] = "message(send_error ) calls."}, {"--warn-uninitialized", "Warn about uninitialized values.", "Print a warning when an uninitialized variable is used."}, - {"--warn-unused-all", "Warn about unused variables.", + {"--warn-unused-vars", "Warn about unused variables.", "Find variables that are declared or set, but not used."}, {"--no-warn-unused-cli", "Don't warn about command line options.", "Don't find variables that are declared on the command line, but not used."}, From 4cf17062d36ecb87314afbfc0bed4545c8711eeb Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 2 Sep 2010 11:39:22 -0400 Subject: [PATCH 22/87] Add documentation for check-system-vars --- Source/cmakemain.cxx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Source/cmakemain.cxx b/Source/cmakemain.cxx index d7ab951b7..aa455a014 100644 --- a/Source/cmakemain.cxx +++ b/Source/cmakemain.cxx @@ -126,6 +126,9 @@ static const char * cmDocumentationOptions[][3] = "Find variables that are declared or set, but not used."}, {"--no-warn-unused-cli", "Don't warn about command line options.", "Don't find variables that are declared on the command line, but not used."}, + {"--check-system-vars", "Find problems with variable usage in system files.", + "Normally, unused and uninitialized variables are searched for only in CMAKE_SOURCE_DIR " + "and CMAKE_BINARY_DIR. This flag tells CMake to warn about other files as well."}, {"--help-command cmd [file]", "Print help for a single command and exit.", "Full documentation specific to the given command is displayed. " "If a file is specified, the documentation is written into and the output " From 439877f6208e25790ab14ae0e66efc392949e9e2 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 2 Sep 2010 12:14:06 -0400 Subject: [PATCH 23/87] Be consistent with single and double quotes --- Source/cmake.cxx | 2 +- Tests/CMakeLists.txt | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/cmake.cxx b/Source/cmake.cxx index f4680ca97..757ad6f71 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -4496,7 +4496,7 @@ void cmake::RunCheckForUnusedVariables() const { if(!it->second) { - std::string message = "warning: The variable, \"" + it->first + "\", given " + std::string message = "warning: The variable, '" + it->first + "', given " "on the command line, was not used within the build."; cmSystemTools::Message(message.c_str()); } diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 481be09f9..995a7495b 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -1082,9 +1082,9 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ --build-makeprogram ${CMAKE_TEST_MAKEPROGRAM} --build-noclean --build-project WarnUnusedCliUnused - --build-options "-DUNUSED_VARIABLE=Unused") + --build-options "-DUNUSED_CLI_VARIABLE=Unused") SET_TESTS_PROPERTIES(WarnUnusedCliUnused PROPERTIES - PASS_REGULAR_EXPRESSION "warning: The variable, \"UNUSED_VARIABLE\"") + PASS_REGULAR_EXPRESSION "warning: The variable, 'UNUSED_CLI_VARIABLE'") LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedCliUnused") ADD_TEST(WarnUnusedCliUsed ${CMAKE_CTEST_COMMAND} @@ -1099,7 +1099,7 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ SET_TESTS_PROPERTIES(WarnUnusedCliUsed PROPERTIES PASS_REGULAR_EXPRESSION "Usage proven") SET_TESTS_PROPERTIES(WarnUnusedCliUsed PROPERTIES - FAIL_REGULAR_EXPRESSION "warning: The variable, \"USED_VARIABLE\"") + FAIL_REGULAR_EXPRESSION "warning: The variable, 'USED_VARIABLE'") LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedCliUsed") ADD_TEST(WarnUninitialized ${CMAKE_CTEST_COMMAND} From 8b520158c3c378acde541d2e99103dc9ab834595 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 7 Sep 2010 15:09:33 -0400 Subject: [PATCH 24/87] Push the initialize and unused states when copying --- Source/cmMakefile.cxx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 26b9a5867..33c61a7c8 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -99,6 +99,8 @@ cmMakefile::cmMakefile(): Internal(new Internals) cmMakefile::cmMakefile(const cmMakefile& mf): Internal(new Internals) { this->Internal->VarStack.push(mf.Internal->VarStack.top().Closure()); + this->Internal->VarInitStack.push(mf.Internal->VarInitStack.top()); + this->Internal->VarUsageStack.push(mf.Internal->VarUsageStack.top()); this->Prefix = mf.Prefix; this->AuxSourceDirectories = mf.AuxSourceDirectories; From 3801463c9f99fdbf0b1a115f5a5f55d211c08de1 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 8 Sep 2010 12:03:42 -0400 Subject: [PATCH 25/87] Use built-ins for readability and maintainability --- Source/cmCommandArgumentParserHelper.cxx | 5 +++-- Source/cmMakefile.cxx | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Source/cmCommandArgumentParserHelper.cxx b/Source/cmCommandArgumentParserHelper.cxx index c0a8127f6..54af13bf9 100644 --- a/Source/cmCommandArgumentParserHelper.cxx +++ b/Source/cmCommandArgumentParserHelper.cxx @@ -132,8 +132,9 @@ char* cmCommandArgumentParserHelper::ExpandVariable(const char* var) { const char* srcRoot = this->Makefile->GetDefinition("CMAKE_SOURCE_DIR"); const char* binRoot = this->Makefile->GetDefinition("CMAKE_BINARY_DIR"); - if (this->CheckSystemVars || strstr(this->FileName, srcRoot) == this->FileName || - strstr(this->FileName, binRoot) == this->FileName) + if (this->CheckSystemVars || + cmSystemTools::IsSubDirectory(this->FileName, this->Makefile->GetHomeDirectory()) || + cmSystemTools::IsSubDirectory(this->FileName, this->Makefile->GetHomeOutputDirectory())) { cmOStringStream msg; msg << this->FileName << ":" << this->FileLine << ":" << diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 33c61a7c8..c15f5b36a 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -778,7 +778,7 @@ void cmMakefile::SetLocalGenerator(cmLocalGenerator* lg) this->Internal->VarUsageStack.push(std::set()); } } - this->CheckSystemVars = this->GetCMakeInstance()->GetCheckSystemVars(); + this->CheckSystemVars = this->GetCMakeInstance()->GetCheckSystemVars(); } bool cmMakefile::NeedBackwardsCompatibility(unsigned int major, @@ -3394,9 +3394,9 @@ void cmMakefile::PopScope() if (this->WarnUnused && usage.find(*it) == usage.end()) { const char* cdir = this->ListFileStack.back().c_str(); - const char* srcRoot = this->GetDefinition("CMAKE_SOURCE_DIR"); - const char* binRoot = this->GetDefinition("CMAKE_BINARY_DIR"); - if (this->CheckSystemVars || strstr(cdir, srcRoot) == cdir || strstr(cdir, binRoot) == cdir) + if (this->CheckSystemVars || + cmSystemTools::IsSubDirectory(cdir, this->GetHomeDirectory()) || + cmSystemTools::IsSubDirectory(cdir, this->GetHomeOutputDirectory())) { cmOStringStream m; m << "unused variable \'" << *it << "\'"; From 83acb0a4b22aa57d3c345320333b75301a99b2ba Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 8 Sep 2010 13:29:57 -0400 Subject: [PATCH 26/87] Remove now unused variables --- Source/cmCommandArgumentParserHelper.cxx | 2 -- 1 file changed, 2 deletions(-) diff --git a/Source/cmCommandArgumentParserHelper.cxx b/Source/cmCommandArgumentParserHelper.cxx index 54af13bf9..8b009e064 100644 --- a/Source/cmCommandArgumentParserHelper.cxx +++ b/Source/cmCommandArgumentParserHelper.cxx @@ -130,8 +130,6 @@ char* cmCommandArgumentParserHelper::ExpandVariable(const char* var) // not been "cleared"/initialized with a set(foo ) call if(this->WarnUninitialized && !this->Makefile->VariableInitialized(var)) { - const char* srcRoot = this->Makefile->GetDefinition("CMAKE_SOURCE_DIR"); - const char* binRoot = this->Makefile->GetDefinition("CMAKE_BINARY_DIR"); if (this->CheckSystemVars || cmSystemTools::IsSubDirectory(this->FileName, this->Makefile->GetHomeDirectory()) || cmSystemTools::IsSubDirectory(this->FileName, this->Makefile->GetHomeOutputDirectory())) From 980e048a7d5356a881dbaaf25b1595091fe5cb8b Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 14 Sep 2010 15:22:31 -0400 Subject: [PATCH 27/87] Factor out checks for unused variables --- Source/cmMakefile.cxx | 49 ++++++++++++++++++++++++++++--------------- Source/cmMakefile.h | 5 ++++- 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index c15f5b36a..c174bd665 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1649,6 +1649,11 @@ void cmMakefile::AddDefinition(const char* name, const char* value) #endif this->Internal->VarStack.top().Set(name, value); + if (this->Internal->VarUsageStack.size() > 1) + { + this->CheckForUnused("changing definition", name); + this->Internal->VarUsageStack.top().erase(name); + } #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); @@ -1714,6 +1719,11 @@ void cmMakefile::AddDefinition(const char* name, bool value) { this->Internal->VarStack.top().Set(name, value? "ON" : "OFF"); this->Internal->VarInitStack.top().insert(name); + if (this->Internal->VarUsageStack.size() > 1) + { + this->CheckForUnused("changing definition", name); + this->Internal->VarUsageStack.top().erase(name); + } #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); if ( vv ) @@ -1751,15 +1761,32 @@ bool cmMakefile::VariableCleared(const char* var) const return false; } +bool cmMakefile::CheckForUnused(const char* reason, const char* name) +{ + if (this->WarnUnused && !this->VariableUsed(name)) + { + const char* cdir = this->ListFileStack.back().c_str(); + if (this->CheckSystemVars || + cmSystemTools::IsSubDirectory(cdir, this->GetHomeDirectory()) || + cmSystemTools::IsSubDirectory(cdir, this->GetHomeOutputDirectory())) + { + cmOStringStream msg; + const cmListFileContext* file = this->CallStack.back().Context; + msg << file->FilePath << ":" << file->Line << ":" << + " warning: (" << reason << ") unused variable \'" << name << "\'"; + cmSystemTools::Message(msg.str().c_str()); + return true; + } + } + return false; +} + void cmMakefile::RemoveDefinition(const char* name) { this->Internal->VarStack.top().Set(name, 0); this->Internal->VarRemoved.insert(name); this->Internal->VarInitStack.top().insert(name); - if (this->WarnUnused) - { - this->Internal->VarUsageStack.top().insert(name); - } + this->CheckForUnused("unsetting", name); #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); if ( vv ) @@ -3391,19 +3418,7 @@ void cmMakefile::PopScope() for (; it != locals.end(); ++it) { init.erase(*it); - if (this->WarnUnused && usage.find(*it) == usage.end()) - { - const char* cdir = this->ListFileStack.back().c_str(); - if (this->CheckSystemVars || - cmSystemTools::IsSubDirectory(cdir, this->GetHomeDirectory()) || - cmSystemTools::IsSubDirectory(cdir, this->GetHomeOutputDirectory())) - { - cmOStringStream m; - m << "unused variable \'" << *it << "\'"; - this->IssueMessage(cmake::AUTHOR_WARNING, m.str()); - } - } - else + if (!this->CheckForUnused("out of scope", it->c_str())) { usage.erase(*it); } diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index f1ad54d3f..89979d6a3 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -840,7 +840,10 @@ public: protected: // add link libraries and directories to the target void AddGlobalLinkInformation(const char* name, cmTarget& target); - + + // Check for a an unused variable + bool CheckForUnused(const char* reason, const char* name); + std::string Prefix; std::vector AuxSourceDirectories; // From 056b44113f86a79ea9d68548005292a9d48d45f8 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 14 Sep 2010 17:10:22 -0400 Subject: [PATCH 28/87] Fix missing case for usage of a variable --- Source/cmMakefile.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index c174bd665..f6c8c7379 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1649,6 +1649,7 @@ void cmMakefile::AddDefinition(const char* name, const char* value) #endif this->Internal->VarStack.top().Set(name, value); + this->Internal->VarInitStack.top().insert(name); if (this->Internal->VarUsageStack.size() > 1) { this->CheckForUnused("changing definition", name); From ae3eff35b4814334f3a7e5f3240911aad0936c84 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 14 Sep 2010 17:14:25 -0400 Subject: [PATCH 29/87] Fix the path used for ignoring system warnings --- Source/cmMakefile.cxx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index f6c8c7379..4084d0e44 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1766,13 +1766,12 @@ bool cmMakefile::CheckForUnused(const char* reason, const char* name) { if (this->WarnUnused && !this->VariableUsed(name)) { - const char* cdir = this->ListFileStack.back().c_str(); + const cmListFileContext* file = this->CallStack.back().Context; if (this->CheckSystemVars || - cmSystemTools::IsSubDirectory(cdir, this->GetHomeDirectory()) || - cmSystemTools::IsSubDirectory(cdir, this->GetHomeOutputDirectory())) + cmSystemTools::IsSubDirectory(file->FilePath.c_str(), this->GetHomeDirectory()) || + cmSystemTools::IsSubDirectory(file->FilePath.c_str(), this->GetHomeOutputDirectory())) { cmOStringStream msg; - const cmListFileContext* file = this->CallStack.back().Context; msg << file->FilePath << ":" << file->Line << ":" << " warning: (" << reason << ") unused variable \'" << name << "\'"; cmSystemTools::Message(msg.str().c_str()); From a0b0d23f0cc3f596135ace5d675bbb09911b7f02 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 14 Sep 2010 17:51:43 -0400 Subject: [PATCH 30/87] CMAKE_DO_TRY_COMPILE is no longer used --- Source/cmCoreTryCompile.cxx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Source/cmCoreTryCompile.cxx b/Source/cmCoreTryCompile.cxx index b8a0c95b1..30356c82d 100644 --- a/Source/cmCoreTryCompile.cxx +++ b/Source/cmCoreTryCompile.cxx @@ -245,10 +245,8 @@ int cmCoreTryCompile::TryCompileCode(std::vector const& argv) CMAKE_TRY_COMPILE_OSX_ARCHITECTURE first to i386 and then to ppc to have the tests run for each specific architecture. Since cmLocalGenerator doesn't allow building for "the other" - architecture only via CMAKE_OSX_ARCHITECTURES,use to CMAKE_DO_TRY_COMPILE - to enforce it for this case here. + architecture only via CMAKE_OSX_ARCHITECTURES. */ - cmakeFlags.push_back("-DCMAKE_DO_TRY_COMPILE=TRUE"); if(this->Makefile->GetDefinition("CMAKE_TRY_COMPILE_OSX_ARCHITECTURES")!=0) { std::string flag="-DCMAKE_OSX_ARCHITECTURES="; From 02a114dfe8304e6fa90b9c6565349eb7e0fb1168 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 15 Sep 2010 11:34:35 -0400 Subject: [PATCH 31/87] Add method to allow variables to be marked as used --- Source/cmMakefile.cxx | 5 +++++ Source/cmMakefile.h | 2 ++ 2 files changed, 7 insertions(+) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 4084d0e44..85a0ccc90 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1735,6 +1735,11 @@ void cmMakefile::AddDefinition(const char* name, bool value) #endif } +void cmMakefile::MarkVariableAsUsed(const char* var) +{ + this->Internal->VarUsageStack.top().insert(var); +} + bool cmMakefile::VariableInitialized(const char* var) const { if(this->Internal->VarInitStack.top().find(var) != this->Internal->VarInitStack.top().end()) diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 89979d6a3..6b0bfa26a 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -61,6 +61,8 @@ public: unsigned int GetCacheMajorVersion(); unsigned int GetCacheMinorVersion(); + /* Mark a variable as used */ + void MarkVariableAsUsed(const char* var); /* return true if a variable has been initialized */ bool VariableInitialized(const char* ) const; /* return true if a variable has been used */ From a17aff74c7a571657414a0b82e431bbefbbb857b Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 15 Sep 2010 11:35:05 -0400 Subject: [PATCH 32/87] Ignore CMAKE_MATCH_* variables for usage --- Source/cmStringCommand.cxx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Source/cmStringCommand.cxx b/Source/cmStringCommand.cxx index 19f5c0f22..bab08fee1 100644 --- a/Source/cmStringCommand.cxx +++ b/Source/cmStringCommand.cxx @@ -482,6 +482,7 @@ void cmStringCommand::ClearMatches(cmMakefile* mf) char name[128]; sprintf(name, "CMAKE_MATCH_%d", i); mf->AddDefinition(name, ""); + mf->MarkVariableAsUsed(name); } } @@ -493,6 +494,7 @@ void cmStringCommand::StoreMatches(cmMakefile* mf,cmsys::RegularExpression& re) char name[128]; sprintf(name, "CMAKE_MATCH_%d", i); mf->AddDefinition(name, re.match(i).c_str()); + mf->MarkVariableAsUsed(name); } } From e01e40cb87f464dd5b1d5ca1c8f0884f573c1b72 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 15 Sep 2010 11:35:50 -0400 Subject: [PATCH 33/87] Mark ARGC, ARGV*, and ARGN as used --- Source/cmFunctionCommand.cxx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Source/cmFunctionCommand.cxx b/Source/cmFunctionCommand.cxx index b05642e3d..b2265746a 100644 --- a/Source/cmFunctionCommand.cxx +++ b/Source/cmFunctionCommand.cxx @@ -112,15 +112,19 @@ bool cmFunctionHelperCommand::InvokeInitialPass // set the value of argc cmOStringStream strStream; strStream << expandedArgs.size(); + this->Makefile->MarkVariableAsUsed("ARGC"); this->Makefile->AddDefinition("ARGC",strStream.str().c_str()); + this->Makefile->MarkVariableAsUsed("ARGC"); // set the values for ARGV0 ARGV1 ... for (unsigned int t = 0; t < expandedArgs.size(); ++t) { cmOStringStream tmpStream; tmpStream << "ARGV" << t; + this->Makefile->MarkVariableAsUsed(tmpStream.str().c_str()); this->Makefile->AddDefinition(tmpStream.str().c_str(), expandedArgs[t].c_str()); + this->Makefile->MarkVariableAsUsed(tmpStream.str().c_str()); } // define the formal arguments @@ -152,8 +156,12 @@ bool cmFunctionHelperCommand::InvokeInitialPass } cnt ++; } + this->Makefile->MarkVariableAsUsed("ARGV"); this->Makefile->AddDefinition("ARGV", argvDef.c_str()); + this->Makefile->MarkVariableAsUsed("ARGV"); + this->Makefile->MarkVariableAsUsed("ARGN"); this->Makefile->AddDefinition("ARGN", argnDef.c_str()); + this->Makefile->MarkVariableAsUsed("ARGN"); // Invoke all the functions that were collected in the block. // for each function From e49a935c20550482e9f92ceb05d1f2a0bc519c91 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 15 Sep 2010 11:41:27 -0400 Subject: [PATCH 34/87] Improve unused warning logic Only warn when changing the definition of an initialized variable. --- Source/cmMakefile.cxx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 85a0ccc90..9d5c59e65 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1649,12 +1649,12 @@ void cmMakefile::AddDefinition(const char* name, const char* value) #endif this->Internal->VarStack.top().Set(name, value); - this->Internal->VarInitStack.top().insert(name); - if (this->Internal->VarUsageStack.size() > 1) + if ((this->Internal->VarUsageStack.size() > 1) && this->VariableInitialized(name)) { this->CheckForUnused("changing definition", name); this->Internal->VarUsageStack.top().erase(name); } + this->Internal->VarInitStack.top().insert(name); #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); @@ -1719,12 +1719,12 @@ void cmMakefile::AddCacheDefinition(const char* name, const char* value, void cmMakefile::AddDefinition(const char* name, bool value) { this->Internal->VarStack.top().Set(name, value? "ON" : "OFF"); - this->Internal->VarInitStack.top().insert(name); - if (this->Internal->VarUsageStack.size() > 1) + if ((this->Internal->VarUsageStack.size() > 1) && this->VariableInitialized(name)) { this->CheckForUnused("changing definition", name); this->Internal->VarUsageStack.top().erase(name); } + this->Internal->VarInitStack.top().insert(name); #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); if ( vv ) From f117423336a91f4f50f031d0acc892d4c10316c3 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 16 Sep 2010 11:49:58 -0400 Subject: [PATCH 35/87] Fix line lengths to be no more than 78 --- Source/cmCommandArgumentParserHelper.cxx | 6 ++++-- Source/cmMakefile.cxx | 18 ++++++++++++------ Source/cmake.cxx | 6 ++++-- Source/cmakemain.cxx | 10 ++++++---- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/Source/cmCommandArgumentParserHelper.cxx b/Source/cmCommandArgumentParserHelper.cxx index 8b009e064..3f1ba530b 100644 --- a/Source/cmCommandArgumentParserHelper.cxx +++ b/Source/cmCommandArgumentParserHelper.cxx @@ -131,8 +131,10 @@ char* cmCommandArgumentParserHelper::ExpandVariable(const char* var) if(this->WarnUninitialized && !this->Makefile->VariableInitialized(var)) { if (this->CheckSystemVars || - cmSystemTools::IsSubDirectory(this->FileName, this->Makefile->GetHomeDirectory()) || - cmSystemTools::IsSubDirectory(this->FileName, this->Makefile->GetHomeOutputDirectory())) + cmSystemTools::IsSubDirectory(this->FileName, + this->Makefile->GetHomeDirectory()) || + cmSystemTools::IsSubDirectory(this->FileName, + this->Makefile->GetHomeOutputDirectory())) { cmOStringStream msg; msg << this->FileName << ":" << this->FileLine << ":" << diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 9d5c59e65..592e05e6d 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1649,7 +1649,8 @@ void cmMakefile::AddDefinition(const char* name, const char* value) #endif this->Internal->VarStack.top().Set(name, value); - if ((this->Internal->VarUsageStack.size() > 1) && this->VariableInitialized(name)) + if ((this->Internal->VarUsageStack.size() > 1) && + this->VariableInitialized(name)) { this->CheckForUnused("changing definition", name); this->Internal->VarUsageStack.top().erase(name); @@ -1719,7 +1720,8 @@ void cmMakefile::AddCacheDefinition(const char* name, const char* value, void cmMakefile::AddDefinition(const char* name, bool value) { this->Internal->VarStack.top().Set(name, value? "ON" : "OFF"); - if ((this->Internal->VarUsageStack.size() > 1) && this->VariableInitialized(name)) + if ((this->Internal->VarUsageStack.size() > 1) && + this->VariableInitialized(name)) { this->CheckForUnused("changing definition", name); this->Internal->VarUsageStack.top().erase(name); @@ -1742,7 +1744,8 @@ void cmMakefile::MarkVariableAsUsed(const char* var) bool cmMakefile::VariableInitialized(const char* var) const { - if(this->Internal->VarInitStack.top().find(var) != this->Internal->VarInitStack.top().end()) + if(this->Internal->VarInitStack.top().find(var) != + this->Internal->VarInitStack.top().end()) { return true; } @@ -1751,7 +1754,8 @@ bool cmMakefile::VariableInitialized(const char* var) const bool cmMakefile::VariableUsed(const char* var) const { - if(this->Internal->VarUsageStack.top().find(var) != this->Internal->VarUsageStack.top().end()) + if(this->Internal->VarUsageStack.top().find(var) != + this->Internal->VarUsageStack.top().end()) { return true; } @@ -1773,8 +1777,10 @@ bool cmMakefile::CheckForUnused(const char* reason, const char* name) { const cmListFileContext* file = this->CallStack.back().Context; if (this->CheckSystemVars || - cmSystemTools::IsSubDirectory(file->FilePath.c_str(), this->GetHomeDirectory()) || - cmSystemTools::IsSubDirectory(file->FilePath.c_str(), this->GetHomeOutputDirectory())) + cmSystemTools::IsSubDirectory(file->FilePath.c_str(), + this->GetHomeDirectory()) || + cmSystemTools::IsSubDirectory(file->FilePath.c_str(), + this->GetHomeOutputDirectory())) { cmOStringStream msg; msg << file->FilePath << ":" << file->Line << ":" << diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 757ad6f71..007f52dc7 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -643,12 +643,14 @@ void cmake::SetArgs(const std::vector& args) } else if(arg.find("--no-warn-unused-cli",0) == 0) { - std::cout << "Not finding unused variables given on the command line.\n"; + std::cout << "Not searching for unused variables given on the " << + "command line.\n"; this->SetWarnUnusedCli(false); } else if(arg.find("--check-system-vars",0) == 0) { - std::cout << "Also check system files when warning about unused and uninitialized variables.\n"; + std::cout << "Also check system files when warning about unused and " << + "uninitialized variables.\n"; this->SetCheckSystemVars(true); } else if(arg.find("-G",0) == 0) diff --git a/Source/cmakemain.cxx b/Source/cmakemain.cxx index aa455a014..a51673cb9 100644 --- a/Source/cmakemain.cxx +++ b/Source/cmakemain.cxx @@ -125,10 +125,12 @@ static const char * cmDocumentationOptions[][3] = {"--warn-unused-vars", "Warn about unused variables.", "Find variables that are declared or set, but not used."}, {"--no-warn-unused-cli", "Don't warn about command line options.", - "Don't find variables that are declared on the command line, but not used."}, - {"--check-system-vars", "Find problems with variable usage in system files.", - "Normally, unused and uninitialized variables are searched for only in CMAKE_SOURCE_DIR " - "and CMAKE_BINARY_DIR. This flag tells CMake to warn about other files as well."}, + "Don't find variables that are declared on the command line, but not " + "used."}, + {"--check-system-vars", "Find problems with variable usage in system " + "files.", "Normally, unused and uninitialized variables are searched for " + "only in CMAKE_SOURCE_DIR and CMAKE_BINARY_DIR. This flag tells CMake to " + "warn about other files as well."}, {"--help-command cmd [file]", "Print help for a single command and exit.", "Full documentation specific to the given command is displayed. " "If a file is specified, the documentation is written into and the output " From 59463ef1a33a22113e1c577c450d7b2c70916bbc Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 16 Sep 2010 11:50:57 -0400 Subject: [PATCH 36/87] Rework CheckVariableForUnused usage --- Source/cmMakefile.cxx | 10 ++++++---- Source/cmMakefile.h | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 592e05e6d..df871ec41 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1771,7 +1771,7 @@ bool cmMakefile::VariableCleared(const char* var) const return false; } -bool cmMakefile::CheckForUnused(const char* reason, const char* name) +void cmMakefile::CheckForUnused(const char* reason, const char* name) const { if (this->WarnUnused && !this->VariableUsed(name)) { @@ -1786,10 +1786,8 @@ bool cmMakefile::CheckForUnused(const char* reason, const char* name) msg << file->FilePath << ":" << file->Line << ":" << " warning: (" << reason << ") unused variable \'" << name << "\'"; cmSystemTools::Message(msg.str().c_str()); - return true; } } - return false; } void cmMakefile::RemoveDefinition(const char* name) @@ -3429,7 +3427,11 @@ void cmMakefile::PopScope() for (; it != locals.end(); ++it) { init.erase(*it); - if (!this->CheckForUnused("out of scope", it->c_str())) + if (!this->VariableUsed(it->c_str())) + { + this->CheckForUnused("out of scope", it->c_str()); + } + else { usage.erase(*it); } diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 6b0bfa26a..782690318 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -844,7 +844,7 @@ protected: void AddGlobalLinkInformation(const char* name, cmTarget& target); // Check for a an unused variable - bool CheckForUnused(const char* reason, const char* name); + void CheckForUnused(const char* reason, const char* name) const; std::string Prefix; std::vector AuxSourceDirectories; // From a8e97f8a08f143ebc255c05a50f20b783a0fffa9 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 16 Sep 2010 13:50:39 -0400 Subject: [PATCH 37/87] Remove VarRemoved code since it's been superceded --- Source/cmMakefile.cxx | 11 ----------- Source/cmMakefile.h | 4 ---- 2 files changed, 15 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index df871ec41..a7f86aead 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -45,7 +45,6 @@ public: std::stack > VarStack; std::stack > VarInitStack; std::stack > VarUsageStack; - std::set VarRemoved; }; // default is not to be building executables @@ -1762,15 +1761,6 @@ bool cmMakefile::VariableUsed(const char* var) const return false; } -bool cmMakefile::VariableCleared(const char* var) const -{ - if(this->Internal->VarRemoved.find(var) != this->Internal->VarRemoved.end()) - { - return true; - } - return false; -} - void cmMakefile::CheckForUnused(const char* reason, const char* name) const { if (this->WarnUnused && !this->VariableUsed(name)) @@ -1793,7 +1783,6 @@ void cmMakefile::CheckForUnused(const char* reason, const char* name) const void cmMakefile::RemoveDefinition(const char* name) { this->Internal->VarStack.top().Set(name, 0); - this->Internal->VarRemoved.insert(name); this->Internal->VarInitStack.top().insert(name); this->CheckForUnused("unsetting", name); #ifdef CMAKE_BUILD_WITH_CMAKE diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 782690318..9d9606d28 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -67,10 +67,6 @@ public: bool VariableInitialized(const char* ) const; /* return true if a variable has been used */ bool VariableUsed(const char* ) const; - /* return true if a variable has been set with - set(foo ) - */ - bool VariableCleared(const char* ) const; /** Return whether compatibility features needed for a version of the cache or lower should be enabled. */ bool NeedCacheCompatibility(int major, int minor); From aefc91dd3750430e085417ed7047ca3c64cdab56 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 16 Sep 2010 13:51:51 -0400 Subject: [PATCH 38/87] Add test for usage checks via unset --- Tests/CMakeLists.txt | 15 +++++++++++++++ Tests/VariableUnusedViaUnset/CMakeLists.txt | 8 ++++++++ 2 files changed, 23 insertions(+) create mode 100644 Tests/VariableUnusedViaUnset/CMakeLists.txt diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 995a7495b..be0c27ccf 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -1074,6 +1074,21 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/BundleGeneratorTest") ENDIF(APPLE AND CTEST_TEST_CPACK) + ADD_TEST(WarnUnusedUnusedViaUnset ${CMAKE_CTEST_COMMAND} + --build-and-test + "${CMake_SOURCE_DIR}/Tests/VariableUnusedViaUnset" + "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaUnset" + --build-generator ${CMAKE_TEST_GENERATOR} + --build-makeprogram ${CMAKE_TEST_MAKEPROGRAM} + --build-noclean + --build-project WarnUnusedUnusedViaUnset + --build-options "--warn-unused-vars") + SET_TESTS_PROPERTIES(WarnUnusedUnusedViaUnset PROPERTIES + PASS_REGULAR_EXPRESSION ":7: warning: \\(unsetting\\) unused variable 'UNUSED_VARIABLE'") + SET_TESTS_PROPERTIES(WarnUnusedUnusedViaUnset PROPERTIES + FAIL_REGULAR_EXPRESSION ":5: warning: \\(unsetting\\) unused variable 'UNUSED_VARIABLE'") + LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaUnset") + ADD_TEST(WarnUnusedCliUnused ${CMAKE_CTEST_COMMAND} --build-and-test "${CMake_SOURCE_DIR}/Tests/VariableUsage" diff --git a/Tests/VariableUnusedViaUnset/CMakeLists.txt b/Tests/VariableUnusedViaUnset/CMakeLists.txt new file mode 100644 index 000000000..886508ee2 --- /dev/null +++ b/Tests/VariableUnusedViaUnset/CMakeLists.txt @@ -0,0 +1,8 @@ +# 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 ne issued for this line: +set(UNUSED_VARIABLE) +# Warning should occur here: +set(UNUSED_VARIABLE) +message(STATUS "${UNUSED_VARIABLE}") From 995cfb0e2a4d2787371f87568bc943e6ad34d55c Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 16 Sep 2010 13:52:17 -0400 Subject: [PATCH 39/87] Don't warn if the variable wasn't defined --- Source/cmMakefile.cxx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index a7f86aead..1e12658ec 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1783,8 +1783,13 @@ void cmMakefile::CheckForUnused(const char* reason, const char* name) const void cmMakefile::RemoveDefinition(const char* name) { this->Internal->VarStack.top().Set(name, 0); + if (this->Internal->VarUsageStack.size() && + this->VariableInitialized(name)) + { + this->CheckForUnused("unsetting", name); + this->Internal->VarUsageStack.top().erase(name); + } this->Internal->VarInitStack.top().insert(name); - this->CheckForUnused("unsetting", name); #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); if ( vv ) From f7438ca7ac62007aec39d0e9134d556607ccbff3 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 16 Sep 2010 13:52:54 -0400 Subject: [PATCH 40/87] Add test for unused detection via setting it --- Tests/CMakeLists.txt | 15 +++++++++++++++ Tests/VariableUnusedViaSet/CMakeLists.txt | 4 ++++ 2 files changed, 19 insertions(+) create mode 100644 Tests/VariableUnusedViaSet/CMakeLists.txt diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index be0c27ccf..739dcdf45 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -1074,6 +1074,21 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/BundleGeneratorTest") ENDIF(APPLE AND CTEST_TEST_CPACK) + ADD_TEST(WarnUnusedUnusedViaSet ${CMAKE_CTEST_COMMAND} + --build-and-test + "${CMake_SOURCE_DIR}/Tests/VariableUnusedViaSet" + "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaSet" + --build-generator ${CMAKE_TEST_GENERATOR} + --build-makeprogram ${CMAKE_TEST_MAKEPROGRAM} + --build-noclean + --build-project WarnUnusedUnusedViaSet + --build-options "--warn-unused-vars") + SET_TESTS_PROPERTIES(WarnUnusedUnusedViaSet PROPERTIES + PASS_REGULAR_EXPRESSION "warning: \\(changing definition\\) unused variable 'UNUSED_VARIABLE'") + SET_TESTS_PROPERTIES(WarnUnusedUnusedViaSet PROPERTIES + FAIL_REGULAR_EXPRESSION "warning: \\(unsetting\\) unused variable '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" diff --git a/Tests/VariableUnusedViaSet/CMakeLists.txt b/Tests/VariableUnusedViaSet/CMakeLists.txt new file mode 100644 index 000000000..0123ab221 --- /dev/null +++ b/Tests/VariableUnusedViaSet/CMakeLists.txt @@ -0,0 +1,4 @@ +set(UNUSED_VARIABLE) +# Warning should occur here +set(UNUSED_VARIABLE "Usage") +message(STATUS "${UNUSED_VARIABLE}") From ca90f673a0f162424bd6b1ab74e72439bad9c429 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 16 Sep 2010 13:53:41 -0400 Subject: [PATCH 41/87] Fix detection of unused variables when setting --- Source/cmMakefile.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 1e12658ec..844b3022f 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1648,7 +1648,7 @@ void cmMakefile::AddDefinition(const char* name, const char* value) #endif this->Internal->VarStack.top().Set(name, value); - if ((this->Internal->VarUsageStack.size() > 1) && + if (this->Internal->VarUsageStack.size() && this->VariableInitialized(name)) { this->CheckForUnused("changing definition", name); @@ -1719,7 +1719,7 @@ void cmMakefile::AddCacheDefinition(const char* name, const char* value, void cmMakefile::AddDefinition(const char* name, bool value) { this->Internal->VarStack.top().Set(name, value? "ON" : "OFF"); - if ((this->Internal->VarUsageStack.size() > 1) && + if (this->Internal->VarUsageStack.size() && this->VariableInitialized(name)) { this->CheckForUnused("changing definition", name); @@ -1763,7 +1763,7 @@ bool cmMakefile::VariableUsed(const char* var) const void cmMakefile::CheckForUnused(const char* reason, const char* name) const { - if (this->WarnUnused && !this->VariableUsed(name)) + if (this->WarnUnused && !this->VariableUsed(name) && this->CallStack.size()) { const cmListFileContext* file = this->CallStack.back().Context; if (this->CheckSystemVars || From 91c4c9921c40361df860c1384a46ed77c6118c23 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 16 Sep 2010 13:54:16 -0400 Subject: [PATCH 42/87] Add test for unused warnings at the end of scope --- Tests/CMakeLists.txt | 15 +++++++++++++++ Tests/VariableUnusedViaScope/CMakeLists.txt | 2 ++ 2 files changed, 17 insertions(+) create mode 100644 Tests/VariableUnusedViaScope/CMakeLists.txt diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 739dcdf45..adc4f907f 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -1104,6 +1104,21 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ FAIL_REGULAR_EXPRESSION ":5: warning: \\(unsetting\\) unused variable 'UNUSED_VARIABLE'") LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaUnset") + ADD_TEST(WarnUnusedUnusedViaScope ${CMAKE_CTEST_COMMAND} + --build-and-test + "${CMake_SOURCE_DIR}/Tests/VariableUnusedViaScope" + "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaScope" + --build-generator ${CMAKE_TEST_GENERATOR} + --build-makeprogram ${CMAKE_TEST_MAKEPROGRAM} + --build-noclean + --build-project WarnUnusedUnusedViaScope + --build-options "--warn-unused-vars") + SET_TESTS_PROPERTIES(WarnUnusedUnusedViaScope PROPERTIES + PASS_REGULAR_EXPRESSION "warning: \\(out of scope\\) unused variable 'UNUSED_VARIABLE'") + SET_TESTS_PROPERTIES(WarnUnusedUnusedViaScope PROPERTIES + FAIL_REGULAR_EXPRESSION "warning: \\(unsetting\\) unused variable 'UNUSED_VARIABLE'") + LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaScope") + ADD_TEST(WarnUnusedCliUnused ${CMAKE_CTEST_COMMAND} --build-and-test "${CMake_SOURCE_DIR}/Tests/VariableUsage" diff --git a/Tests/VariableUnusedViaScope/CMakeLists.txt b/Tests/VariableUnusedViaScope/CMakeLists.txt new file mode 100644 index 000000000..a024bc5a7 --- /dev/null +++ b/Tests/VariableUnusedViaScope/CMakeLists.txt @@ -0,0 +1,2 @@ +set(UNUSED_VARIABLE) +# Warning should occur here From 05cb0f4daf6941482784eb06c549c5c63a9d895b Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 16 Sep 2010 16:05:59 -0400 Subject: [PATCH 43/87] Check for unused variables in the dtor --- Source/cmMakefile.cxx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 844b3022f..774612a95 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -178,6 +178,15 @@ bool cmMakefile::NeedCacheCompatibility(int major, int minor) cmMakefile::~cmMakefile() { + std::set usage = this->Internal->VarUsageStack.top(); + std::set::const_iterator it = usage.begin(); + for (; it != usage.end(); ++it) + { + if (!this->VariableUsed(it->c_str())) + { + this->CheckForUnused("out of scope", it->c_str()); + } + } for(std::vector::iterator i = this->InstallGenerators.begin(); i != this->InstallGenerators.end(); ++i) From bef3aeebab1a07573cde8886a7e32054bec1e850 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 16 Sep 2010 16:06:16 -0400 Subject: [PATCH 44/87] Use the API so that warnings can be tracked --- Source/cmMakefile.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 774612a95..38d8b6d89 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -3477,7 +3477,7 @@ void cmMakefile::RaiseScope(const char *var, const char *varDef) // directory's scope was initialized by the closure of the parent // scope, so we do not need to localize the definition first. cmMakefile* parent = plg->GetMakefile(); - parent->Internal->VarStack.top().Set(var, varDef); + parent->AddDefinition(var, varDef); } else { From 7740a738e0578e67ec2c408e2244703bdc22c62a Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 16 Sep 2010 16:07:34 -0400 Subject: [PATCH 45/87] Only return local keys that are defined --- Source/cmDefinitions.cxx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 34ca68d4a..9d2870058 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -92,7 +92,10 @@ std::set cmDefinitions::LocalKeys() const for(MapType::const_iterator mi = this->Map.begin(); mi != this->Map.end(); ++mi) { - keys.insert(mi->first); + if (mi->second.Exists) + { + keys.insert(mi->first); + } } return keys; } From 6d7d449cb1c5d4438034936f3fb8f0e8a6800116 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 16 Sep 2010 17:49:37 -0400 Subject: [PATCH 46/87] Ignore CLI warnings for ABI determination --- Modules/CMakeDetermineCompilerABI.cmake | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Modules/CMakeDetermineCompilerABI.cmake b/Modules/CMakeDetermineCompilerABI.cmake index aa21c3127..81ec71ccd 100644 --- a/Modules/CMakeDetermineCompilerABI.cmake +++ b/Modules/CMakeDetermineCompilerABI.cmake @@ -32,6 +32,10 @@ FUNCTION(CMAKE_DETERMINE_COMPILER_ABI lang src) ${CMAKE_BINARY_DIR} ${src} CMAKE_FLAGS "${CMAKE_FLAGS}" "-DCMAKE_${lang}_STANDARD_LIBRARIES=" + # We need ignore these warnings because some platforms need + # CMAKE_${lang}_STANDARD_LIBRARIES to link properly and we + # don't care when we are just determining the ABI. + "--no-warn-unused-cli" OUTPUT_VARIABLE OUTPUT COPY_FILE "${BIN}" ) From 2c82f2b75979208d9db6b514b0962f4e6b9aa05e Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 16 Sep 2010 17:50:07 -0400 Subject: [PATCH 47/87] Exempt CMAKE(CURRENT|PARENT)_LIST_FILE from usage --- Source/cmMakefile.cxx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 38d8b6d89..2908fa34e 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -590,6 +590,7 @@ bool cmMakefile::ReadListFile(const char* filename_in, std::string currentFile = this->GetSafeDefinition("CMAKE_CURRENT_LIST_FILE"); this->AddDefinition("CMAKE_PARENT_LIST_FILE", filename_in); + this->MarkVariableAsUsed("CMAKE_PARENT_LIST_FILE"); const char* external = 0; std::string external_abs; @@ -630,6 +631,7 @@ bool cmMakefile::ReadListFile(const char* filename_in, } this->AddDefinition("CMAKE_CURRENT_LIST_FILE", filenametoread); + this->MarkVariableAsUsed("CMAKE_CURRENT_LIST_FILE"); // try to see if the list file is the top most // list file for a project, and if it is, then it @@ -662,7 +664,9 @@ bool cmMakefile::ReadListFile(const char* filename_in, *fullPath = ""; } this->AddDefinition("CMAKE_PARENT_LIST_FILE", currentParentFile.c_str()); + this->MarkVariableAsUsed("CMAKE_PARENT_LIST_FILE"); this->AddDefinition("CMAKE_CURRENT_LIST_FILE", currentFile.c_str()); + this->MarkVariableAsUsed("CMAKE_CURRENT_LIST_FILE"); return false; } // add this list file to the list of dependencies @@ -702,7 +706,9 @@ bool cmMakefile::ReadListFile(const char* filename_in, } this->AddDefinition("CMAKE_PARENT_LIST_FILE", currentParentFile.c_str()); + this->MarkVariableAsUsed("CMAKE_PARENT_LIST_FILE"); this->AddDefinition("CMAKE_CURRENT_LIST_FILE", currentFile.c_str()); + this->MarkVariableAsUsed("CMAKE_CURRENT_LIST_FILE"); // pop the listfile off the stack this->ListFileStack.pop_back(); From a117e02cc8e204ed5992e110c62942d80390bfe7 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Fri, 17 Sep 2010 09:47:17 -0400 Subject: [PATCH 48/87] Revert "Add test for unused warnings at the end of scope" This reverts commit 91c4c9921c40361df860c1384a46ed77c6118c23. The test doesn't work yet and should not have been put on this branch yet. --- Tests/CMakeLists.txt | 15 --------------- Tests/VariableUnusedViaScope/CMakeLists.txt | 2 -- 2 files changed, 17 deletions(-) delete mode 100644 Tests/VariableUnusedViaScope/CMakeLists.txt diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index adc4f907f..739dcdf45 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -1104,21 +1104,6 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ FAIL_REGULAR_EXPRESSION ":5: warning: \\(unsetting\\) unused variable 'UNUSED_VARIABLE'") LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaUnset") - ADD_TEST(WarnUnusedUnusedViaScope ${CMAKE_CTEST_COMMAND} - --build-and-test - "${CMake_SOURCE_DIR}/Tests/VariableUnusedViaScope" - "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaScope" - --build-generator ${CMAKE_TEST_GENERATOR} - --build-makeprogram ${CMAKE_TEST_MAKEPROGRAM} - --build-noclean - --build-project WarnUnusedUnusedViaScope - --build-options "--warn-unused-vars") - SET_TESTS_PROPERTIES(WarnUnusedUnusedViaScope PROPERTIES - PASS_REGULAR_EXPRESSION "warning: \\(out of scope\\) unused variable 'UNUSED_VARIABLE'") - SET_TESTS_PROPERTIES(WarnUnusedUnusedViaScope PROPERTIES - FAIL_REGULAR_EXPRESSION "warning: \\(unsetting\\) unused variable 'UNUSED_VARIABLE'") - LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaScope") - ADD_TEST(WarnUnusedCliUnused ${CMAKE_CTEST_COMMAND} --build-and-test "${CMake_SOURCE_DIR}/Tests/VariableUsage" diff --git a/Tests/VariableUnusedViaScope/CMakeLists.txt b/Tests/VariableUnusedViaScope/CMakeLists.txt deleted file mode 100644 index a024bc5a7..000000000 --- a/Tests/VariableUnusedViaScope/CMakeLists.txt +++ /dev/null @@ -1,2 +0,0 @@ -set(UNUSED_VARIABLE) -# Warning should occur here From f231ce5ce3968d963f81dd3a5093ff2b13f17062 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Fri, 17 Sep 2010 10:02:15 -0400 Subject: [PATCH 49/87] Remove old false positive avoidance code From email explaining existence in the first place: This is from before when the used checks throwing false positives about unused due to changing the definition without checking whether it *had* a value to begin with and me not realizing they were false positives. I was thinking that it was warning from ARGC et. al. not being used since the previous macro or function call and the new value warning about overwriting the old value. --- Source/cmFunctionCommand.cxx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Source/cmFunctionCommand.cxx b/Source/cmFunctionCommand.cxx index b2265746a..ec4fd16bc 100644 --- a/Source/cmFunctionCommand.cxx +++ b/Source/cmFunctionCommand.cxx @@ -112,7 +112,6 @@ bool cmFunctionHelperCommand::InvokeInitialPass // set the value of argc cmOStringStream strStream; strStream << expandedArgs.size(); - this->Makefile->MarkVariableAsUsed("ARGC"); this->Makefile->AddDefinition("ARGC",strStream.str().c_str()); this->Makefile->MarkVariableAsUsed("ARGC"); @@ -121,7 +120,6 @@ bool cmFunctionHelperCommand::InvokeInitialPass { cmOStringStream tmpStream; tmpStream << "ARGV" << t; - this->Makefile->MarkVariableAsUsed(tmpStream.str().c_str()); this->Makefile->AddDefinition(tmpStream.str().c_str(), expandedArgs[t].c_str()); this->Makefile->MarkVariableAsUsed(tmpStream.str().c_str()); @@ -156,10 +154,8 @@ bool cmFunctionHelperCommand::InvokeInitialPass } cnt ++; } - this->Makefile->MarkVariableAsUsed("ARGV"); this->Makefile->AddDefinition("ARGV", argvDef.c_str()); this->Makefile->MarkVariableAsUsed("ARGV"); - this->Makefile->MarkVariableAsUsed("ARGN"); this->Makefile->AddDefinition("ARGN", argnDef.c_str()); this->Makefile->MarkVariableAsUsed("ARGN"); From dee19760a701408411d6b20cc623ccb53ce54732 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Fri, 17 Sep 2010 10:04:18 -0400 Subject: [PATCH 50/87] Fix typo in VariableUnusedViaUnset test --- Tests/VariableUnusedViaUnset/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/VariableUnusedViaUnset/CMakeLists.txt b/Tests/VariableUnusedViaUnset/CMakeLists.txt index 886508ee2..4b4031da3 100644 --- a/Tests/VariableUnusedViaUnset/CMakeLists.txt +++ b/Tests/VariableUnusedViaUnset/CMakeLists.txt @@ -1,7 +1,7 @@ # 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 ne issued for this line: +# A warning should NOT be issued for this line: set(UNUSED_VARIABLE) # Warning should occur here: set(UNUSED_VARIABLE) From 5e41ba8e4a25bec2b5d7c175f80a1ace0999a8ff Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Fri, 17 Sep 2010 10:13:19 -0400 Subject: [PATCH 51/87] When using the API, check for Add vs. Remove --- Source/cmMakefile.cxx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 2908fa34e..a9faa6b89 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -3483,7 +3483,14 @@ void cmMakefile::RaiseScope(const char *var, const char *varDef) // directory's scope was initialized by the closure of the parent // scope, so we do not need to localize the definition first. cmMakefile* parent = plg->GetMakefile(); - parent->AddDefinition(var, varDef); + if (varDef) + { + parent->AddDefinition(var, varDef); + } + else + { + parent->RemoveDefinition(var); + } } else { From c6e7fabc0f3563a258b8b3e10757e1916d680db0 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 22 Sep 2010 12:40:43 -0400 Subject: [PATCH 52/87] Factor out the checks for unused variables --- Source/cmMakefile.cxx | 26 +++++++++++++++++--------- Source/cmMakefile.h | 2 ++ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index a9faa6b89..5aa643c5c 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -178,15 +178,9 @@ bool cmMakefile::NeedCacheCompatibility(int major, int minor) cmMakefile::~cmMakefile() { - std::set usage = this->Internal->VarUsageStack.top(); - std::set::const_iterator it = usage.begin(); - for (; it != usage.end(); ++it) - { - if (!this->VariableUsed(it->c_str())) - { - this->CheckForUnused("out of scope", it->c_str()); - } - } + // Check for unused variables + this->CheckForUnusedVariables(); + for(std::vector::iterator i = this->InstallGenerators.begin(); i != this->InstallGenerators.end(); ++i) @@ -713,6 +707,9 @@ bool cmMakefile::ReadListFile(const char* filename_in, // pop the listfile off the stack this->ListFileStack.pop_back(); + // Check for unused variables + this->CheckForUnusedVariables(); + return true; } @@ -1751,6 +1748,17 @@ void cmMakefile::AddDefinition(const char* name, bool value) #endif } +void cmMakefile::CheckForUnusedVariables() const +{ + const cmDefinitions& defs = this->Internal->VarStack.top(); + const std::set& locals = defs.LocalKeys(); + std::set::const_iterator it = locals.begin(); + for (; it != locals.end(); ++it) + { + this->CheckForUnused("out of scope", it->c_str()); + } +} + void cmMakefile::MarkVariableAsUsed(const char* var) { this->Internal->VarUsageStack.top().insert(var); diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 9d9606d28..240a9cfb3 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -61,6 +61,8 @@ public: unsigned int GetCacheMajorVersion(); unsigned int GetCacheMinorVersion(); + /* Check for unused variables in this scope */ + void CheckForUnusedVariables() const; /* Mark a variable as used */ void MarkVariableAsUsed(const char* var); /* return true if a variable has been initialized */ From 62be1f78aea0f38e245e7f8b748a24f95eadb37a Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 22 Sep 2010 12:41:29 -0400 Subject: [PATCH 53/87] Initialize the usage stack earlier --- Source/cmMakefile.cxx | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 5aa643c5c..70d349502 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -54,6 +54,7 @@ cmMakefile::cmMakefile(): Internal(new Internals) const std::set globalKeys = defs.LocalKeys(); this->Internal->VarStack.push(defs); this->Internal->VarInitStack.push(globalKeys); + this->Internal->VarUsageStack.push(globalKeys); // Setup the default include file regular expression (match everything). this->IncludeFileRegularExpression = "^.*$"; @@ -775,20 +776,7 @@ void cmMakefile::SetLocalGenerator(cmLocalGenerator* lg) this->AddSourceGroup("Resources", "\\.plist$"); #endif - if (this->Internal->VarUsageStack.empty()) - { - const cmDefinitions& defs = cmDefinitions(); - const std::set globalKeys = defs.LocalKeys(); - this->WarnUnused = this->GetCMakeInstance()->GetWarnUnused(); - if (this->WarnUnused) - { - this->Internal->VarUsageStack.push(globalKeys); - } - else - { - this->Internal->VarUsageStack.push(std::set()); - } - } + this->WarnUnused = this->GetCMakeInstance()->GetWarnUnused(); this->CheckSystemVars = this->GetCMakeInstance()->GetCheckSystemVars(); } From cbb286c0b25e806d817ceb540312f34a7e1aa9a5 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 22 Sep 2010 12:41:58 -0400 Subject: [PATCH 54/87] Fix the path detection to work for top-level --- Source/cmMakefile.cxx | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 70d349502..57c354f74 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1774,17 +1774,30 @@ bool cmMakefile::VariableUsed(const char* var) const void cmMakefile::CheckForUnused(const char* reason, const char* name) const { - if (this->WarnUnused && !this->VariableUsed(name) && this->CallStack.size()) + if (this->WarnUnused && !this->VariableUsed(name)) { - const cmListFileContext* file = this->CallStack.back().Context; + cmStdString path; + int line; + if (this->CallStack.size()) + { + const cmListFileContext* file = this->CallStack.back().Context; + path = file->FilePath.c_str(); + line = file->Line; + } + else + { + path = this->GetStartDirectory(); + path += "/CMakeLists.txt"; + line = 0; + } if (this->CheckSystemVars || - cmSystemTools::IsSubDirectory(file->FilePath.c_str(), + cmSystemTools::IsSubDirectory(path.c_str(), this->GetHomeDirectory()) || - cmSystemTools::IsSubDirectory(file->FilePath.c_str(), + cmSystemTools::IsSubDirectory(path.c_str(), this->GetHomeOutputDirectory())) { cmOStringStream msg; - msg << file->FilePath << ":" << file->Line << ":" << + msg << path << ":" << line << ":" << " warning: (" << reason << ") unused variable \'" << name << "\'"; cmSystemTools::Message(msg.str().c_str()); } From 535253f38598d6dd46aca944a82cfe1684b2f07f Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 22 Sep 2010 12:42:26 -0400 Subject: [PATCH 55/87] Initialize the warning variables earlier --- Source/cmMakefile.cxx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 57c354f74..86aca5a41 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -56,6 +56,10 @@ cmMakefile::cmMakefile(): Internal(new Internals) this->Internal->VarInitStack.push(globalKeys); this->Internal->VarUsageStack.push(globalKeys); + // Initialize these first since AddDefaultDefinitions calls AddDefinition + this->WarnUnused = false; + this->CheckSystemVars = false; + // Setup the default include file regular expression (match everything). this->IncludeFileRegularExpression = "^.*$"; // Setup the default include complaint regular expression (match nothing). @@ -92,8 +96,6 @@ cmMakefile::cmMakefile(): Internal(new Internals) this->AddDefaultDefinitions(); this->Initialize(); this->PreOrder = false; - this->WarnUnused = false; - this->CheckSystemVars = false; } cmMakefile::cmMakefile(const cmMakefile& mf): Internal(new Internals) From 33c63b19ab4f192f95c72735d810ef3921370d1f Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 22 Sep 2010 12:42:49 -0400 Subject: [PATCH 56/87] Add a method to put a watch for variables --- Source/cmake.cxx | 13 +++++++++---- Source/cmake.h | 1 + 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 007f52dc7..cb68f2212 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -380,10 +380,7 @@ bool cmake::SetCacheArgs(const std::vector& args) "No help, variable specified on the command line.", type); if(this->WarnUnusedCli) { -#ifdef CMAKE_BUILD_WITH_CMAKE - this->VariableWatch->AddWatch(var, cmWarnUnusedCliWarning, this); - this->UsedCliVariables[var] = false; -#endif + this->WatchUnusedCli(var.c_str()); } } else @@ -4488,6 +4485,14 @@ int cmake::Build(const std::string& dir, 0, nativeOptions); } +void cmake::WatchUnusedCli(const char* var) +{ +#ifdef CMAKE_BUILD_WITH_CMAKE + this->VariableWatch->AddWatch(var, cmWarnUnusedCliWarning, this); + this->UsedCliVariables[var] = false; +#endif +} + void cmake::RunCheckForUnusedVariables() const { #ifdef CMAKE_BUILD_WITH_CMAKE diff --git a/Source/cmake.h b/Source/cmake.h index 4277bdd01..72effd38d 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -363,6 +363,7 @@ class cmake const std::vector& nativeOptions, bool clean); + void WatchUnusedCli(const char* var); void RunCheckForUnusedVariables() const; protected: void InitializeProperties(); From 5d30cfc5f70e9bf7f604a64ca47d4c5cd213d11f Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 22 Sep 2010 12:43:15 -0400 Subject: [PATCH 57/87] Set a watch on variables added through the gui --- Source/QtDialog/QCMake.cxx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Source/QtDialog/QCMake.cxx b/Source/QtDialog/QCMake.cxx index c319cb43d..48871c204 100644 --- a/Source/QtDialog/QCMake.cxx +++ b/Source/QtDialog/QCMake.cxx @@ -249,6 +249,8 @@ void QCMake::setProperties(const QCMakePropertyList& newProps) // add some new properites foreach(QCMakeProperty s, props) { + this->CMakeInstance->WatchUnusedCli(s.Key.toAscii().data()); + if(s.Type == QCMakeProperty::BOOL) { this->CMakeInstance->AddCacheEntry(s.Key.toAscii().data(), From fe56002a167df5d5c28de9e5a5c0b4bb1660fccf Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 23 Sep 2010 10:14:37 -0400 Subject: [PATCH 58/87] Fix long lines for KWStyle --- Source/cmCommandArgumentParserHelper.cxx | 2 +- Source/cmake.cxx | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Source/cmCommandArgumentParserHelper.cxx b/Source/cmCommandArgumentParserHelper.cxx index 3f1ba530b..23101b86c 100644 --- a/Source/cmCommandArgumentParserHelper.cxx +++ b/Source/cmCommandArgumentParserHelper.cxx @@ -134,7 +134,7 @@ char* cmCommandArgumentParserHelper::ExpandVariable(const char* var) cmSystemTools::IsSubDirectory(this->FileName, this->Makefile->GetHomeDirectory()) || cmSystemTools::IsSubDirectory(this->FileName, - this->Makefile->GetHomeOutputDirectory())) + this->Makefile->GetHomeOutputDirectory())) { cmOStringStream msg; msg << this->FileName << ":" << this->FileLine << ":" << diff --git a/Source/cmake.cxx b/Source/cmake.cxx index cb68f2212..07abc8ea8 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -4499,12 +4499,13 @@ void cmake::RunCheckForUnusedVariables() const if(this->WarnUnusedCli) { std::map::const_iterator it; - for(it = this->UsedCliVariables.begin(); it != this->UsedCliVariables.end(); ++it) + for(it = this->UsedCliVariables.begin(); + it != this->UsedCliVariables.end(); ++it) { if(!it->second) { - std::string message = "warning: The variable, '" + it->first + "', given " - "on the command line, was not used within the build."; + std::string message = "warning: The variable, '" + it->first + + "', given on the command line, was not used within the build."; cmSystemTools::Message(message.c_str()); } } From 9b90040edba1f89395295c27cd9919b0d5efd30b Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Fri, 1 Oct 2010 13:44:55 -0400 Subject: [PATCH 59/87] When calling CMake, set the args and the cache --- Source/cmMakefile.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 86aca5a41..99b6bc311 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -2810,6 +2810,7 @@ int cmMakefile::TryCompile(const char *srcdir, const char *bindir, // if cmake args were provided then pass them in if (cmakeArgs) { + cm.SetArgs(*cmakeArgs); cm.SetCacheArgs(*cmakeArgs); } // to save time we pass the EnableLanguage info directly From ab5d4e43d9a9ddde24a242092a0d5e5f9a6cbd01 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Fri, 1 Oct 2010 16:52:16 -0400 Subject: [PATCH 60/87] Revert "When calling CMake, set the args and the cache" This reverts commit 9b90040edba1f89395295c27cd9919b0d5efd30b. --- Source/cmMakefile.cxx | 1 - 1 file changed, 1 deletion(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 99b6bc311..86aca5a41 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -2810,7 +2810,6 @@ int cmMakefile::TryCompile(const char *srcdir, const char *bindir, // if cmake args were provided then pass them in if (cmakeArgs) { - cm.SetArgs(*cmakeArgs); cm.SetCacheArgs(*cmakeArgs); } // to save time we pass the EnableLanguage info directly From 367e5c37bbaba8f2169ed5a56e553e9327b8951a Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 5 Oct 2010 12:27:37 -0400 Subject: [PATCH 61/87] Revert "Revert "When calling CMake, set the args and the cache"" This reverts commit ab5d4e43d9a9ddde24a242092a0d5e5f9a6cbd01. --- Source/cmMakefile.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 86aca5a41..99b6bc311 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -2810,6 +2810,7 @@ int cmMakefile::TryCompile(const char *srcdir, const char *bindir, // if cmake args were provided then pass them in if (cmakeArgs) { + cm.SetArgs(*cmakeArgs); cm.SetCacheArgs(*cmakeArgs); } // to save time we pass the EnableLanguage info directly From 5aa535bdcb9d1148690e55b601e1c8f8763f842c Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 5 Oct 2010 13:44:28 -0400 Subject: [PATCH 62/87] Add argument to arg parsing to not set directories Argument parsing sets the source/build directories, but they may have been (meaningfully) set before hand. Let's not overwrite them. --- Source/cmake.cxx | 4 ++-- Source/cmake.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 07abc8ea8..845ab9be4 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -524,9 +524,9 @@ void cmake::ReadListFile(const char *path) } // Parse the args -void cmake::SetArgs(const std::vector& args) +void cmake::SetArgs(const std::vector& args, bool directoriesSetBefore) { - bool directoriesSet = false; + bool directoriesSet = directoriesSetBefore; for(unsigned int i=1; i < args.size(); ++i) { std::string arg = args[i]; diff --git a/Source/cmake.h b/Source/cmake.h index 72effd38d..36c0f4a9f 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -212,7 +212,7 @@ class cmake bool CommandExists(const char* name) const; ///! Parse command line arguments - void SetArgs(const std::vector&); + void SetArgs(const std::vector&, bool directoriesSetBefore = false); ///! Is this cmake running as a result of a TRY_COMPILE command bool GetIsInTryCompile() { return this->InTryCompile; } From 82ed104dcb7d380e23fecfb4e7911f5eab093436 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 5 Oct 2010 13:45:34 -0400 Subject: [PATCH 63/87] Flag that the directories have been set --- Source/cmMakefile.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 99b6bc311..a4a79befb 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -2810,7 +2810,7 @@ int cmMakefile::TryCompile(const char *srcdir, const char *bindir, // if cmake args were provided then pass them in if (cmakeArgs) { - cm.SetArgs(*cmakeArgs); + cm.SetArgs(*cmakeArgs, true); cm.SetCacheArgs(*cmakeArgs); } // to save time we pass the EnableLanguage info directly From a267b99cd64a17781bf7af3fd6b53a4acb9c10ff Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 6 Oct 2010 15:00:52 -0400 Subject: [PATCH 64/87] Fix line lengths --- Source/cmake.cxx | 3 ++- Source/cmake.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 845ab9be4..37373aea3 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -524,7 +524,8 @@ void cmake::ReadListFile(const char *path) } // Parse the args -void cmake::SetArgs(const std::vector& args, bool directoriesSetBefore) +void cmake::SetArgs(const std::vector& args, + bool directoriesSetBefore) { bool directoriesSet = directoriesSetBefore; for(unsigned int i=1; i < args.size(); ++i) diff --git a/Source/cmake.h b/Source/cmake.h index 36c0f4a9f..9001748f3 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -212,7 +212,8 @@ class cmake bool CommandExists(const char* name) const; ///! Parse command line arguments - void SetArgs(const std::vector&, bool directoriesSetBefore = false); + void SetArgs(const std::vector&, + bool directoriesSetBefore = false); ///! Is this cmake running as a result of a TRY_COMPILE command bool GetIsInTryCompile() { return this->InTryCompile; } From d4ee998b61cf3c16e9e04da2173490589790a894 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 7 Oct 2010 12:22:00 -0400 Subject: [PATCH 65/87] Hard-code the --no-warn-unused-cli flag --- Source/cmMakefile.cxx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index a4a79befb..ed3456962 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -2810,7 +2810,11 @@ int cmMakefile::TryCompile(const char *srcdir, const char *bindir, // if cmake args were provided then pass them in if (cmakeArgs) { - cm.SetArgs(*cmakeArgs, true); + // FIXME: Workaround to ignore unused CLI variables until the + // 'ArgumentExpansion' test succeeds with CMAKE_STRICT on + cm.SetWarnUnusedCli(true); + //cm.SetArgs(*cmakeArgs, true); + cm.SetCacheArgs(*cmakeArgs); } // to save time we pass the EnableLanguage info directly From 8dbb2090a2fac15f61710afef9887e224c1c7f72 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 7 Oct 2010 12:28:25 -0400 Subject: [PATCH 66/87] Wrong boolean value for CLI warnings --- Source/cmMakefile.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index ed3456962..15c5370db 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -2812,7 +2812,7 @@ int cmMakefile::TryCompile(const char *srcdir, const char *bindir, { // FIXME: Workaround to ignore unused CLI variables until the // 'ArgumentExpansion' test succeeds with CMAKE_STRICT on - cm.SetWarnUnusedCli(true); + cm.SetWarnUnusedCli(false); //cm.SetArgs(*cmakeArgs, true); cm.SetCacheArgs(*cmakeArgs); From fe390a2607afcd8f0985a54990236a3ea16643ab Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 7 Oct 2010 12:29:43 -0400 Subject: [PATCH 67/87] Add 'ArgumentExpansion' test --- Tests/ArgumentExpansion/CMakeLists.txt | 59 ++++++++++++++++++++++++++ Tests/CMakeLists.txt | 15 +++++++ 2 files changed, 74 insertions(+) create mode 100644 Tests/ArgumentExpansion/CMakeLists.txt diff --git a/Tests/ArgumentExpansion/CMakeLists.txt b/Tests/ArgumentExpansion/CMakeLists.txt new file mode 100644 index 000000000..bd781691b --- /dev/null +++ b/Tests/ArgumentExpansion/CMakeLists.txt @@ -0,0 +1,59 @@ +cmake_minimum_required(VERSION 2.8) + +project(ArgumentExpansion) + +function (argument_tester expected expected_len) + list(LENGTH ARGN argn_len) + list(LENGTH ${expected} expected_received_len) + + if (NOT ${expected_received_len} EQUAL ${expected_len}) + message(STATUS "Error: Expanding expected values isn't working") + endif (NOT ${expected_received_len} EQUAL ${expected_len}) + + if (${argn_len} EQUAL ${expected_len}) + set(i 0) + while (i LESS ${argn_len}) + list(GET ARGN ${i} argn_value) + list(GET ${expected} ${i} expected_value) + + if (NOT ${argn_value} STREQUAL ${expected_value}) + message(STATUS "Error: Argument ${i} doesn't match") + message(STATUS " Expected: ${expected_value}") + message(STATUS " Received: ${argn_value}") + endif (NOT ${argn_value} STREQUAL ${expected_value}) + + math(EXPR i "${i} + 1") + endwhile (i LESS ${argn_len}) + else (${argn_len} EQUAL ${expected_len}) + message(STATUS "Error: Lengths of arguments don't match") + message(STATUS " Expected: ${expected_len}") + message(STATUS " Received: ${argn_len}") + endif (${argn_len} EQUAL ${expected_len}) +endfunction (argument_tester expected) + +set(empty_test) +message(STATUS "Test: Empty arguments") +argument_tester(empty_test 0 ${empty_test}) + +set(single_arg_test + "single arg") +message(STATUS "Test: Single argument") +argument_tester(single_arg_test 1 ${single_arg_test}) + +set(multiple_arg_test + "first arg" + "second arg") +message(STATUS "Test: Multiple arguments") +argument_tester(multiple_arg_test 2 ${multiple_arg_test}) + +set(nested_list_arg_test + "${multiple_arg_test}" + "first arg" + "second arg") +message(STATUS "Test: Nested list argument") +argument_tester(nested_list_arg_test 3 ${nested_list_arg_test}) + +set(semicolon_arg_test + "pre\;post") +message(STATUS "Test: Semicolon argument") +argument_tester(semicolon_arg_test 1 ${semicolon_arg_test}) diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 739dcdf45..a7253547e 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -368,6 +368,21 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ ) LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/CustComDepend") + ADD_TEST(ArgumentExpansion ${CMAKE_CTEST_COMMAND} + --build-and-test + "${CMake_SOURCE_DIR}/Tests/ArgumentExpansion" + "${CMake_BINARY_DIR}/Tests/ArgumentExpansion" + --build-generator ${CMAKE_TEST_GENERATOR} + --build-project ArgumentExpansion + --build-makeprogram ${CMAKE_TEST_MAKEPROGRAM} + --build-exe-dir "${CMake_BINARY_DIR}/Tests/ArgumentExpansion/bin" + ) + IF(CMAKE_STRICT) + SET_TESTS_PROPERTIES(ArgumentExpansion PROPERTIES + FAIL_REGULAR_EXPRESSION "Error: ") + ENDIF(CMAKE_STRICT) + LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/ArgumentExpansion") + ADD_TEST(CustomCommand ${CMAKE_CTEST_COMMAND} --build-and-test "${CMake_SOURCE_DIR}/Tests/CustomCommand" From 2507f937bd3f287e1905ef25ce2c37e616037671 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Fri, 8 Oct 2010 09:46:38 -0400 Subject: [PATCH 68/87] Change the failure case string to 'Unexpected' VS6 detects the 'Error' string and fails itself even though we don't actually care about it unless CMAKE_STRICT is on. --- Tests/ArgumentExpansion/CMakeLists.txt | 6 +++--- Tests/CMakeLists.txt | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Tests/ArgumentExpansion/CMakeLists.txt b/Tests/ArgumentExpansion/CMakeLists.txt index bd781691b..62017067a 100644 --- a/Tests/ArgumentExpansion/CMakeLists.txt +++ b/Tests/ArgumentExpansion/CMakeLists.txt @@ -7,7 +7,7 @@ function (argument_tester expected expected_len) list(LENGTH ${expected} expected_received_len) if (NOT ${expected_received_len} EQUAL ${expected_len}) - message(STATUS "Error: Expanding expected values isn't working") + message(STATUS "Unexpected: Expanding expected values isn't working") endif (NOT ${expected_received_len} EQUAL ${expected_len}) if (${argn_len} EQUAL ${expected_len}) @@ -17,7 +17,7 @@ function (argument_tester expected expected_len) list(GET ${expected} ${i} expected_value) if (NOT ${argn_value} STREQUAL ${expected_value}) - message(STATUS "Error: Argument ${i} doesn't match") + message(STATUS "Unexpected: Argument ${i} doesn't match") message(STATUS " Expected: ${expected_value}") message(STATUS " Received: ${argn_value}") endif (NOT ${argn_value} STREQUAL ${expected_value}) @@ -25,7 +25,7 @@ function (argument_tester expected expected_len) math(EXPR i "${i} + 1") endwhile (i LESS ${argn_len}) else (${argn_len} EQUAL ${expected_len}) - message(STATUS "Error: Lengths of arguments don't match") + message(STATUS "Unexpected: Lengths of arguments don't match") message(STATUS " Expected: ${expected_len}") message(STATUS " Received: ${argn_len}") endif (${argn_len} EQUAL ${expected_len}) diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index a7253547e..ac363279f 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -379,7 +379,7 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ ) IF(CMAKE_STRICT) SET_TESTS_PROPERTIES(ArgumentExpansion PROPERTIES - FAIL_REGULAR_EXPRESSION "Error: ") + FAIL_REGULAR_EXPRESSION "Unexpected: ") ENDIF(CMAKE_STRICT) LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/ArgumentExpansion") From 3f1121f72260789dd61ce947e208aa182341859e Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Fri, 8 Oct 2010 13:49:39 -0400 Subject: [PATCH 69/87] Use a long int since Line is a long as well --- Source/cmMakefile.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 15c5370db..45599d7ae 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1779,7 +1779,7 @@ void cmMakefile::CheckForUnused(const char* reason, const char* name) const if (this->WarnUnused && !this->VariableUsed(name)) { cmStdString path; - int line; + long line; if (this->CallStack.size()) { const cmListFileContext* file = this->CallStack.back().Context; From c18c977ce81d730a870958b591f61a65a16c2758 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Mon, 15 Nov 2010 10:32:15 -0500 Subject: [PATCH 70/87] When checking for variables, specify a reason Allow reasons to begiven for checking for unused variables. --- Source/cmMakefile.cxx | 2 +- Source/cmake.cxx | 5 +++-- Source/cmake.h | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 45599d7ae..9ecc988bc 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -832,7 +832,7 @@ void cmMakefile::ConfigureFinalPass() l->second.FinishConfigure(); } - this->GetCMakeInstance()->RunCheckForUnusedVariables(); + this->GetCMakeInstance()->RunCheckForUnusedVariables("configure"); } //---------------------------------------------------------------------------- diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 37373aea3..5d910d0f7 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -4494,7 +4494,7 @@ void cmake::WatchUnusedCli(const char* var) #endif } -void cmake::RunCheckForUnusedVariables() const +void cmake::RunCheckForUnusedVariables(const std::string& reason) const { #ifdef CMAKE_BUILD_WITH_CMAKE if(this->WarnUnusedCli) @@ -4506,7 +4506,8 @@ void cmake::RunCheckForUnusedVariables() const if(!it->second) { std::string message = "warning: The variable, '" + it->first + - "', given on the command line, was not used within the build."; + "', given on the command line, was not used during the " + reason + + "."; cmSystemTools::Message(message.c_str()); } } diff --git a/Source/cmake.h b/Source/cmake.h index 9001748f3..b132164cc 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -365,7 +365,7 @@ class cmake bool clean); void WatchUnusedCli(const char* var); - void RunCheckForUnusedVariables() const; + void RunCheckForUnusedVariables(const std::string& reason) const; protected: void InitializeProperties(); int HandleDeleteCacheVariables(const char* var); From b97ee21fc65487f4b55ac8e5054aeddb33d5127e Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Mon, 15 Nov 2010 10:33:14 -0500 Subject: [PATCH 71/87] Check for unused variables at the end of generate --- Source/cmGlobalGenerator.cxx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx index bd26b5fca..201fceb07 100644 --- a/Source/cmGlobalGenerator.cxx +++ b/Source/cmGlobalGenerator.cxx @@ -905,6 +905,8 @@ void cmGlobalGenerator::Generate() } this->CMakeInstance->UpdateProgress("Generating done", -1); + + this->CMakeInstance->RunCheckForUnusedVariables("generation"); } //---------------------------------------------------------------------------- From 447a04c31c7540882fb823df0890450e677083ee Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Mon, 15 Nov 2010 11:01:19 -0500 Subject: [PATCH 72/87] Don't warn during configure when doing everything This prevents warnings from being generated after configure *and* after generation if both are going to be run anyways. --- Source/cmake.cxx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 5d910d0f7..d9217ff9c 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -2292,6 +2292,11 @@ int cmake::Run(const std::vector& args, bool noconfigure) std::string oldstartoutputdir = this->GetStartOutputDirectory(); this->SetStartDirectory(this->GetHomeDirectory()); this->SetStartOutputDirectory(this->GetHomeOutputDirectory()); + const bool warncli = this->WarnUnusedCli; + if (!this->ScriptMode) + { + this->WarnUnusedCli = false; + } int ret = this->Configure(); if (ret || this->ScriptMode) { @@ -2313,6 +2318,7 @@ int cmake::Run(const std::vector& args, bool noconfigure) #endif return ret; } + this->WarnUnusedCli = warncli; ret = this->Generate(); std::string message = "Build files have been written to: "; message += this->GetHomeOutputDirectory(); From fd50f06b215f81f2ee28ae8afab32d0b3049fac5 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 2 Dec 2010 16:57:30 -0500 Subject: [PATCH 73/87] Don't check for unused vars at configure time The generate step should catch all of them. --- Source/cmMakefile.cxx | 2 -- 1 file changed, 2 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 9ecc988bc..82b012999 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -831,8 +831,6 @@ void cmMakefile::ConfigureFinalPass() { l->second.FinishConfigure(); } - - this->GetCMakeInstance()->RunCheckForUnusedVariables("configure"); } //---------------------------------------------------------------------------- From cf8b15a5c1eb32f756cf76f06fd18c23241dd103 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Fri, 3 Dec 2010 12:49:37 -0500 Subject: [PATCH 74/87] Ignore files under the CMakeFiles directory --- Source/cmMakefile.cxx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 82b012999..feeee2fdb 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1793,8 +1793,10 @@ void cmMakefile::CheckForUnused(const char* reason, const char* name) const if (this->CheckSystemVars || cmSystemTools::IsSubDirectory(path.c_str(), this->GetHomeDirectory()) || - cmSystemTools::IsSubDirectory(path.c_str(), - this->GetHomeOutputDirectory())) + (cmSystemTools::IsSubDirectory(path.c_str(), + this->GetHomeOutputDirectory()) && + !cmSystemTools::IsSubDirectory(path.c_str(), + cmake::GetCMakeFilesDirectory()))) { cmOStringStream msg; msg << path << ":" << line << ":" << From 3c3b98ddd384214d191c1b061f8c7eba6845810d Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Fri, 3 Dec 2010 12:52:36 -0500 Subject: [PATCH 75/87] Initialize the class before setting warn flags Since Initialize sets variables that we don't want to warn about, don't leak the original class' settings. --- Source/cmMakefile.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index feeee2fdb..ed435dac9 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -141,9 +141,9 @@ cmMakefile::cmMakefile(const cmMakefile& mf): Internal(new Internals) this->Properties = mf.Properties; this->PreOrder = mf.PreOrder; this->WarnUnused = mf.WarnUnused; + this->Initialize(); this->CheckSystemVars = mf.CheckSystemVars; this->ListFileStack = mf.ListFileStack; - this->Initialize(); } //---------------------------------------------------------------------------- From 88cd4c1e923817f0a38bbc9d14d0bf40a2151897 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 7 Dec 2010 14:17:50 -0500 Subject: [PATCH 76/87] Use 'CMake Warning' versus 'warning' for CDash --- Source/cmCommandArgumentParserHelper.cxx | 2 +- Source/cmMakefile.cxx | 2 +- Source/cmake.cxx | 2 +- Tests/CMakeLists.txt | 14 +++++++------- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Source/cmCommandArgumentParserHelper.cxx b/Source/cmCommandArgumentParserHelper.cxx index 23101b86c..decdaaaf8 100644 --- a/Source/cmCommandArgumentParserHelper.cxx +++ b/Source/cmCommandArgumentParserHelper.cxx @@ -138,7 +138,7 @@ char* cmCommandArgumentParserHelper::ExpandVariable(const char* var) { cmOStringStream msg; msg << this->FileName << ":" << this->FileLine << ":" << - " warning: uninitialized variable \'" << var << "\'"; + " CMake Warning: uninitialized variable \'" << var << "\'"; cmSystemTools::Message(msg.str().c_str()); } } diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index ed435dac9..016d5fea9 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1800,7 +1800,7 @@ void cmMakefile::CheckForUnused(const char* reason, const char* name) const { cmOStringStream msg; msg << path << ":" << line << ":" << - " warning: (" << reason << ") unused variable \'" << name << "\'"; + " CMake Warning: (" << reason << ") unused variable \'" << name << "\'"; cmSystemTools::Message(msg.str().c_str()); } } diff --git a/Source/cmake.cxx b/Source/cmake.cxx index d9217ff9c..8c12ca910 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -4511,7 +4511,7 @@ void cmake::RunCheckForUnusedVariables(const std::string& reason) const { if(!it->second) { - std::string message = "warning: The variable, '" + it->first + + std::string message = "CMake Warning: The variable, '" + it->first + "', given on the command line, was not used during the " + reason + "."; cmSystemTools::Message(message.c_str()); diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index ac363279f..4cfbf9392 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -1099,9 +1099,9 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ --build-project WarnUnusedUnusedViaSet --build-options "--warn-unused-vars") SET_TESTS_PROPERTIES(WarnUnusedUnusedViaSet PROPERTIES - PASS_REGULAR_EXPRESSION "warning: \\(changing definition\\) unused variable 'UNUSED_VARIABLE'") + PASS_REGULAR_EXPRESSION "CMake Warning: \\(changing definition\\) unused variable 'UNUSED_VARIABLE'") SET_TESTS_PROPERTIES(WarnUnusedUnusedViaSet PROPERTIES - FAIL_REGULAR_EXPRESSION "warning: \\(unsetting\\) unused variable 'UNUSED_VARIABLE'") + FAIL_REGULAR_EXPRESSION "CMake Warning: \\(unsetting\\) unused variable 'UNUSED_VARIABLE'") LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaSet") ADD_TEST(WarnUnusedUnusedViaUnset ${CMAKE_CTEST_COMMAND} @@ -1114,9 +1114,9 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ --build-project WarnUnusedUnusedViaUnset --build-options "--warn-unused-vars") SET_TESTS_PROPERTIES(WarnUnusedUnusedViaUnset PROPERTIES - PASS_REGULAR_EXPRESSION ":7: warning: \\(unsetting\\) unused variable 'UNUSED_VARIABLE'") + PASS_REGULAR_EXPRESSION ":7: CMake Warning: \\(unsetting\\) unused variable 'UNUSED_VARIABLE'") SET_TESTS_PROPERTIES(WarnUnusedUnusedViaUnset PROPERTIES - FAIL_REGULAR_EXPRESSION ":5: warning: \\(unsetting\\) unused variable 'UNUSED_VARIABLE'") + FAIL_REGULAR_EXPRESSION ":5: CMake Warning: \\(unsetting\\) unused variable 'UNUSED_VARIABLE'") LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaUnset") ADD_TEST(WarnUnusedCliUnused ${CMAKE_CTEST_COMMAND} @@ -1129,7 +1129,7 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ --build-project WarnUnusedCliUnused --build-options "-DUNUSED_CLI_VARIABLE=Unused") SET_TESTS_PROPERTIES(WarnUnusedCliUnused PROPERTIES - PASS_REGULAR_EXPRESSION "warning: The variable, 'UNUSED_CLI_VARIABLE'") + PASS_REGULAR_EXPRESSION "CMake Warning: The variable, 'UNUSED_CLI_VARIABLE'") LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedCliUnused") ADD_TEST(WarnUnusedCliUsed ${CMAKE_CTEST_COMMAND} @@ -1144,7 +1144,7 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ SET_TESTS_PROPERTIES(WarnUnusedCliUsed PROPERTIES PASS_REGULAR_EXPRESSION "Usage proven") SET_TESTS_PROPERTIES(WarnUnusedCliUsed PROPERTIES - FAIL_REGULAR_EXPRESSION "warning: The variable, 'USED_VARIABLE'") + FAIL_REGULAR_EXPRESSION "CMake Warning: The variable, 'USED_VARIABLE'") LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedCliUsed") ADD_TEST(WarnUninitialized ${CMAKE_CTEST_COMMAND} @@ -1157,7 +1157,7 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ --build-project WarnUninitialized --build-options "--warn-uninitialized") SET_TESTS_PROPERTIES(WarnUninitialized PROPERTIES - PASS_REGULAR_EXPRESSION "warning: uninitialized variable 'USED_VARIABLE'") + PASS_REGULAR_EXPRESSION "CMake Warning: uninitialized variable 'USED_VARIABLE'") LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUninitialized") # Make sure CTest can handle a test with no newline in output. From 668e005db5d9cece95965e23b8c589bd50a29faa Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 7 Dec 2010 16:38:25 -0500 Subject: [PATCH 77/87] Use cmake::IssueMessage for warnings --- Source/cmCommandArgumentParserHelper.cxx | 11 ++++++++--- Source/cmMakefile.cxx | 16 ++++++++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/Source/cmCommandArgumentParserHelper.cxx b/Source/cmCommandArgumentParserHelper.cxx index decdaaaf8..a781767e3 100644 --- a/Source/cmCommandArgumentParserHelper.cxx +++ b/Source/cmCommandArgumentParserHelper.cxx @@ -137,9 +137,14 @@ char* cmCommandArgumentParserHelper::ExpandVariable(const char* var) this->Makefile->GetHomeOutputDirectory())) { cmOStringStream msg; - msg << this->FileName << ":" << this->FileLine << ":" << - " CMake Warning: uninitialized variable \'" << var << "\'"; - cmSystemTools::Message(msg.str().c_str()); + cmListFileBacktrace bt; + cmListFileContext lfc; + lfc.FilePath = this->FileName; + lfc.Line = this->FileLine; + bt.push_back(lfc); + msg << "uninitialized variable \'" << var << "\'"; + this->Makefile->GetCMakeInstance()->IssueMessage(cmake::AUTHOR_WARNING, + msg.str().c_str(), bt); } } return 0; diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 016d5fea9..e22ade37a 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1777,18 +1777,21 @@ void cmMakefile::CheckForUnused(const char* reason, const char* name) const if (this->WarnUnused && !this->VariableUsed(name)) { cmStdString path; - long line; + cmListFileBacktrace bt; if (this->CallStack.size()) { const cmListFileContext* file = this->CallStack.back().Context; + bt.push_back(*file); path = file->FilePath.c_str(); - line = file->Line; } else { path = this->GetStartDirectory(); path += "/CMakeLists.txt"; - line = 0; + cmListFileContext lfc; + lfc.FilePath = path; + lfc.Line = 0; + bt.push_back(lfc); } if (this->CheckSystemVars || cmSystemTools::IsSubDirectory(path.c_str(), @@ -1799,9 +1802,10 @@ void cmMakefile::CheckForUnused(const char* reason, const char* name) const cmake::GetCMakeFilesDirectory()))) { cmOStringStream msg; - msg << path << ":" << line << ":" << - " CMake Warning: (" << reason << ") unused variable \'" << name << "\'"; - cmSystemTools::Message(msg.str().c_str()); + msg << "unused variable (" << reason << ") \'" << name << "\'"; + this->GetCMakeInstance()->IssueMessage(cmake::AUTHOR_WARNING, + msg.str().c_str(), + bt); } } } From 8e8c9e49243674579545d5f27101597604c96f12 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 7 Dec 2010 16:38:37 -0500 Subject: [PATCH 78/87] Don't check at destruction for usage --- Source/cmMakefile.cxx | 3 --- 1 file changed, 3 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index e22ade37a..9ccde8f59 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -181,9 +181,6 @@ bool cmMakefile::NeedCacheCompatibility(int major, int minor) cmMakefile::~cmMakefile() { - // Check for unused variables - this->CheckForUnusedVariables(); - for(std::vector::iterator i = this->InstallGenerators.begin(); i != this->InstallGenerators.end(); ++i) From 4e3bea41ee939a039b53d5963f459c354bcb2b42 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 7 Dec 2010 16:46:10 -0500 Subject: [PATCH 79/87] Update expected messages to new format --- Tests/CMakeLists.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 4cfbf9392..b60a6109d 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -1099,9 +1099,9 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ --build-project WarnUnusedUnusedViaSet --build-options "--warn-unused-vars") SET_TESTS_PROPERTIES(WarnUnusedUnusedViaSet PROPERTIES - PASS_REGULAR_EXPRESSION "CMake Warning: \\(changing definition\\) unused variable 'UNUSED_VARIABLE'") + PASS_REGULAR_EXPRESSION "unused variable \\(changing definition\\) 'UNUSED_VARIABLE'") SET_TESTS_PROPERTIES(WarnUnusedUnusedViaSet PROPERTIES - FAIL_REGULAR_EXPRESSION "CMake Warning: \\(unsetting\\) unused variable 'UNUSED_VARIABLE'") + FAIL_REGULAR_EXPRESSION "unused variable \\(unsetting\\) 'UNUSED_VARIABLE'") LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaSet") ADD_TEST(WarnUnusedUnusedViaUnset ${CMAKE_CTEST_COMMAND} @@ -1114,9 +1114,9 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ --build-project WarnUnusedUnusedViaUnset --build-options "--warn-unused-vars") SET_TESTS_PROPERTIES(WarnUnusedUnusedViaUnset PROPERTIES - PASS_REGULAR_EXPRESSION ":7: CMake Warning: \\(unsetting\\) unused variable 'UNUSED_VARIABLE'") + PASS_REGULAR_EXPRESSION "CMake Warning .*:7 \\(set\\):") SET_TESTS_PROPERTIES(WarnUnusedUnusedViaUnset PROPERTIES - FAIL_REGULAR_EXPRESSION ":5: CMake Warning: \\(unsetting\\) unused variable 'UNUSED_VARIABLE'") + FAIL_REGULAR_EXPRESSION "CMake Warning .*:5 \\(set\\):") LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUnusedUnusedViaUnset") ADD_TEST(WarnUnusedCliUnused ${CMAKE_CTEST_COMMAND} From 544d0c37742a068fa07b265380315a25af3ae9f3 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 7 Dec 2010 17:00:41 -0500 Subject: [PATCH 80/87] Fix expected output for WarnUninitialized test --- Tests/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index b60a6109d..c421e690e 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -1157,7 +1157,7 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ --build-project WarnUninitialized --build-options "--warn-uninitialized") SET_TESTS_PROPERTIES(WarnUninitialized PROPERTIES - PASS_REGULAR_EXPRESSION "CMake Warning: uninitialized variable 'USED_VARIABLE'") + PASS_REGULAR_EXPRESSION "uninitialized variable 'USED_VARIABLE'") LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/WarnUninitialized") # Make sure CTest can handle a test with no newline in output. From 5625dee390fc0c35755683b7302e49967107bbf0 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Mon, 3 Jan 2011 08:45:48 -0500 Subject: [PATCH 81/87] Don't output to stderr in the GUI --- Source/QtDialog/QCMake.cxx | 1 - 1 file changed, 1 deletion(-) diff --git a/Source/QtDialog/QCMake.cxx b/Source/QtDialog/QCMake.cxx index 48871c204..182db0f87 100644 --- a/Source/QtDialog/QCMake.cxx +++ b/Source/QtDialog/QCMake.cxx @@ -166,7 +166,6 @@ void QCMake::configure() this->CMakeInstance->CreateGlobalGenerator(this->Generator.toAscii().data())); this->CMakeInstance->LoadCache(); this->CMakeInstance->SetSuppressDevWarnings(this->SuppressDevWarnings); - std::cerr << "set warn uninitialized " << this->WarnUninitializedMode << "\n"; this->CMakeInstance->SetWarnUninitialized(this->WarnUninitializedMode); this->CMakeInstance->SetWarnUnused(this->WarnUnusedMode); this->CMakeInstance->PreLoadCMakeFiles(); From 89c25443a62de7a06fc0daa9f552a70aa9692aa0 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Mon, 3 Jan 2011 08:47:04 -0500 Subject: [PATCH 82/87] Checking for a definition is a usage --- Source/cmMakefile.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 0913e0add..704404926 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -2176,6 +2176,7 @@ const char* cmMakefile::GetRequiredDefinition(const char* name) const bool cmMakefile::IsDefinitionSet(const char* name) const { const char* def = this->Internal->VarStack.top().Get(name); + this->Internal->VarUsageStack.top().insert(name); if(!def) { def = this->GetCacheManager()->GetCacheValue(name); From 729db484efac18194076c4020fe9b6a87f24ed22 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 11 Jan 2011 17:10:28 -0500 Subject: [PATCH 83/87] Fix ArgumentExpansion test expected results Teach the ArgumentExpansion test to expect flattened lists as has always been the case in the CMake language. Now that the test should pass enable the failure regex even when CMAKE_STRICT is not on. Replace the reference to the old ArgumentExpansion test behavior in the workaround comment in cmMakefile::TryCompile with a full inline explanation. --- Source/cmMakefile.cxx | 24 ++++++++++++++++++++++-- Tests/ArgumentExpansion/CMakeLists.txt | 13 +++++++------ Tests/CMakeLists.txt | 6 ++---- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 704404926..c6e34f88b 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -2845,8 +2845,28 @@ int cmMakefile::TryCompile(const char *srcdir, const char *bindir, // if cmake args were provided then pass them in if (cmakeArgs) { - // FIXME: Workaround to ignore unused CLI variables until the - // 'ArgumentExpansion' test succeeds with CMAKE_STRICT on + // FIXME: Workaround to ignore unused CLI variables in try-compile. + // + // Ideally we should use SetArgs to honor options like --warn-unused-vars. + // 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 + // this case a list argument like "-DVAR=a;b" gets split into multiple + // cmake arguments "-DVAR=a" and "b". Currently SetCacheArgs ignores + // argument "b" and uses just "-DVAR=a", leading to a subtle bug in that + // the try_compile or try_run does not get the proper value of VAR. If we + // call SetArgs here then it would treat "b" as the source directory and + // cause an error such as "The source directory .../CMakeFiles/CMakeTmp/b + // does not exist", thus breaking the try_compile or try_run completely. + // + // Strictly speaking the bug is in the wrapper macro because the CMake + // language has always flattened nested lists and the macro should escape + // the semicolons in its arguments before forwarding them. However, this + // bug is so subtle that projects typically work anyway, usually because + // 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. cm.SetWarnUnusedCli(false); //cm.SetArgs(*cmakeArgs, true); diff --git a/Tests/ArgumentExpansion/CMakeLists.txt b/Tests/ArgumentExpansion/CMakeLists.txt index 62017067a..a24636f58 100644 --- a/Tests/ArgumentExpansion/CMakeLists.txt +++ b/Tests/ArgumentExpansion/CMakeLists.txt @@ -16,11 +16,11 @@ function (argument_tester expected expected_len) list(GET ARGN ${i} argn_value) list(GET ${expected} ${i} expected_value) - if (NOT ${argn_value} STREQUAL ${expected_value}) + if (NOT "${argn_value}" STREQUAL "${expected_value}") message(STATUS "Unexpected: Argument ${i} doesn't match") message(STATUS " Expected: ${expected_value}") message(STATUS " Received: ${argn_value}") - endif (NOT ${argn_value} STREQUAL ${expected_value}) + endif () math(EXPR i "${i} + 1") endwhile (i LESS ${argn_len}) @@ -50,10 +50,11 @@ set(nested_list_arg_test "${multiple_arg_test}" "first arg" "second arg") -message(STATUS "Test: Nested list argument") -argument_tester(nested_list_arg_test 3 ${nested_list_arg_test}) +message(STATUS "Test: Nested list argument flattens") +argument_tester(nested_list_arg_test 4 ${nested_list_arg_test}) set(semicolon_arg_test "pre\;post") -message(STATUS "Test: Semicolon argument") -argument_tester(semicolon_arg_test 1 ${semicolon_arg_test}) +set(semicolon_arg_test_flat "pre;post") +message(STATUS "Test: Semicolon argument flattens") +argument_tester(semicolon_arg_test_flat 2 ${semicolon_arg_test}) diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 02d393bb5..27cff3f8f 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -383,10 +383,8 @@ ${CMake_BINARY_DIR}/bin/cmake -DVERSION=master -P ${CMake_SOURCE_DIR}/Utilities/ --build-makeprogram ${CMAKE_TEST_MAKEPROGRAM} --build-exe-dir "${CMake_BINARY_DIR}/Tests/ArgumentExpansion/bin" ) - IF(CMAKE_STRICT) - SET_TESTS_PROPERTIES(ArgumentExpansion PROPERTIES - FAIL_REGULAR_EXPRESSION "Unexpected: ") - ENDIF(CMAKE_STRICT) + SET_TESTS_PROPERTIES(ArgumentExpansion PROPERTIES + FAIL_REGULAR_EXPRESSION "Unexpected: ") LIST(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/ArgumentExpansion") ADD_TEST(CustomCommand ${CMAKE_CTEST_COMMAND} From 8ed3c85c4749274cd30eef808d585634c38f3c72 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 13 Jan 2011 17:32:33 -0500 Subject: [PATCH 84/87] Give a better message for unused variables --- Source/cmake.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/cmake.cxx b/Source/cmake.cxx index c5de2e831..548e59a09 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -4321,7 +4321,7 @@ void cmake::RunCheckForUnusedVariables(const std::string& reason) const if(!it->second) { std::string message = "CMake Warning: The variable, '" + it->first + - "', given on the command line, was not used during the " + reason + + "', specified manually, was not used during the " + reason + "."; cmSystemTools::Message(message.c_str()); } From 8354413463fd4b13185388f57dc1b2f5cfd8298d Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 13 Jan 2011 17:58:04 -0500 Subject: [PATCH 85/87] Add method to unwatch a manual variable --- Source/cmake.cxx | 8 ++++++++ Source/cmake.h | 1 + 2 files changed, 9 insertions(+) diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 548e59a09..0d1c4ef2f 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -4309,6 +4309,14 @@ void cmake::WatchUnusedCli(const char* var) #endif } +void cmake::UnwatchUnusedCli(const char* var) +{ +#ifdef CMAKE_BUILD_WITH_CMAKE + this->VariableWatch->RemoveWatch(var, cmWarnUnusedCliWarning); + this->UsedCliVariables[var] = true; +#endif +} + void cmake::RunCheckForUnusedVariables(const std::string& reason) const { #ifdef CMAKE_BUILD_WITH_CMAKE diff --git a/Source/cmake.h b/Source/cmake.h index 4d348bb8a..1bb42d32c 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -366,6 +366,7 @@ class cmake const std::vector& nativeOptions, bool clean); + void UnwatchUnusedCli(const char* var); void WatchUnusedCli(const char* var); void RunCheckForUnusedVariables(const std::string& reason) const; protected: From 393903218dc5ca67d23cc5d31f9a5c87cd06ca17 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 13 Jan 2011 17:58:23 -0500 Subject: [PATCH 86/87] Unwatch manual variables upon removal in ccmake --- Source/CursesDialog/cmCursesMainForm.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/Source/CursesDialog/cmCursesMainForm.cxx b/Source/CursesDialog/cmCursesMainForm.cxx index 7f3e36010..71c06d500 100644 --- a/Source/CursesDialog/cmCursesMainForm.cxx +++ b/Source/CursesDialog/cmCursesMainForm.cxx @@ -786,6 +786,7 @@ void cmCursesMainForm::RemoveEntry(const char* value) const char* val = (*it)->GetValue(); if ( val && !strcmp(value, val) ) { + this->CMakeInstance->UnwatchUnusedCli(value); this->Entries->erase(it); break; } From 949d32c3067830a1376950fc78dbcde39bc378a8 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 13 Jan 2011 17:59:04 -0500 Subject: [PATCH 87/87] Unwatch manual variables upon removal in cmake-gui --- Source/QtDialog/QCMake.cxx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Source/QtDialog/QCMake.cxx b/Source/QtDialog/QCMake.cxx index 182db0f87..a40a175f7 100644 --- a/Source/QtDialog/QCMake.cxx +++ b/Source/QtDialog/QCMake.cxx @@ -242,6 +242,8 @@ void QCMake::setProperties(const QCMakePropertyList& newProps) // remove some properites foreach(QString s, toremove) { + this->CMakeInstance->UnwatchUnusedCli(s.toAscii().data()); + cachem->RemoveCacheEntry(s.toAscii().data()); }