foreach: Allow multiple iteration variables for `ZIP_LIST` mode

1 files changed, 87 insertions, 20 deletions

diff --git a/Source/cmForEachCommand.cxx b/Source/cmForEachCommand.cxx
index 163bb9ab19..ac48287892 100644
--- a/Source/cmForEachCommand.cxx
+++ b/Source/cmForEachCommand.cxx
@@ -3,6 +3,7 @@
 #include "cmForEachCommand.h"
 
 #include <algorithm>
+#include <cassert>
 #include <cstddef>
 // NOTE The declaration of `std::abs` has moved to `cmath` since C++17
 // See https://en.cppreference.com/w/cpp/numeric/math/abs
@@ -43,6 +44,10 @@ public:
   bool Replay(std::vector<cmListFileFunction> functions,
               cmExecutionStatus& inStatus) override;
 
+  void SetIterationVarsCount(const std::size_t varsCount)
+  {
+    this->IterationVarsCount = varsCount;
+  }
   void SetZipLists() { this->ZipLists = true; }
 
   std::vector<std::string> Args;
@@ -64,6 +69,7 @@ private:
                       cmExecutionStatus& inStatus, cmMakefile& mf);
 
   cmMakefile* Makefile;
+  std::size_t IterationVarsCount = 0u;
   bool ZipLists = false;
 };
 
