Merge topic 'return-propagate-variables'

838a5fae23 return(): Propagate variables to result scope
8f0e1f2111 cmMakefile: add RaiseScope for list of variables

Acked-by: Kitware Robot <kwrobot@kitware.com>
Tested-by: buildbot <buildbot@kitware.com>
Acked-by: Robert Maynard <robertjmaynard@gmail.com>
Merge-request: !7634

11 files changed, 86 insertions, 17 deletions

diff --git a/Source/cmBlockCommand.cxx b/Source/cmBlockCommand.cxx
index c358aa2..ec79149 100644
--- a/Source/cmBlockCommand.cxx
+++ b/Source/cmBlockCommand.cxx
@@ -73,6 +73,7 @@ public:
 
 private:
   cmMakefile* Makefile;
+  ScopeSet Scopes;
   BlockScopePushPop BlockScope;
   std::vector<std::string> VariableNames;
 };
@@ -81,6 +82,7 @@ cmBlockFunctionBlocker::cmBlockFunctionBlocker(
   cmMakefile* const mf, const ScopeSet& scopes,
   std::vector<std::string> variableNames)
   : Makefile{ mf }
+  , Scopes{ scopes }
   , BlockScope{ mf, scopes }
   , VariableNames{ std::move(variableNames) }
 {
@@ -88,14 +90,8 @@ cmBlockFunctionBlocker::cmBlockFunctionBlocker(
 
 cmBlockFunctionBlocker::~cmBlockFunctionBlocker()
 {
-  for (auto const& varName : this->VariableNames) {
-    if (this->Makefile->IsNormalDefinitionSet(varName)) {
-      this->Makefile->RaiseScope(varName,
-                                 this->Makefile->GetDefinition(varName));
-    } else {
-      // unset variable in parent scope
-      this->Makefile->RaiseScope(varName, nullptr);
-    }
+  if (this->Scopes.contains(ScopeType::VARIABLES)) {
+    this->Makefile->RaiseScope(this->VariableNames);
   }
 }
 
@@ -118,7 +114,8 @@ bool cmBlockFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
     cmExecutionStatus status(mf);
     mf.ExecuteCommand(fn, status);
     if (status.GetReturnInvoked()) {
-      inStatus.SetReturnInvoked();
+      mf.RaiseScope(status.GetReturnVariables());
+      inStatus.SetReturnInvoked(status.GetReturnVariables());
       return true;
     }
     if (status.GetBreakInvoked()) {
diff --git a/Source/cmExecutionStatus.h b/Source/cmExecutionStatus.h
index 0feaedf..ced3548 100644
--- a/Source/cmExecutionStatus.h
+++ b/Source/cmExecutionStatus.h
@@ -5,6 +5,7 @@
 #include <cmConfigure.h> // IWYU pragma: keep
 
 #include <string>
+#include <vector>
 
 class cmMakefile;
 
@@ -27,8 +28,21 @@ public:
   void SetError(std::string const& e) { this->Error = e; }
   std::string const& GetError() const { return this->Error; }
 
-  void SetReturnInvoked() { this->ReturnInvoked = true; }
+  void SetReturnInvoked()
+  {
+    this->Variables.clear();
+    this->ReturnInvoked = true;
+  }
+  void SetReturnInvoked(std::vector<std::string> variables)
+  {
+    this->Variables = std::move(variables);
+    this->ReturnInvoked = true;
+  }
   bool GetReturnInvoked() const { return this->ReturnInvoked; }
+  const std::vector<std::string>& GetReturnVariables() const
+  {
+    return this->Variables;
+  }
 
   void SetBreakInvoked() { this->BreakInvoked = true; }
   bool GetBreakInvoked() const { return this->BreakInvoked; }
@@ -46,4 +60,5 @@ private:
   bool BreakInvoked = false;
   bool ContinueInvoked = false;
   bool NestedError = false;
+  std::vector<std::string> Variables;
 };
diff --git a/Source/cmForEachCommand.cxx b/Source/cmForEachCommand.cxx
index b9400c9..3465c23 100644
--- a/Source/cmForEachCommand.cxx
+++ b/Source/cmForEachCommand.cxx
@@ -260,7 +260,7 @@ auto cmForEachFunctionBlocker::invoke(
     cmExecutionStatus status(mf);
     mf.ExecuteCommand(func, status);
     if (status.GetReturnInvoked()) {
-      inStatus.SetReturnInvoked();
+      inStatus.SetReturnInvoked(status.GetReturnVariables());
       result.Break = true;
       break;
     }
diff --git a/Source/cmFunctionCommand.cxx b/Source/cmFunctionCommand.cxx
index 1359009..f4768b6 100644
--- a/Source/cmFunctionCommand.cxx
+++ b/Source/cmFunctionCommand.cxx
@@ -120,6 +120,7 @@ bool cmFunctionHelperCommand::operator()(
       return false;
     }
     if (status.GetReturnInvoked()) {
+      makefile.RaiseScope(status.GetReturnVariables());
       break;
     }
   }
diff --git a/Source/cmIfCommand.cxx b/Source/cmIfCommand.cxx
index 0da72b1..c2a09c1 100644
--- a/Source/cmIfCommand.cxx
+++ b/Source/cmIfCommand.cxx
@@ -150,7 +150,7 @@ bool cmIfFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
       cmExecutionStatus status(mf);
       mf.ExecuteCommand(func, status);
       if (status.GetReturnInvoked()) {
-        inStatus.SetReturnInvoked();
+        inStatus.SetReturnInvoked(status.GetReturnVariables());
         return true;
       }
       if (status.GetBreakInvoked()) {
diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx
index ef12487..47ad749 100644
--- a/Source/cmMacroCommand.cxx
+++ b/Source/cmMacroCommand.cxx
@@ -126,7 +126,7 @@ bool cmMacroHelperCommand::operator()(
       return false;
     }
     if (status.GetReturnInvoked()) {
-      inStatus.SetReturnInvoked();
+      inStatus.SetReturnInvoked(status.GetReturnVariables());
       return true;
     }
     if (status.GetBreakInvoked()) {
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index b228e2b..760ed5f 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -789,6 +789,7 @@ void cmMakefile::RunListFile(cmListFile const& listFile,
       break;
     }
     if (status.GetReturnInvoked()) {
+      this->RaiseScope(status.GetReturnVariables());
       // Exit early due to return command.
       break;
     }
@@ -4188,6 +4189,18 @@ void cmMakefile::RaiseScope(const std::string& var, const char* varDef)
 #endif
 }
 
+void cmMakefile::RaiseScope(const std::vector<std::string>& variables)
+{
+  for (auto const& varName : variables) {
+    if (this->IsNormalDefinitionSet(varName)) {
+      this->RaiseScope(varName, this->GetDefinition(varName));
+    } else {
+      // unset variable in parent scope
+      this->RaiseScope(varName, nullptr);
+    }
+  }
+}
+
 cmTarget* cmMakefile::AddImportedTarget(const std::string& name,
                                         cmStateEnums::TargetType type,
                                         bool global)
diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index 10432a8..1763177 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -880,6 +880,7 @@ public:
   {
     this->RaiseScope(var, value.GetCStr());
   }
+  void RaiseScope(const std::vector<std::string>& variables);
 
   // push and pop loop scopes
   void PushLoopBlockBarrier();
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index cb7402c..fdda31e 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -421,7 +421,9 @@ class cmMakefile;
   SELECT(                                                                     \
     POLICY, CMP0139,                                                          \
     "The if() command supports path comparisons using PATH_EQUAL operator.",  \
-    3, 24, 0, cmPolicies::WARN)
+    3, 24, 0, cmPolicies::WARN)                                               \
+  SELECT(POLICY, CMP0140, "The return() command checks its arguments.", 3,    \
+         25, 0, cmPolicies::WARN)
 
 #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
 #define CM_FOR_EACH_POLICY_ID(POLICY)                                         \
diff --git a/Source/cmReturnCommand.cxx b/Source/cmReturnCommand.cxx
index 5905669..765b772 100644
--- a/Source/cmReturnCommand.cxx
+++ b/Source/cmReturnCommand.cxx
@@ -2,12 +2,52 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmReturnCommand.h"
 
+#include <cm/string_view>
+#include <cmext/string_view>
+
 #include "cmExecutionStatus.h"
+#include "cmMakefile.h"
+#include "cmMessageType.h"
+#include "cmPolicies.h"
+#include "cmStringAlgorithms.h"
+#include "cmSystemTools.h"
 
 // cmReturnCommand
-bool cmReturnCommand(std::vector<std::string> const&,
+bool cmReturnCommand(std::vector<std::string> const& args,
                      cmExecutionStatus& status)
 {
-  status.SetReturnInvoked();
+  if (!args.empty()) {
+    switch (status.GetMakefile().GetPolicyStatus(cmPolicies::CMP0140)) {
+      case cmPolicies::WARN:
+        status.GetMakefile().IssueMessage(
+          MessageType::AUTHOR_WARNING,
+          cmStrCat(
+            cmPolicies::GetPolicyWarning(cmPolicies::CMP0140), '\n',
+            "return() checks its arguments when the policy is set to NEW. "
+            "Since the policy is not set the OLD behavior will be used so "
+            "the arguments will be ignored."));
+        CM_FALLTHROUGH;
+      case cmPolicies::OLD:
+        return true;
+      case cmPolicies::REQUIRED_IF_USED:
+      case cmPolicies::REQUIRED_ALWAYS:
+        status.GetMakefile().IssueMessage(
+          MessageType::FATAL_ERROR,
+          cmStrCat('\n', cmPolicies::GetPolicyWarning(cmPolicies::CMP0140)));
+        cmSystemTools::SetFatalErrorOccurred();
+        return false;
+      default:
+        break;
+    }
+    if (args[0] != "PROPAGATE"_s) {
+      status.SetError(
+        cmStrCat("called with unsupported argument \"", args[0], '"'));
+      cmSystemTools::SetFatalErrorOccurred();
+      return false;
+    }
+    status.SetReturnInvoked({ args.begin() + 1, args.end() });
+  } else {
+    status.SetReturnInvoked();
+  }
   return true;
 }
diff --git a/Source/cmWhileCommand.cxx b/Source/cmWhileCommand.cxx
index fb94273..e80d1fc 100644
--- a/Source/cmWhileCommand.cxx
+++ b/Source/cmWhileCommand.cxx
@@ -96,7 +96,7 @@ bool cmWhileFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
       cmExecutionStatus status(mf);
       mf.ExecuteCommand(fn, status);
       if (status.GetReturnInvoked()) {
-        inStatus.SetReturnInvoked();
+        inStatus.SetReturnInvoked(status.GetReturnVariables());
         return true;
       }
       if (status.GetBreakInvoked()) {