From 46896d98bb8ed8a1068da70220581f8bbc3847fc Mon Sep 17 00:00:00 2001 From: Marc Chevrier Date: Sun, 25 Apr 2021 14:22:09 +0200 Subject: foreach(): loop variables are only available in the loop scope Fixes: #20553 --- Help/command/foreach.rst | 5 +- Help/manual/cmake-policies.7.rst | 1 + Help/policy/CMP0124.rst | 20 ++++++++ Help/release/dev/foreach-variable-scope.rst | 5 ++ Source/cmForEachCommand.cxx | 37 +++++++++++---- Source/cmMakefile.cxx | 14 ++++++ Source/cmMakefile.h | 1 + Source/cmPolicies.h | 5 +- Tests/RunCMake/foreach/RunCMakeTest.cmake | 2 + .../foreach/foreach-var-scope-CMP0124-NEW.cmake | 51 +++++++++++++++++++++ .../foreach/foreach-var-scope-CMP0124-OLD.cmake | 53 ++++++++++++++++++++++ 11 files changed, 182 insertions(+), 12 deletions(-) create mode 100644 Help/policy/CMP0124.rst create mode 100644 Help/release/dev/foreach-variable-scope.rst create mode 100644 Tests/RunCMake/foreach/foreach-var-scope-CMP0124-NEW.cmake create mode 100644 Tests/RunCMake/foreach/foreach-var-scope-CMP0124-OLD.cmake diff --git a/Help/command/foreach.rst b/Help/command/foreach.rst index 8de6deb..d9f54ca 100644 --- a/Help/command/foreach.rst +++ b/Help/command/foreach.rst @@ -14,9 +14,12 @@ semicolon or whitespace. All commands between ``foreach`` and the matching ``endforeach`` are recorded without being invoked. Once the ``endforeach`` is evaluated, the recorded list of commands is invoked once for each item in ````. -At the beginning of each iteration the variable ``loop_var`` will be set +At the beginning of each iteration the variable ```` will be set to the value of the current item. +The scope of ```` is restricted to the loop scope. See policy +:policy:`CMP0124` for details. + The commands :command:`break` and :command:`continue` provide means to escape from the normal control flow. diff --git a/Help/manual/cmake-policies.7.rst b/Help/manual/cmake-policies.7.rst index b41ce59..08449a7 100644 --- a/Help/manual/cmake-policies.7.rst +++ b/Help/manual/cmake-policies.7.rst @@ -57,6 +57,7 @@ Policies Introduced by CMake 3.21 .. toctree:: :maxdepth: 1 + CMP0124: foreach() loop variables are only available in the loop scope. CMP0123: ARMClang cpu/arch compile and link flags must be set explicitly. CMP0122: UseSWIG use standard library name conventions for csharp language. CMP0121: The list command detects invalid indicies. diff --git a/Help/policy/CMP0124.rst b/Help/policy/CMP0124.rst new file mode 100644 index 0000000..88d03e3 --- /dev/null +++ b/Help/policy/CMP0124.rst @@ -0,0 +1,20 @@ +CMP0124 +------- + +.. versionadded:: 3.21 + +The loop variables created by :command:`foreach` command have now their scope +restricted to the loop scope. + +Starting with CMake 3.21, the :command:`foreach` command ensures that the loop +variables have their scope restricted to the loop scope. + +The ``OLD`` behavior for this policy let the loop variables to exist, with an +empty value, in the outer scope of loop scope. + +This policy was introduced in CMake version 3.21. Use the +:command:`cmake_policy` command to set it to ``OLD`` or ``NEW`` explicitly. +Unlike many policies, CMake version |release| does *not* warn when the policy +is not set and simply uses ``OLD`` behavior. + +.. include:: DEPRECATED.txt diff --git a/Help/release/dev/foreach-variable-scope.rst b/Help/release/dev/foreach-variable-scope.rst new file mode 100644 index 0000000..29a57bb --- /dev/null +++ b/Help/release/dev/foreach-variable-scope.rst @@ -0,0 +1,5 @@ +foreach-variable-scope +---------------------- + +* The :command:`foreach` command restrict loop variables to the loop scope. + See policy :policy:`CMP0124` for details. diff --git a/Source/cmForEachCommand.cxx b/Source/cmForEachCommand.cxx index bcacb15..4845a6d 100644 --- a/Source/cmForEachCommand.cxx +++ b/Source/cmForEachCommand.cxx @@ -17,6 +17,7 @@ #include #include +#include #include #include @@ -25,7 +26,7 @@ #include "cmListFileCache.h" #include "cmMakefile.h" #include "cmMessageType.h" -#include "cmProperty.h" +#include "cmPolicies.h" #include "cmRange.h" #include "cmStringAlgorithms.h" #include "cmSystemTools.h" @@ -113,9 +114,11 @@ bool cmForEachFunctionBlocker::ReplayItems( // At end of for each execute recorded commands // store the old value - std::string oldDef; - if (cmProp d = mf.GetDefinition(this->Args.front())) { - oldDef = *d; + cm::optional oldDef; + if (mf.GetPolicyStatus(cmPolicies::CMP0124) != cmPolicies::NEW) { + oldDef = mf.GetSafeDefinition(this->Args.front()); + } else if (mf.IsNormalDefinitionSet(this->Args.front())) { + oldDef = *mf.GetDefinition(this->Args.front()); } auto restore = false; @@ -131,9 +134,14 @@ bool cmForEachFunctionBlocker::ReplayItems( } if (restore) { - // restore the variable to its prior value - mf.AddDefinition(this->Args.front(), oldDef); + if (oldDef) { + // restore the variable to its prior value + mf.AddDefinition(this->Args.front(), *oldDef); + } else { + mf.RemoveDefinition(this->Args.front()); + } } + return true; } @@ -185,10 +193,15 @@ bool cmForEachFunctionBlocker::ReplayZipLists( assert("Sanity check" && iterationVars.size() == values.size()); // Store old values for iteration variables - std::map oldDefs; + std::map> oldDefs; for (auto i = 0u; i < values.size(); ++i) { - if (cmProp d = mf.GetDefinition(iterationVars[i])) { - oldDefs.emplace(iterationVars[i], *d); + const auto& varName = iterationVars[i]; + if (mf.GetPolicyStatus(cmPolicies::CMP0124) != cmPolicies::NEW) { + oldDefs.emplace(varName, mf.GetSafeDefinition(varName)); + } else if (mf.IsNormalDefinitionSet(varName)) { + oldDefs.emplace(varName, *mf.GetDefinition(varName)); + } else { + oldDefs.emplace(varName, cm::nullopt); } } @@ -226,7 +239,11 @@ bool cmForEachFunctionBlocker::ReplayZipLists( // Restore the variables to its prior value if (restore) { for (auto const& p : oldDefs) { - mf.AddDefinition(p.first, p.second); + if (p.second) { + mf.AddDefinition(p.first, *p.second); + } else { + mf.RemoveDefinition(p.first); + } } } return true; diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 40a67a3..dba8560 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -2507,6 +2507,20 @@ bool cmMakefile::IsDefinitionSet(const std::string& name) const return def != nullptr; } +bool cmMakefile::IsNormalDefinitionSet(const std::string& name) const +{ + cmProp def = this->StateSnapshot.GetDefinition(name); +#ifndef CMAKE_BOOTSTRAP + if (cmVariableWatch* vv = this->GetVariableWatch()) { + if (!def) { + vv->VariableAccessed( + name, cmVariableWatch::UNKNOWN_VARIABLE_DEFINED_ACCESS, nullptr, this); + } + } +#endif + return def != nullptr; +} + cmProp cmMakefile::GetDefinition(const std::string& name) const { cmProp def = this->StateSnapshot.GetDefinition(name); diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 71d765c..3c07808 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -486,6 +486,7 @@ public: const std::string& GetSafeDefinition(const std::string&) const; const std::string& GetRequiredDefinition(const std::string& name) const; bool IsDefinitionSet(const std::string&) const; + bool IsNormalDefinitionSet(const std::string&) const; bool GetDefExpandList(const std::string& name, std::vector& out, bool emptyArgs = false) const; /** diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h index 0a5bb4b..0ff12d5 100644 --- a/Source/cmPolicies.h +++ b/Source/cmPolicies.h @@ -369,7 +369,10 @@ class cmMakefile; 21, 0, cmPolicies::WARN) \ SELECT(POLICY, CMP0123, \ "ARMClang cpu/arch compile and link flags must be set explicitly.", \ - 3, 21, 0, cmPolicies::WARN) + 3, 21, 0, cmPolicies::WARN) \ + SELECT(POLICY, CMP0124, \ + "foreach() loop variables are only available in the loop scope.", 3, \ + 21, 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/Tests/RunCMake/foreach/RunCMakeTest.cmake b/Tests/RunCMake/foreach/RunCMakeTest.cmake index d3f7c23..15ca477 100644 --- a/Tests/RunCMake/foreach/RunCMakeTest.cmake +++ b/Tests/RunCMake/foreach/RunCMakeTest.cmake @@ -20,3 +20,5 @@ run_cmake(foreach-RANGE-non-int-test-3-2) run_cmake(foreach-RANGE-non-int-test-3-3) run_cmake(foreach-RANGE-invalid-test) run_cmake(foreach-RANGE-out-of-range-test) +run_cmake(foreach-var-scope-CMP0124-OLD) +run_cmake(foreach-var-scope-CMP0124-NEW) diff --git a/Tests/RunCMake/foreach/foreach-var-scope-CMP0124-NEW.cmake b/Tests/RunCMake/foreach/foreach-var-scope-CMP0124-NEW.cmake new file mode 100644 index 0000000..7e2eee0 --- /dev/null +++ b/Tests/RunCMake/foreach/foreach-var-scope-CMP0124-NEW.cmake @@ -0,0 +1,51 @@ + +cmake_policy(SET CMP0124 NEW) + +foreach(VAR a b c) +endforeach() +if (DEFINED VAR) + message(SEND_ERROR "Variable 'VAR' unexpectedly defined.") +endif() + +set(LIST1 a b c) +set(LIST2 x y z) +foreach(VAR1_1 VAR1_2 IN ZIP_LISTS LIST1 LIST2) +endforeach() +if (DEFINED VAR1_1 OR DEFINED VAR1_2) + message(SEND_ERROR "Variables 'VAR1_1' or 'VAR1_2' unexpectedly defined.") +endif() + + +set (VAR2 OLD) +foreach(VAR2 a b c) +endforeach() +if (NOT DEFINED VAR2 OR NOT VAR2 STREQUAL "OLD") + message(SEND_ERROR "Variable 'VAR2' not defined or wrong value.") +endif() + +set (VAR2_2 OLD) +foreach(VAR2_1 VAR2_2 IN ZIP_LISTS LIST1 LIST2) +endforeach() +if (DEFINED VAR2_1 OR NOT DEFINED VAR2_2) + message(SEND_ERROR "Variable 'VAR2_1' unexpectedly defined or variable 'VAR2_2' not defined.") +endif() + + +set (VAR3 OLD CACHE STRING "") +foreach(VAR3 a b c) +endforeach() +# check that only cache variable is defined +set(OLD_VALUE "${VAR3}") +unset(VAR3 CACHE) +if (DEFINED VAR3 OR NOT OLD_VALUE STREQUAL "OLD") + message(SEND_ERROR "Variable 'VAR3' wrongly defined or wrong value.") +endif() + +set (VAR3_2 OLD CACHE STRING "") +foreach(VAR3_1 VAR3_2 IN ZIP_LISTS LIST1 LIST2) +endforeach() +set(OLD_VALUE "${VAR3_2}") +unset(VAR3_2 CACHE) +if (DEFINED VAR3_1 OR DEFINED VAR3_2 OR NOT OLD_VALUE STREQUAL "OLD") + message(SEND_ERROR "Variable 'VAR3_1' unexpectedly defined or variable 'VAR2_2' wrongly defined or wrong value.") +endif() diff --git a/Tests/RunCMake/foreach/foreach-var-scope-CMP0124-OLD.cmake b/Tests/RunCMake/foreach/foreach-var-scope-CMP0124-OLD.cmake new file mode 100644 index 0000000..f955982 --- /dev/null +++ b/Tests/RunCMake/foreach/foreach-var-scope-CMP0124-OLD.cmake @@ -0,0 +1,53 @@ + +cmake_policy(SET CMP0124 OLD) + +foreach(VAR a b c) +endforeach() +if (NOT DEFINED VAR OR NOT VAR STREQUAL "") + message(SEND_ERROR "Variable 'VAR' not defined or wrong value.") +endif() + +set(LIST1 a b c) +set(LIST2 x y z) +foreach(VAR1_1 VAR1_2 IN ZIP_LISTS LIST1 LIST2) +endforeach() +if (NOT DEFINED VAR1_1 OR NOT VAR1_1 STREQUAL "" + OR NOT DEFINED VAR1_2 OR NOT VAR1_2 STREQUAL "") + message(SEND_ERROR "Variables 'VAR1_1' or 'VAR1_2' not defined or wrong value.") +endif() + + +set (VAR2 OLD) +foreach(VAR2 a b c) +endforeach() +if (NOT DEFINED VAR2 OR NOT VAR2 STREQUAL "OLD") + message(SEND_ERROR "Variable 'VAR2' not defined or wrong value.") +endif() + +set (VAR2_2 OLD) +foreach(VAR2_1 VAR2_2 IN ZIP_LISTS LIST1 LIST2) +endforeach() +if (NOT DEFINED VAR2_1 OR NOT VAR2_1 STREQUAL "" + OR NOT DEFINED VAR2_2 OR NOT VAR2_2 STREQUAL "OLD") + message(SEND_ERROR "Variables 'VAR2_1' or 'VAR2_2' not defined or wrong value.") +endif() + + +set (VAR3 OLD CACHE STRING "") +foreach(VAR3 a b c) +endforeach() +# a normal variable is defined, holding cache variable value +unset(VAR3 CACHE) +if (NOT DEFINED VAR3 OR NOT VAR3 STREQUAL "OLD") + message(SEND_ERROR "Variable 'VAR3' not defined or wrong value.") +endif() + +set (VAR3_2 OLD CACHE STRING "") +foreach(VAR3_1 VAR3_2 IN ZIP_LISTS LIST1 LIST2) +endforeach() +# a normal variable is defined, holding cache variable value +unset(VAR3_2 CACHE) +if (NOT DEFINED VAR3_1 OR NOT VAR3_1 STREQUAL "" + OR NOT DEFINED VAR3_2 OR NOT VAR3_2 STREQUAL "OLD") + message(SEND_ERROR "Variables 'VAR3_1' or 'VAR3_2' not defined or wrong value.") +endif() -- cgit v0.12