Browse Source

return(): Propagate variables to result scope

Fixes: #23871
Marc Chevrier 3 years ago
parent
commit
838a5fae23

+ 1 - 0
Help/command/block.rst

@@ -71,4 +71,5 @@ See Also
 ^^^^^^^^
 ^^^^^^^^
 
 
   * :command:`endblock`
   * :command:`endblock`
+  * :command:`return`
   * :command:`cmake_policy`
   * :command:`cmake_policy`

+ 35 - 1
Help/command/return.rst

@@ -5,7 +5,7 @@ Return from a file, directory or function.
 
 
 .. code-block:: cmake
 .. code-block:: cmake
 
 
-  return()
+  return([PROPAGATE <var-name>...])
 
 
 Returns from a file, directory or function.  When this command is
 Returns from a file, directory or function.  When this command is
 encountered in an included file (via :command:`include` or
 encountered in an included file (via :command:`include` or
@@ -16,5 +16,39 @@ deferred calls scheduled by :command:`cmake_language(DEFER)` are invoked and
 control is returned to the parent directory if there is one.  If return is
 control is returned to the parent directory if there is one.  If return is
 called in a function, control is returned to the caller of the function.
 called in a function, control is returned to the caller of the function.
 
 
+``PROPAGATE``
+  .. versionadded:: 3.25
+
+  This option set or unset the specified variables in the parent directory or
+  function caller scope. This is equivalent to :command:`set(PARENT_SCOPE)` or
+  :command:`unset(PARENT_SCOPE)` commands.
+
+  The option ``PROPAGATE`` can be very useful in conjunction with the
+  :command:`block` command because the :command:`return` will cross over
+  various scopes created by the :command:`block` commands.
+
+  .. code-block:: cmake
+
+    function(MULTI_SCOPES RESULT_VARIABLE)
+      block(SCOPE_FOR VARIABLES)
+        # here set(PARENT_SCOPE) is not usable because it will not set the
+        # variable in the caller scope but in the parent scope of the block()
+        set(${RESULT_VARIABLE} "new-value")
+        return(PROPAGATE ${RESULT_VARIABLE})
+      endblock()
+    endfunction()
+
+    set(MY_VAR "initial-value")
+    multi_scopes(MY_VAR)
+    # here MY_VAR will holds "new-value"
+
+Policy :policy:`CMP0140` controls the behavior regarding the arguments of the
+command.
+
 Note that a :command:`macro <macro>`, unlike a :command:`function <function>`,
 Note that a :command:`macro <macro>`, unlike a :command:`function <function>`,
 is expanded in place and therefore cannot handle ``return()``.
 is expanded in place and therefore cannot handle ``return()``.
+
+See Also
+^^^^^^^^
+
+  * :command:`block`

+ 3 - 3
Help/command/set.rst

@@ -30,9 +30,9 @@ applicable to the case at hand). The previous state of the variable's value
 stays the same in the current scope (e.g., if it was undefined before, it is
 stays the same in the current scope (e.g., if it was undefined before, it is
 still undefined and if it had a value, it is still that value).
 still undefined and if it had a value, it is still that value).
 
 
-The :command:`block(PROPAGATE)` command can be used as an alternate method to
-:command:`set(PARENT_SCOPE)` and :command:`unset(PARENT_SCOPE)` commands to
-update the parent scope.
+The :command:`block(PROPAGATE)` and :command:`return(PROPAGATE)` commands can
+be used as an alternate method to the :command:`set(PARENT_SCOPE)` and
+:command:`unset(PARENT_SCOPE)` commands to update the parent scope.
 
 
 Set Cache Entry
 Set Cache Entry
 ^^^^^^^^^^^^^^^
 ^^^^^^^^^^^^^^^

+ 8 - 0
Help/manual/cmake-policies.7.rst

@@ -52,6 +52,14 @@ to determine whether to report an error on use of deprecated macros or
 functions.
 functions.
 
 
 
 
+Policies Introduced by CMake 3.25
+=================================
+
+.. toctree::
+   :maxdepth: 1
+
+   CMP0140: The return() command checks its arguments. </policy/CMP0140>
+
 Policies Introduced by CMake 3.24
 Policies Introduced by CMake 3.24
 =================================
 =================================
 
 

+ 17 - 0
Help/policy/CMP0140.rst

@@ -0,0 +1,17 @@
+CMP0140
+-------
+
+.. versionadded:: 3.25
+
+The :command:`return` command checks its parameters.
+
+The ``OLD`` behavior for this policy is to ignore any parameters given to the
+command.
+The ``NEW`` behavior is to check validity of the parameters.
+
+This policy was introduced in CMake version 3.25.
+CMake version |release| warns when the policy is not set and uses
+``OLD`` behavior.  Use the :command:`cmake_policy` command to set
+it to ``OLD`` or ``NEW`` explicitly.
+
+.. include:: DEPRECATED.txt

+ 5 - 0
Help/release/dev/return-PROPAGATE.rst

@@ -0,0 +1,5 @@
+return-PROPAGATE
+----------------
+
+* The :command:`return` command gains the capability to propagate variables to
+  the include directory of function caller scope. See policy :policy:`CMP0140`.

+ 6 - 9
Source/cmBlockCommand.cxx

@@ -73,6 +73,7 @@ public:
 
 
 private:
 private:
   cmMakefile* Makefile;
   cmMakefile* Makefile;
+  ScopeSet Scopes;
   BlockScopePushPop BlockScope;
   BlockScopePushPop BlockScope;
   std::vector<std::string> VariableNames;
   std::vector<std::string> VariableNames;
 };
 };
