Merge topic 'dev/strict-mode'

dd2f814 Merge branch 'dev/add_test-working-directory' into dev/strict-mode
949d32c Unwatch manual variables upon removal in cmake-gui
3939032 Unwatch manual variables upon removal in ccmake
8354413 Add method to unwatch a manual variable
8ed3c85 Give a better message for unused variables
729db48 Fix ArgumentExpansion test expected results
89c2544 Checking for a definition is a usage
5625dee Don't output to stderr in the GUI
ad25a96 Merge branch 'ImprovedDotSupport2' into dev/strict-mode
c128abe Merge branch 'AddCMAKE_CURRENT_LIST_DIR' into dev/strict-mode
9bcaff0 Merge branch 'cmake-guiRememberAdvancedCheckbox' into dev/strict-mode
544d0c3 Fix expected output for WarnUninitialized test
4e3bea4 Update expected messages to new format
8e8c9e4 Don't check at destruction for usage
668e005 Use cmake::IssueMessage for warnings
88cd4c1 Use 'CMake Warning' versus 'warning' for CDash
3c3b98d Initialize the class before setting warn flags
cf8b15a Ignore files under the CMakeFiles directory
fd50f06 Don't check for unused vars at configure time
447a04c Don't warn during configure when doing everything
b97ee21 Check for unused variables at the end of generate
c18c977 When checking for variables, specify a reason
3f1121f Use a long int since Line is a long as well
2507f93 Change the failure case string to 'Unexpected'
fe390a2 Add 'ArgumentExpansion' test
8dbb209 Wrong boolean value for CLI warnings
d4ee998 Hard-code the --no-warn-unused-cli flag
a267b99 Fix line lengths
82ed104 Flag that the directories have been set
5aa535b Add argument to arg parsing to not set directories
367e5c3 Revert "Revert "When calling CMake, set the args and the cache""
ab5d4e4 Revert "When calling CMake, set the args and the cache"
9b90040 When calling CMake, set the args and the cache
fe56002 Fix long lines for KWStyle
5d30cfc Set a watch on variables added through the gui
33c63b1 Add a method to put a watch for variables
535253f Initialize the warning variables earlier
cbb286c Fix the path detection to work for top-level
62be1f7 Initialize the usage stack earlier
c6e7fab Factor out the checks for unused variables
5e41ba8 When using the API, check for Add vs. Remove
dee1976 Fix typo in VariableUnusedViaUnset test
f231ce5 Remove old false positive avoidance code
a117e02 Revert "Add test for unused warnings at the end of scope"
2c82f2b Exempt CMAKE(CURRENT|PARENT)_LIST_FILE from usage
6d7d449 Ignore CLI warnings for ABI determination
7740a73 Only return local keys that are defined
bef3aee Use the API so that warnings can be tracked
05cb0f4 Check for unused variables in the dtor
91c4c99 Add test for unused warnings at the end of scope
ca90f67 Fix detection of unused variables when setting
f7438ca Add test for unused detection via setting it
995cfb0 Don't warn if the variable wasn't defined
aefc91d Add test for usage checks via unset
a8e97f8 Remove VarRemoved code since it's been superceded
59463ef Rework CheckVariableForUnused usage
f117423 Fix line lengths to be no more than 78
e49a935 Improve unused warning logic
e01e40c Mark ARGC, ARGV*, and ARGN as used
a17aff7 Ignore CMAKE_MATCH_* variables for usage
02a114d Add method to allow variables to be marked as used
a0b0d23 CMAKE_DO_TRY_COMPILE is no longer used
ae3eff3 Fix the path used for ignoring system warnings
056b441 Fix missing case for usage of a variable
980e048 Factor out checks for unused variables
83acb0a Remove now unused variables
3801463 Use built-ins for readability and maintainability
8b52015 Push the initialize and unused states when copying
439877f Be consistent with single and double quotes
4cf1706 Add documentation for check-system-vars
b74777f Fix the spelling of the flag for warn-unused-vars
b948120 Change logic of flag to turn off cli unused checks
f047a17 Add test for uninitialized variables
75bda38 Add tests for unused command line variables
300fc15 Fix detection of system files
d784e6a Run the unused variables check on the final pass
9efc057 VariableWatch is not available when bootstrapping
2e78224 Add a missing comma to the warning message
7499700 Add a flag to warn about system files
fff9f6d Rename flags again and use variablewatch for cli
786e269 Add warn-unused to the Qt interface
636e6c4 Default to marking things as used
4ff0340 Rename find-unused to warn-unused
d7999e9 Rename strict-mode to warn-uninitialized
e141bc9 Detect unused variables
d3e8eb5 Add flags to detect unused variables
f332e14 Complete strict-mode checks for uninitialized vars
52f9637 Add method to get the local scope variables
f794d58 Make --strict-mode option, and integrate with cmake-gui
48b5b85 Add a warning when variables are used uninitialized.
cd626ea For macros make sure the FilePath points to a valid pointer in the args.

