Merge topic 'fix-warn-uninitialized-in-configure'

cbf0c0fce4 cmake: Enable --warn-uninitialized inside string(CONFIGURE) and configure_file
1d32a35c10 cmCommandArgumentParserHelper: use cmMakefile::MaybeWarnUninitialized
67ac4ed1dc cmMakefile: Move uninitialized vars logic into MaybeWarnUninitialized()
5257af3634 cmMakefile: move common logic to IsProjectFile function

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !2676

7 files changed, 118 insertions, 58 deletions

diff --git a/Source/cmCommandArgumentParserHelper.cxx b/Source/cmCommandArgumentParserHelper.cxx
index 2b4ceaa..ca29967 100644
--- a/Source/cmCommandArgumentParserHelper.cxx
+++ b/Source/cmCommandArgumentParserHelper.cxx
@@ -6,7 +6,6 @@
 #include "cmMakefile.h"
 #include "cmState.h"
 #include "cmSystemTools.h"
-#include "cmake.h"
 
 #include <iostream>
 #include <sstream>
@@ -16,8 +15,6 @@ int cmCommandArgument_yyparse(yyscan_t yyscanner);
 //
 cmCommandArgumentParserHelper::cmCommandArgumentParserHelper()
 {
-  this->WarnUninitialized = false;
-  this->CheckSystemVars = false;
   this->FileLine = -1;
   this->FileName = nullptr;
   this->RemoveEmpty = true;
@@ -95,23 +92,11 @@ const char* cmCommandArgumentParserHelper::ExpandVariable(const char* var)
     return this->AddString(ostr.str());
   }
   const char* value = this->Makefile->GetDefinition(var);