@@ -98,6 +104,9 @@ bool cmForEachFunctionBlocker::ReplayItems(
   std::vector<cmListFileFunction> const& functions,
   cmExecutionStatus& inStatus)
 {
+  assert("Unexpected number of iteration variables" &&
+         this->IterationVarsCount == 1);
+
   auto& mf = inStatus.GetMakefile();
 
   // At end of for each execute recorded commands
@@ -130,35 +139,55 @@ bool cmForEachFunctionBlocker::ReplayZipLists(
   std::vector<cmListFileFunction> const& functions,
   cmExecutionStatus& inStatus)
 {
+  assert("Unexpected number of iteration variables" &&
+         this->IterationVarsCount >= 1);
+
   auto& mf = inStatus.GetMakefile();
 
   // Expand the list of list-variables into a list of lists of strings
   std::vector<std::vector<std::string>> values;
-  values.reserve(this->Args.size() - 1);
+  values.reserve(this->Args.size() - this->IterationVarsCount);
   // Also track the longest list size
-  std::size_t max_items = 0u;
-  for (auto const& var : cmMakeRange(this->Args).advance(1)) {
+  std::size_t maxItems = 0u;
+  for (auto const& var :
+       cmMakeRange(this->Args).advance(this->IterationVarsCount)) {
     std::vector<std::string> items;
     auto const& value = mf.GetSafeDefinition(var);
     if (!value.empty()) {
       cmExpandList(value, items, true);
     }
-    max_items = std::max(max_items, items.size());
+    maxItems = std::max(maxItems, items.size());
     values.emplace_back(std::move(items));
   }
 
+  // Form the list of iteration variables
+  std::vector<std::string> iterationVars;
+  if (this->IterationVarsCount > 1) {
+    // If multiple iteration variables has given,
+    // just copy them to the `iterationVars` list.
+    iterationVars.reserve(values.size());
+    std::copy(this->Args.begin(),
+              this->Args.begin() + this->IterationVarsCount,
+              std::back_inserter(iterationVars));
+  } else {
+    // In case of the only iteration variable,
+    // generate names as `var_name_N`,
+    // where `N` is the count of lists to zip
+    iterationVars.resize(values.size());
+    const auto iter_var_prefix = this->Args.front() + "_";
+    auto i = 0u;
+    std::generate(
+      iterationVars.begin(), iterationVars.end(),
+      [&]() -> std::string { return iter_var_prefix + std::to_string(i++); });
+  }
+  assert("Sanity check" && iterationVars.size() == values.size());
+
   // Store old values for iteration variables
   std::map<std::string, std::string> oldDefs;
-  // Also, form the vector of iteration variable names
-  std::vector<std::string> iteration_vars;
-  iteration_vars.reserve(values.size());
-  const auto iter_var_prefix = this->Args.front();
   for (auto i = 0u; i < values.size(); ++i) {
-    auto iter_var_name = iter_var_prefix + "_" + std::to_string(i);
-    if (mf.GetDefinition(iter_var_name)) {
-      oldDefs.emplace(iter_var_name, mf.GetDefinition(iter_var_name));
+    if (mf.GetDefinition(iterationVars[i])) {
+      oldDefs.emplace(iterationVars[i], mf.GetDefinition(iterationVars[i]));
     }
-    iteration_vars.emplace_back(std::move(iter_var_name));
   }
 
   // Form a vector of current positions in all lists (Ok, vectors) of values
@@ -168,19 +197,20 @@ bool cmForEachFunctionBlocker::ReplayZipLists(
     values.begin(), values.end(), std::back_inserter(positions),
     // Set the initial position to the beginning of every list
     [](decltype(values)::value_type& list) { return list.begin(); });
+  assert("Sanity check" && positions.size() == values.size());
 
   auto restore = false;
   // Iterate over all the lists simulateneously
-  for (auto i = 0u; i < max_items; ++i) {
+  for (auto i = 0u; i < maxItems; ++i) {
     // Declare iteration variables
     for (auto j = 0u; j < values.size(); ++j) {
       // Define (or not) the iteration variable if the current position
       // still not at the end...
       if (positions[j] != values[j].end()) {
-        mf.AddDefinition(iteration_vars[j], *positions[j]);
+        mf.AddDefinition(iterationVars[j], *positions[j]);
         ++positions[j];
       } else {
-        mf.RemoveDefinition(iteration_vars[j]);
+        mf.RemoveDefinition(iterationVars[j]);
       }
     }
     // Invoke all the functions that were collected in the block.
@@ -230,10 +260,19 @@ auto cmForEachFunctionBlocker::invoke(
   return result;
 }
 
-bool HandleInMode(std::vector<std::string> const& args, cmMakefile& makefile)
+bool HandleInMode(std::vector<std::string> const& args,
+                  std::vector<std::string>::const_iterator kwInIter,
+                  cmMakefile& makefile)
 {
+  assert("A valid iterator expected" && kwInIter != args.end());
+
   auto fb = cm::make_unique<cmForEachFunctionBlocker>(&makefile);
-  fb->Args.push_back(args.front());
+
+  // Copy iteration variable names first
+  std::copy(args.begin(), kwInIter, std::back_inserter(fb->Args));
+  // Remember the count of given iteration variable names
+  const auto varsCount = fb->Args.size();
+  fb->SetIterationVarsCount(varsCount);
 
   enum Doing
   {
@@ -243,13 +282,20 @@ bool HandleInMode(std::vector<std::string> const& args, cmMakefile& makefile)
     DoingZipLists
   };
   Doing doing = DoingNone;
-  for (std::string const& arg : cmMakeRange(args).advance(2)) {
+  // Iterate over arguments past the "IN" keyword
+  for (std::string const& arg : cmMakeRange(++kwInIter, args.end())) {
     if (arg == "LISTS") {
       if (doing == DoingZipLists) {
         makefile.IssueMessage(MessageType::FATAL_ERROR,
                               "ZIP_LISTS can not be used with LISTS or ITEMS");
         return true;
       }
+      if (varsCount != 1u) {
+        makefile.IssueMessage(
+          MessageType::FATAL_ERROR,
+          "ITEMS or LISTS require exactly one iteration variable");
+        return true;
+      }
       doing = DoingLists;
 
     } else if (arg == "ITEMS") {
@@ -258,6 +304,12 @@ bool HandleInMode(std::vector<std::string> const& args, cmMakefile& makefile)
                               "ZIP_LISTS can not be used with LISTS or ITEMS");
         return true;
       }
+      if (varsCount != 1u) {
+        makefile.IssueMessage(
+          MessageType::FATAL_ERROR,
+          "ITEMS or LISTS require exactly one iteration variable");
+        return true;
+      }
       doing = DoingItems;
 
     } else if (arg == "ZIP_LISTS") {
@@ -285,6 +337,18 @@ bool HandleInMode(std::vector<std::string> const& args, cmMakefile& makefile)
     }
   }
 
+  // If `ZIP_LISTS` given and variables count more than 1,
+  // make sure the given lists count matches variables...
+  if (doing == DoingZipLists && varsCount > 1u &&
+      (2u * varsCount) != fb->Args.size()) {
+    makefile.IssueMessage(
+      MessageType::FATAL_ERROR,
+      cmStrCat("Expected ", std::to_string(varsCount),
+               " list variables, but given ",
+               std::to_string(fb->Args.size() - varsCount)));
+    return true;
+  }
+
   makefile.AddFunctionBlocker(std::move(fb));
 
   return true;
@@ -299,8 +363,9 @@ bool cmForEachCommand(std::vector<std::string> const& args,
     status.SetError("called with incorrect number of arguments");
     return false;
   }
-  if (args.size() > 1 && args[1] == "IN") {
-    return HandleInMode(args, status.GetMakefile());
+  auto kwInIter = std::find(args.begin(), args.end(), "IN");
+  if (kwInIter != args.end()) {
+    return HandleInMode(args, kwInIter, status.GetMakefile());
   }
 
   // create a function blocker
@@ -360,6 +425,8 @@ bool cmForEachCommand(std::vector<std::string> const& args,
   } else {
     fb->Args = args;
   }
+
+  fb->SetIterationVarsCount(1u);
   status.GetMakefile().AddFunctionBlocker(std::move(fb));
 
   return true;