1 files changed, 199 insertions, 3 deletions

diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index e443b86..e14e44d 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -44,12 +44,22 @@ class cmMakefile::Internals
 {
 public:
   std::stack<cmDefinitions, std::list<cmDefinitions> > VarStack;
+  std::stack<std::set<cmStdString> > VarInitStack;
+  std::stack<std::set<cmStdString> > VarUsageStack;
 };
 
 // default is not to be building executables
 cmMakefile::cmMakefile(): Internal(new Internals)
 {
-  this->Internal->VarStack.push(cmDefinitions());
+  const cmDefinitions& defs = cmDefinitions();
+  const std::set<cmStdString> globalKeys = defs.LocalKeys();
+  this->Internal->VarStack.push(defs);
+  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 = "^.*$";
@@ -92,6 +102,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;
@@ -129,8 +141,10 @@ cmMakefile::cmMakefile(const cmMakefile& mf): Internal(new Internals)
   this->SubDirectoryOrder = mf.SubDirectoryOrder;
   this->Properties = mf.Properties;
   this->PreOrder = mf.PreOrder;
-  this->ListFileStack = mf.ListFileStack;
+  this->WarnUnused = mf.WarnUnused;
   this->Initialize();
+  this->CheckSystemVars = mf.CheckSystemVars;
+  this->ListFileStack = mf.ListFileStack;
 }
 
 //----------------------------------------------------------------------------
@@ -571,6 +585,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;
@@ -611,8 +626,10 @@ bool cmMakefile::ReadListFile(const char* filename_in,
     }
 
   this->AddDefinition("CMAKE_CURRENT_LIST_FILE", filenametoread);
+  this->MarkVariableAsUsed("CMAKE_CURRENT_LIST_FILE");
   this->AddDefinition("CMAKE_CURRENT_LIST_DIR",
                        cmSystemTools::GetFilenamePath(filenametoread).c_str());
+  this->MarkVariableAsUsed("CMAKE_CURRENT_LIST_DIR");
 
   // try to see if the list file is the top most
   // list file for a project, and if it is, then it
@@ -645,9 +662,12 @@ 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");
     this->AddDefinition("CMAKE_CURRENT_LIST_DIR",
                         cmSystemTools::GetFilenamePath(currentFile).c_str());
+    this->MarkVariableAsUsed("CMAKE_CURRENT_LIST_DIR");
     return false;
     }
   // add this list file to the list of dependencies
@@ -687,13 +707,19 @@ 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");
   this->AddDefinition("CMAKE_CURRENT_LIST_DIR",
                       cmSystemTools::GetFilenamePath(currentFile).c_str());
+  this->MarkVariableAsUsed("CMAKE_CURRENT_LIST_DIR");
 
   // pop the listfile off the stack
   this->ListFileStack.pop_back();
 
+  // Check for unused variables
+  this->CheckForUnusedVariables();
+
   return true;
 }
 
@@ -759,6 +785,8 @@ void cmMakefile::SetLocalGenerator(cmLocalGenerator* lg)
   this->AddSourceGroup("Resources", "\\.plist$");
 #endif
 
+  this->WarnUnused = this->GetCMakeInstance()->GetWarnUnused();
+  this->CheckSystemVars = this->GetCMakeInstance()->GetCheckSystemVars();
 }
 
 bool cmMakefile::NeedBackwardsCompatibility(unsigned int major,
@@ -1627,6 +1655,13 @@ void cmMakefile::AddDefinition(const char* name, const char* value)
 #endif
 
   this->Internal->VarStack.top().Set(name, value);
+  if (this->Internal->VarUsageStack.size() &&
+      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();
@@ -1691,6 +1726,13 @@ 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() &&
+      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 )
@@ -1701,9 +1743,90 @@ void cmMakefile::AddDefinition(const char* name, bool value)
 #endif
 }
 