-  if (!value && !this->RemoveEmpty) {
-    // 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->WarnUninitialized && !this->Makefile->VariableInitialized(var)) {
-      if (this->CheckSystemVars ||
-          (this->FileName &&
-           (cmSystemTools::IsSubDirectory(
-              this->FileName, this->Makefile->GetHomeDirectory()) ||
-            cmSystemTools::IsSubDirectory(
-              this->FileName, this->Makefile->GetHomeOutputDirectory())))) {
-        std::ostringstream msg;
-        msg << "uninitialized variable \'" << var << "\'";
-        this->Makefile->IssueMessage(cmake::AUTHOR_WARNING, msg.str());
-      }
+  if (!value) {
+    this->Makefile->MaybeWarnUninitialized(var, this->FileName);
+    if (!this->RemoveEmpty) {
+      return nullptr;
     }
-    return nullptr;
   }
   if (this->EscapeQuotes && value) {
     return this->AddString(cmSystemTools::EscapeQuotes(value));
@@ -286,8 +271,6 @@ void cmCommandArgumentParserHelper::Error(const char* str)
 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 098c000..4dc238e 100644
--- a/Source/cmCommandArgumentParserHelper.h
+++ b/Source/cmCommandArgumentParserHelper.h
@@ -75,8 +75,6 @@ private:
   long FileLine;
   int CurrentLine;
   int Verbose;
-  bool WarnUninitialized;
-  bool CheckSystemVars;
   bool EscapeQuotes;
   bool NoEscapeMode;
   bool ReplaceAtSyntax;
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index d7c4f22..68a5101 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -1837,6 +1837,23 @@ bool cmMakefile::VariableInitialized(const std::string& var) const
   return this->StateSnapshot.IsInitialized(var);
 }
 
+void cmMakefile::MaybeWarnUninitialized(std::string const& variable,
+                                        const char* sourceFilename) const
+{
+  // 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->GetCMakeInstance()->GetWarnUninitialized() &&
+      !this->VariableInitialized(variable)) {
+    if (this->CheckSystemVars ||
+        (sourceFilename && this->IsProjectFile(sourceFilename))) {
+      std::ostringstream msg;
+      msg << "uninitialized variable \'" << variable << "\'";
+      this->IssueMessage(cmake::AUTHOR_WARNING, msg.str());
+    }
+  }
+}
+
 void cmMakefile::LogUnused(const char* reason, const std::string& name) const
 {
   if (this->WarnUnused) {
@@ -1848,11 +1865,7 @@ void cmMakefile::LogUnused(const char* reason, const std::string& name) const
       path += "/CMakeLists.txt";
     }
 
-    if (this->CheckSystemVars ||
-        cmSystemTools::IsSubDirectory(path, this->GetHomeDirectory()) ||
-        (cmSystemTools::IsSubDirectory(path, this->GetHomeOutputDirectory()) &&
-         !cmSystemTools::IsSubDirectory(path,
-                                        cmake::GetCMakeFilesDirectory()))) {
+    if (this->CheckSystemVars || this->IsProjectFile(path.c_str())) {
       std::ostringstream msg;
       msg << "unused variable (" << reason << ") \'" << name << "\'";
       this->IssueMessage(cmake::AUTHOR_WARNING, msg.str());
@@ -2509,9 +2522,9 @@ const std::string& cmMakefile::ExpandVariablesInString(
       // Suppress variable watches to avoid calling hooks twice. Suppress new
       // dereferences since the OLD behavior is still what is actually used.
       this->SuppressSideEffects = true;
-      newError = ExpandVariablesInStringNew(
-        newErrorstr, newResult, escapeQuotes, noEscapes, atOnly, filename,
-        line, removeEmpty, replaceAt);
+      newError = ExpandVariablesInStringNew(newErrorstr, newResult,
+                                            escapeQuotes, noEscapes, atOnly,
+                                            filename, line, replaceAt);
       this->SuppressSideEffects = false;
       CM_FALLTHROUGH;
     }
@@ -2524,9 +2537,9 @@ const std::string& cmMakefile::ExpandVariablesInString(
     case cmPolicies::REQUIRED_ALWAYS:
     // Messaging here would be *very* verbose.
     case cmPolicies::NEW:
-      mtype = ExpandVariablesInStringNew(errorstr, source, escapeQuotes,
-                                         noEscapes, atOnly, filename, line,
-                                         removeEmpty, replaceAt);
+      mtype =
+        ExpandVariablesInStringNew(errorstr, source, escapeQuotes, noEscapes,
+                                   atOnly, filename, line, replaceAt);
       break;
   }
 
@@ -2702,10 +2715,18 @@ struct t_lookup
   size_t loc = 0;
 };
 
+bool cmMakefile::IsProjectFile(const char* filename) const
+{
+  return cmSystemTools::IsSubDirectory(filename, this->GetHomeDirectory()) ||
+    (cmSystemTools::IsSubDirectory(filename, this->GetHomeOutputDirectory()) &&
+     !cmSystemTools::IsSubDirectory(filename,
+                                    cmake::GetCMakeFilesDirectory()));
+}
+
 cmake::MessageType cmMakefile::ExpandVariablesInStringNew(
   std::string& errorstr, std::string& source, bool escapeQuotes,
   bool noEscapes, bool atOnly, const char* filename, long line,
-  bool removeEmpty, bool replaceAt) const
+  bool replaceAt) const
 {
   // This method replaces ${VAR} and @VAR@ where VAR is looked up
   // with GetDefinition(), if not found in the map, nothing is expanded.
@@ -2762,23 +2783,8 @@ cmake::MessageType cmMakefile::ExpandVariablesInStringNew(
             } else {
               varresult = value;
             }
-          } else if (!removeEmpty && !this->SuppressSideEffects) {
-            // 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->GetCMakeInstance()->GetWarnUninitialized() &&
-                !this->VariableInitialized(lookup)) {
-              if (this->CheckSystemVars ||
-                  (filename &&
-                   (cmSystemTools::IsSubDirectory(filename,
-                                                  this->GetHomeDirectory()) ||
-                    cmSystemTools::IsSubDirectory(
-                      filename, this->GetHomeOutputDirectory())))) {
-                std::ostringstream msg;
-                msg << "uninitialized variable \'" << lookup << "\'";
-                this->IssueMessage(cmake::AUTHOR_WARNING, msg.str());
-              }
-            }
+          } else if (!this->SuppressSideEffects) {
+            this->MaybeWarnUninitialized(lookup, filename);
           }
           result.replace(var.loc, result.size() - var.loc, varresult);
           // Start looking from here on out.