@@ -81,6 +82,7 @@ cmBlockFunctionBlocker::cmBlockFunctionBlocker(
   cmMakefile* const mf, const ScopeSet& scopes,
   cmMakefile* const mf, const ScopeSet& scopes,
   std::vector<std::string> variableNames)
   std::vector<std::string> variableNames)
   : Makefile{ mf }
   : Makefile{ mf }
+  , Scopes{ scopes }
   , BlockScope{ mf, scopes }
   , BlockScope{ mf, scopes }
   , VariableNames{ std::move(variableNames) }
   , VariableNames{ std::move(variableNames) }
 {
 {
@@ -88,14 +90,8 @@ cmBlockFunctionBlocker::cmBlockFunctionBlocker(
 
 
 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);
     cmExecutionStatus status(mf);
     mf.ExecuteCommand(fn, status);
     mf.ExecuteCommand(fn, status);
     if (status.GetReturnInvoked()) {
     if (status.GetReturnInvoked()) {
-      inStatus.SetReturnInvoked();
+      mf.RaiseScope(status.GetReturnVariables());
+      inStatus.SetReturnInvoked(status.GetReturnVariables());
       return true;
       return true;
     }
     }
     if (status.GetBreakInvoked()) {
     if (status.GetBreakInvoked()) {

+ 16 - 1
Source/cmExecutionStatus.h

@@ -5,6 +5,7 @@
 #include <cmConfigure.h> // IWYU pragma: keep
 #include <cmConfigure.h> // IWYU pragma: keep
 
 
 #include <string>
 #include <string>
+#include <vector>
 
 
 class cmMakefile;
 class cmMakefile;
 
 
@@ -27,8 +28,21 @@ public:
   void SetError(std::string const& e) { this->Error = e; }
   void SetError(std::string const& e) { this->Error = e; }
   std::string const& GetError() const { return this->Error; }
   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; }
   bool GetReturnInvoked() const { return this->ReturnInvoked; }
+  const std::vector<std::string>& GetReturnVariables() const
+  {
+    return this->Variables;
+  }
 
 
   void SetBreakInvoked() { this->BreakInvoked = true; }
   void SetBreakInvoked() { this->BreakInvoked = true; }
   bool GetBreakInvoked() const { return this->BreakInvoked; }
   bool GetBreakInvoked() const { return this->BreakInvoked; }
@@ -46,4 +60,5 @@ private:
   bool BreakInvoked = false;
   bool BreakInvoked = false;
   bool ContinueInvoked = false;
   bool ContinueInvoked = false;
   bool NestedError = false;
   bool NestedError = false;
+  std::vector<std::string> Variables;
 };
 };

+ 1 - 1
Source/cmForEachCommand.cxx

@@ -260,7 +260,7 @@ auto cmForEachFunctionBlocker::invoke(
     cmExecutionStatus status(mf);
     cmExecutionStatus status(mf);
     mf.ExecuteCommand(func, status);
     mf.ExecuteCommand(func, status);
     if (status.GetReturnInvoked()) {
     if (status.GetReturnInvoked()) {
-      inStatus.SetReturnInvoked();
+      inStatus.SetReturnInvoked(status.GetReturnVariables());
       result.Break = true;
       result.Break = true;
       break;
       break;
     }
     }

+ 1 - 0
Source/cmFunctionCommand.cxx

@@ -120,6 +120,7 @@ bool cmFunctionHelperCommand::operator()(
       return false;
       return false;
     }
     }
     if (status.GetReturnInvoked()) {
     if (status.GetReturnInvoked()) {
+      makefile.RaiseScope(status.GetReturnVariables());
       break;
       break;
     }
     }
   }
   }