+void cmMakefile::CheckForUnusedVariables() const
+{
+  const cmDefinitions& defs = this->Internal->VarStack.top();
+  const std::set<cmStdString>& locals = defs.LocalKeys();
+  std::set<cmStdString>::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);
+}
+
+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::VariableUsed(const char* var) const
+{
+  if(this->Internal->VarUsageStack.top().find(var) !=
+      this->Internal->VarUsageStack.top().end())
+    {
+    return true;
+    }
+  return false;
+}
+
+void cmMakefile::CheckForUnused(const char* reason, const char* name) const
+{
+  if (this->WarnUnused && !this->VariableUsed(name))
+    {
+    cmStdString path;
+    cmListFileBacktrace bt;
+    if (this->CallStack.size())
+      {
+      const cmListFileContext* file = this->CallStack.back().Context;
+      bt.push_back(*file);
+      path = file->FilePath.c_str();
+      }
+    else
+      {
+      path = this->GetStartDirectory();
+      path += "/CMakeLists.txt";
+      cmListFileContext lfc;
+      lfc.FilePath = path;
+      lfc.Line = 0;
+      bt.push_back(lfc);
+      }
+    if (this->CheckSystemVars ||
+        cmSystemTools::IsSubDirectory(path.c_str(),
+                                      this->GetHomeDirectory()) ||
+        (cmSystemTools::IsSubDirectory(path.c_str(),
+                                      this->GetHomeOutputDirectory()) &&
+        !cmSystemTools::IsSubDirectory(path.c_str(),
+                                cmake::GetCMakeFilesDirectory())))
+      {
+      cmOStringStream msg;
+      msg << "unused variable (" << reason << ") \'" << name << "\'";
+      this->GetCMakeInstance()->IssueMessage(cmake::AUTHOR_WARNING,
+                                             msg.str().c_str(),
+                                             bt);
+      }
+    }
+}
+
 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);
 #ifdef CMAKE_BUILD_WITH_CMAKE
   cmVariableWatch* vv = this->GetVariableWatch();
   if ( vv )
@@ -2055,6 +2178,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);
@@ -2082,6 +2206,10 @@ const char* cmMakefile::GetDefinition(const char* name) const
       RecordPropertyAccess(name,cmProperty::VARIABLE);
     }
 #endif
+  if (this->WarnUnused)
+    {
+    this->Internal->VarUsageStack.top().insert(name);
+    }
   const char* def = this->Internal->VarStack.top().Get(name);
   if(!def)
     {
@@ -2733,6 +2861,31 @@ 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 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);
+
     cm.SetCacheArgs(*cmakeArgs);
     }
   // to save time we pass the EnableLanguage info directly
@@ -3416,12 +3569,48 @@ std::string cmMakefile::GetListFileStack()
 void cmMakefile::PushScope()
 {
   cmDefinitions* parent = &this->Internal->VarStack.top();
+  const std::set<cmStdString>& init = this->Internal->VarInitStack.top();
+  const std::set<cmStdString>& 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<cmStdString> init = this->Internal->VarInitStack.top();
+  std::set<cmStdString> usage = this->Internal->VarUsageStack.top();
+  const std::set<cmStdString>& locals = current->LocalKeys();
+  // Remove initialization and usage information for variables in the local
+  // scope.
+  std::set<cmStdString>::const_iterator it = locals.begin();
+  for (; it != locals.end(); ++it)
+    {
+    init.erase(*it);
+    if (!this->VariableUsed(it->c_str()))
+      {
+      this->CheckForUnused("out of scope", it->c_str());
+      }
+    else
+      {
+      usage.erase(*it);
+      }
+    }
   this->Internal->VarStack.pop();
+  this->Internal->VarInitStack.pop();
+  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)
@@ -3446,7 +3635,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->Internal->VarStack.top().Set(var, varDef);
+    if (varDef)
+      {
+      parent->AddDefinition(var, varDef);
+      }
+    else
+      {
+      parent->RemoveDefinition(var);
+      }
     }
   else
     {