@@ -2890,7 +2896,12 @@ cmake::MessageType cmMakefile::ExpandVariablesInStringNew(
             if (filename && variable == lineVar) {
               varresult = std::to_string(line);
             } else {
-              varresult = this->GetSafeDefinition(variable);
+              const std::string* def = this->GetDef(variable);
+              if (def) {
+                varresult = *def;
+              } else if (!this->SuppressSideEffects) {
+                this->MaybeWarnUninitialized(variable, filename);
+              }
             }
 
             if (escapeQuotes) {
diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index aa94054..1607735 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -866,6 +866,9 @@ public:
   std::deque<std::vector<std::string>> FindPackageRootPathStack;
 
   void MaybeWarnCMP0074(std::string const& pkg);
+  void MaybeWarnUninitialized(std::string const& variable,
+                              const char* sourceFilename) const;
+  bool IsProjectFile(const char* filename) const;
 
 protected:
   // add link libraries and directories to the target
@@ -987,7 +990,7 @@ private:
   cmake::MessageType ExpandVariablesInStringNew(
     std::string& errorstr, std::string& source, bool escapeQuotes,
     bool noEscapes, bool atOnly, const char* filename, long line,
-    bool removeEmpty, bool replaceAt) const;
+    bool replaceAt) const;
   /**
    * Old version of GetSourceFileWithOutput(const std::string&) kept for
    * backward-compatibility. It implements a linear search and support
diff --git a/Tests/RunCMake/CommandLine/RunCMakeTest.cmake b/Tests/RunCMake/CommandLine/RunCMakeTest.cmake
index a37b7f1..4cd34de 100644
--- a/Tests/RunCMake/CommandLine/RunCMakeTest.cmake
+++ b/Tests/RunCMake/CommandLine/RunCMakeTest.cmake
@@ -350,7 +350,7 @@ set(RunCMake_TEST_OPTIONS --trace-expand --warn-uninitialized)
 run_cmake(trace-expand-warn-uninitialized)
 unset(RunCMake_TEST_OPTIONS)
 
-set(RunCMake_TEST_OPTIONS --warn-uninitialized)
+set(RunCMake_TEST_OPTIONS -Wno-deprecated --warn-uninitialized)
 run_cmake(warn-uninitialized)
 unset(RunCMake_TEST_OPTIONS)
 
diff --git a/Tests/RunCMake/CommandLine/warn-uninitialized-stderr.txt b/Tests/RunCMake/CommandLine/warn-uninitialized-stderr.txt
index a13402a..41ba098 100644
--- a/Tests/RunCMake/CommandLine/warn-uninitialized-stderr.txt
+++ b/Tests/RunCMake/CommandLine/warn-uninitialized-stderr.txt
@@ -1,5 +1,53 @@
-^CMake Warning \(dev\) at warn-uninitialized.cmake:1 \(set\):
-  uninitialized variable 'WARN_FROM_NORMAL_CMAKE_FILE'
+^CMake Warning \(dev\) at warn-uninitialized.cmake:3 \(set\):
+  uninitialized variable 'OLD_WARN_FROM_NORMAL_CMAKE_FILE_INSIDE_BRACES'
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)
+This warning is for project developers.  Use -Wno-dev to suppress it.
+
+CMake Warning \(dev\) at warn-uninitialized.cmake:4 \(set\):
+  uninitialized variable 'OLD_WARN_FROM_NORMAL_CMAKE_FILE_IN_ATS'
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)
+This warning is for project developers.  Use -Wno-dev to suppress it.
+
+CMake Warning \(dev\) at warn-uninitialized.cmake:5 \(string\):
+  uninitialized variable 'OLD_WARN_FROM_STRING_CONFIGURE_INSIDE_BRACES'
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)
+This warning is for project developers.  Use -Wno-dev to suppress it.
+
+CMake Warning \(dev\) at warn-uninitialized.cmake:7 \(configure_file\):
+  uninitialized variable 'OLD_WARN_FROM_CONFIGURE_FILE_INSIDE_AT'
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)
+This warning is for project developers.  Use -Wno-dev to suppress it.
+
+CMake Warning \(dev\) at warn-uninitialized.cmake:8 \(string\):
+  uninitialized variable 'OLD_WARN_FROM_STRING_CONFIGURE_INSIDE_AT'
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)
+This warning is for project developers.  Use -Wno-dev to suppress it.
+
+CMake Warning \(dev\) at warn-uninitialized.cmake:13 \(set\):
+  uninitialized variable 'NEW_WARN_FROM_NORMAL_CMAKE_FILE_INSIDE_BRACES'
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)
+This warning is for project developers.  Use -Wno-dev to suppress it.
+
+CMake Warning \(dev\) at warn-uninitialized.cmake:14 \(string\):
+  uninitialized variable 'NEW_WARN_FROM_STRING_CONFIGURE_INSIDE_BRACES'
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)
+This warning is for project developers.  Use -Wno-dev to suppress it.
+
+CMake Warning \(dev\) at warn-uninitialized.cmake:16 \(configure_file\):
+  uninitialized variable 'NEW_WARN_FROM_CONFIGURE_FILE_INSIDE_AT'
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)
+This warning is for project developers.  Use -Wno-dev to suppress it.
+
+CMake Warning \(dev\) at warn-uninitialized.cmake:17 \(string\):
+  uninitialized variable 'NEW_WARN_FROM_STRING_CONFIGURE_INSIDE_AT'
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)
 This warning is for project developers.  Use -Wno-dev to suppress it.$
diff --git a/Tests/RunCMake/CommandLine/warn-uninitialized.cmake b/Tests/RunCMake/CommandLine/warn-uninitialized.cmake
index f1a75c9..ff65c16 100644
--- a/Tests/RunCMake/CommandLine/warn-uninitialized.cmake
+++ b/Tests/RunCMake/CommandLine/warn-uninitialized.cmake
@@ -1 +1,18 @@
-set(FOO "${WARN_FROM_NORMAL_CMAKE_FILE}")
+cmake_policy(PUSH)
+cmake_policy(SET CMP0053 OLD)
+set(FOO "${OLD_WARN_FROM_NORMAL_CMAKE_FILE_INSIDE_BRACES}")
+set(FOO "@OLD_WARN_FROM_NORMAL_CMAKE_FILE_IN_ATS@")
+string(CONFIGURE "\${OLD_WARN_FROM_STRING_CONFIGURE_INSIDE_BRACES}" OUT1)
+file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/file1.in "\@OLD_WARN_FROM_CONFIGURE_FILE_INSIDE_AT\@")
+configure_file(${CMAKE_CURRENT_BINARY_DIR}/file1.in file1.out)
+string(CONFIGURE "\@OLD_WARN_FROM_STRING_CONFIGURE_INSIDE_AT\@" OUT2)
+cmake_policy(POP)
+
+cmake_policy(PUSH)
+cmake_policy(SET CMP0053 NEW)
+set(FOO "${NEW_WARN_FROM_NORMAL_CMAKE_FILE_INSIDE_BRACES}")
+string(CONFIGURE "\${NEW_WARN_FROM_STRING_CONFIGURE_INSIDE_BRACES}" OUT3)
+file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/file2.in "\@NEW_WARN_FROM_CONFIGURE_FILE_INSIDE_AT\@")
+configure_file(${CMAKE_CURRENT_BINARY_DIR}/file2.in file2.out)
+string(CONFIGURE "@NEW_WARN_FROM_STRING_CONFIGURE_INSIDE_AT@" OUT4)
+cmake_policy(POP)