+ 1 - 1
Source/cmIfCommand.cxx

@@ -150,7 +150,7 @@ bool cmIfFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
       cmExecutionStatus status(mf);
       cmExecutionStatus status(mf);
       mf.ExecuteCommand(func, status);
       mf.ExecuteCommand(func, status);
       if (status.GetReturnInvoked()) {
       if (status.GetReturnInvoked()) {
-        inStatus.SetReturnInvoked();
+        inStatus.SetReturnInvoked(status.GetReturnVariables());
         return true;
         return true;
       }
       }
       if (status.GetBreakInvoked()) {
       if (status.GetBreakInvoked()) {

+ 1 - 1
Source/cmMacroCommand.cxx

@@ -126,7 +126,7 @@ bool cmMacroHelperCommand::operator()(
       return false;
       return false;
     }
     }
     if (status.GetReturnInvoked()) {
     if (status.GetReturnInvoked()) {
-      inStatus.SetReturnInvoked();
+      inStatus.SetReturnInvoked(status.GetReturnVariables());
       return true;
       return true;
     }
     }
     if (status.GetBreakInvoked()) {
     if (status.GetBreakInvoked()) {

+ 1 - 0
Source/cmMakefile.cxx

@@ -789,6 +789,7 @@ void cmMakefile::RunListFile(cmListFile const& listFile,
       break;
       break;
     }
     }
     if (status.GetReturnInvoked()) {
     if (status.GetReturnInvoked()) {
+      this->RaiseScope(status.GetReturnVariables());
       // Exit early due to return command.
       // Exit early due to return command.
       break;
       break;
     }
     }

+ 3 - 1
Source/cmPolicies.h

@@ -421,7 +421,9 @@ class cmMakefile;
   SELECT(                                                                     \
   SELECT(                                                                     \
     POLICY, CMP0139,                                                          \
     POLICY, CMP0139,                                                          \
     "The if() command supports path comparisons using PATH_EQUAL operator.",  \
     "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_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
 #define CM_FOR_EACH_POLICY_ID(POLICY)                                         \
 #define CM_FOR_EACH_POLICY_ID(POLICY)                                         \

+ 42 - 2
Source/cmReturnCommand.cxx

@@ -2,12 +2,52 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmReturnCommand.h"
 #include "cmReturnCommand.h"
 
 
+#include <cm/string_view>
+#include <cmext/string_view>
+
 #include "cmExecutionStatus.h"
 #include "cmExecutionStatus.h"
+#include "cmMakefile.h"
+#include "cmMessageType.h"
+#include "cmPolicies.h"
+#include "cmStringAlgorithms.h"
+#include "cmSystemTools.h"
 
 
 // cmReturnCommand
 // cmReturnCommand
-bool cmReturnCommand(std::vector<std::string> const&,
+bool cmReturnCommand(std::vector<std::string> const& args,
                      cmExecutionStatus& status)
                      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;
   return true;
 }
 }

+ 1 - 1
Source/cmWhileCommand.cxx

@@ -96,7 +96,7 @@ bool cmWhileFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
       cmExecutionStatus status(mf);
       cmExecutionStatus status(mf);
       mf.ExecuteCommand(fn, status);
       mf.ExecuteCommand(fn, status);
       if (status.GetReturnInvoked()) {
       if (status.GetReturnInvoked()) {
-        inStatus.SetReturnInvoked();
+        inStatus.SetReturnInvoked(status.GetReturnVariables());
         return true;
         return true;
       }
       }
       if (status.GetBreakInvoked()) {
       if (status.GetBreakInvoked()) {

+ 13 - 0
Tests/RunCMake/return/CMP0140-NEW.cmake

@@ -0,0 +1,13 @@
+
+cmake_policy(SET CMP0140 NEW)
+
+function(FUNC)
+  set(VAR "set")
+  return(PROPAGATE VAR)
+endfunction()
+
+set(VAR "initial")
+func()
+if (NOT DEFINED VAR OR NOT VAR  STREQUAL "set")
+  message(FATAL_ERROR "return(PROPAGATE) not handled correctly.")
+endif()

+ 8 - 0
Tests/RunCMake/return/CMP0140-OLD.cmake

@@ -0,0 +1,8 @@
+
+cmake_policy(SET CMP0140 OLD)
+
+function(FUNC)
+  return(PROPAGATE VAR)
+endfunction()
+
+func()

+ 1 - 0
Tests/RunCMake/return/CMP0140-WARN-result.txt

@@ -0,0 +1 @@
+0

+ 12 - 0
Tests/RunCMake/return/CMP0140-WARN-stderr.txt

@@ -0,0 +1,12 @@
+CMake Warning \(dev\) at CMP0140-WARN.cmake:[0-9]+ \(return\):
+  Policy CMP0140 is not set: The return\(\) command checks its arguments.  Run
+  "cmake --help-policy CMP0140" for policy details.  Use the cmake_policy
+  command to set the policy and suppress this warning.
+
+  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.
+Call Stack \(most recent call first\):
+  CMP0140-WARN.cmake:[0-9]+ \(func\)
+  CMakeLists.txt:[0-9]+ \(include\)
+This warning is for project developers.  Use -Wno-dev to suppress it.

+ 8 - 0
Tests/RunCMake/return/CMP0140-WARN.cmake

@@ -0,0 +1,8 @@
+
+cmake_policy(VERSION 3.1)
+
+function(FUNC)
+  return(PROPAGATE VAR)
+endfunction()
+
+func()

+ 1 - 1
Tests/RunCMake/return/CMakeLists.txt

@@ -1,3 +1,3 @@
-cmake_minimum_required(VERSION 3.1)
+cmake_minimum_required(VERSION 3.1...3.25)
 project(${RunCMake_TEST} NONE)
 project(${RunCMake_TEST} NONE)
 include(${RunCMake_TEST}.cmake)
 include(${RunCMake_TEST}.cmake)

+ 10 - 0
Tests/RunCMake/return/PropagateFromDirectory.cmake

@@ -0,0 +1,10 @@
+
+set(VAR1 "initial")
+set(VAR2 "initial")
+
+add_subdirectory(subdir)
+
+if((NOT DEFINED VAR1 OR NOT VAR1 STREQUAL "set")
+    OR DEFINED VAR2)
+  message(SEND_ERROR "erroneous propagation for FUNC1")
+endif()

+ 142 - 0
Tests/RunCMake/return/PropagateFromFunction.cmake

@@ -0,0 +1,142 @@
+
+function(FUNC1)
+  set(VAR1 "set")
+  unset(VAR2)
+  return(PROPAGATE VAR1 VAR2)
+endfunction()
+
+set(VAR1 "initial")
+set(VAR2 "initial")
+func1()
+if((NOT DEFINED VAR1 OR NOT VAR1 STREQUAL "set")
+    OR DEFINED VAR2)
+  message(SEND_ERROR "erroneous propagation for FUNC1")
+endif()
+
+
+function(FUNC2)
+  block()
+    set(VAR1 "set")
+    unset(VAR2)
+    return(PROPAGATE VAR1 VAR2)
+  endblock()
+endfunction()
+
+set(VAR1 "initial")
+set(VAR2 "initial")
+func2()
+if((NOT DEFINED VAR1 OR NOT VAR1 STREQUAL "set")
+    OR DEFINED VAR2)
+  message(SEND_ERROR "erroneous propagation for FUNC2")
+endif()
+
+
+function(FUNC3)
+  block(SCOPE_FOR POLICIES)
+    set(VAR1 "set")
+    unset(VAR2)
+    return(PROPAGATE VAR1 VAR2)
+  endblock()
+endfunction()
+
+set(VAR1 "initial")
+set(VAR2 "initial")
+func3()
+if((NOT DEFINED VAR1 OR NOT VAR1 STREQUAL "set")
+    OR DEFINED VAR2)
+  message(SEND_ERROR "erroneous propagation for FUNC3")
+endif()
+
+
+function(FUNC4)
+  while(TRUE)
+    set(VAR1 "set")
+    unset(VAR2)
+    return(PROPAGATE VAR1 VAR2)
+  endwhile()
+endfunction()
+
+set(VAR1 "initial")
+set(VAR2 "initial")
+func4()
+if((NOT DEFINED VAR1 OR NOT VAR1 STREQUAL "set")
+    OR DEFINED VAR2)
+  message(SEND_ERROR "erroneous propagation for FUNC4")
+endif()
+
+
+function(FUNC5)
+  foreach(item IN ITEMS A B)
+    set(VAR1 "set")
+    unset(VAR2)
+    return(PROPAGATE VAR1 VAR2)
+  endforeach()
+endfunction()
+
+set(VAR1 "initial")
+set(VAR2 "initial")
+func5()
+if((NOT DEFINED VAR1 OR NOT VAR1 STREQUAL "set")
+    OR DEFINED VAR2)
+  message(SEND_ERROR "erroneous propagation for FUNC5")
+endif()
+
+
+function(FUNC6)
+  if(TRUE)
+    set(VAR1 "set")
+    unset(VAR2)
+    return(PROPAGATE VAR1 VAR2)
+  endif()
+endfunction()
+
+set(VAR1 "initial")
+set(VAR2 "initial")
+func6()
+if((NOT DEFINED VAR1 OR NOT VAR1 STREQUAL "set")
+    OR DEFINED VAR2)
+  message(SEND_ERROR "erroneous propagation for FUNC6")
+endif()
+
+
+function(FUNC7)
+  if(FALSE)
+  else()
+    set(VAR1 "set")
+    unset(VAR2)
+    return(PROPAGATE VAR1 VAR2)
+  endif()
+endfunction()
+
+set(VAR1 "initial")
+set(VAR2 "initial")
+func7()
+if((NOT DEFINED VAR1 OR NOT VAR1 STREQUAL "set")
+    OR DEFINED VAR2)
+  message(SEND_ERROR "erroneous propagation for FUNC7")
+endif()
+
+
+set(VAR1 "initial")
+set(VAR2 "initial")
+cmake_language(CALL func7)
+if((NOT DEFINED VAR1 OR NOT VAR1 STREQUAL "set")
+    OR DEFINED VAR2)
+  message(SEND_ERROR "erroneous propagation for cmake_language(CALL FUNC7)")
+endif()
+
+
+set(VAR1 "initial")
+set(VAR2 "initial")
+cmake_language(EVAL CODE "
+  function(FUNC8)
+    set(VAR1 \"set\")
+    unset(VAR2)
+    return(PROPAGATE VAR1 VAR2)
+  endfunction()
+
+  func8()")
+if((NOT DEFINED VAR1 OR NOT VAR1 STREQUAL "set")
+    OR DEFINED VAR2)
+  message(SEND_ERROR "erroneous propagation for cmake_language(EVAL CODE)")
+endif()

+ 2 - 0
Tests/RunCMake/return/PropagateNothing.cmake

@@ -0,0 +1,2 @@
+
+return(PROPAGATE)

+ 9 - 0
Tests/RunCMake/return/RunCMakeTest.cmake

@@ -1,3 +1,12 @@
 include(RunCMake)
 include(RunCMake)
 
 
 run_cmake(ReturnFromForeach)
 run_cmake(ReturnFromForeach)
+
+run_cmake(WrongArgument)
+run_cmake(PropagateNothing)
+run_cmake(PropagateFromFunction)
+run_cmake(PropagateFromDirectory)
+
+run_cmake(CMP0140-NEW)
+run_cmake(CMP0140-OLD)
+run_cmake(CMP0140-WARN)

+ 1 - 0
Tests/RunCMake/return/WrongArgument-result.txt

@@ -0,0 +1 @@
+1

+ 4 - 0
Tests/RunCMake/return/WrongArgument-stderr.txt

@@ -0,0 +1,4 @@
+CMake Error at WrongArgument.cmake:[0-9]+ \(return\):
+  return called with unsupported argument "WRONG_ARG"
+Call Stack \(most recent call first\):
+  CMakeLists.txt:[0-9]+ \(include\)

+ 2 - 0
Tests/RunCMake/return/WrongArgument.cmake

@@ -0,0 +1,2 @@
+
+return(WRONG_ARG)

+ 5 - 0
Tests/RunCMake/return/subdir/CMakeLists.txt

@@ -0,0 +1,5 @@
+
+set(VAR1 "set")
+unset(VAR2)
+
+return(PROPAGATE VAR1 VAR2)