Merge topic 'refactor_cmSetPropertiesCommands'

300bf4e94f set_*_properties: simplify and shorten implementations

Acked-by: Kitware Robot <[email protected]>
Merge-request: !4728

Source/cmSetDirectoryPropertiesCommand.cxx

@@ -5,12 +5,6 @@
 #include "cmExecutionStatus.h"
 #include "cmMakefile.h"
 
-namespace {
-bool RunCommand(cmMakefile& mf, std::vector<std::string>::const_iterator ait,
-                std::vector<std::string>::const_iterator aitend,
-                std::string& errors);
-}
-
 // cmSetDirectoryPropertiesCommand
 bool cmSetDirectoryPropertiesCommand(std::vector<std::string> const& args,
                                      cmExecutionStatus& status)
@@ -20,38 +14,26 @@ bool cmSetDirectoryPropertiesCommand(std::vector<std::string> const& args,
     return false;
   }
 
-  std::string errors;
-  bool ret =
-    RunCommand(status.GetMakefile(), args.begin() + 1, args.end(), errors);
-  if (!ret) {
-    status.SetError(errors);
+  // PROPERTIES followed by prop value pairs
+  if (args.size() % 2 != 1) {
+    status.SetError("Wrong number of arguments");
+    return false;
   }
-  return ret;
-}
 
-namespace {
-bool RunCommand(cmMakefile& mf, std::vector<std::string>::const_iterator ait,
-                std::vector<std::string>::const_iterator aitend,
-                std::string& errors)
-{
-  for (; ait != aitend; ait += 2) {
-    if (ait + 1 == aitend) {
-      errors = "Wrong number of arguments";
-      return false;
-    }
-    const std::string& prop = *ait;
-    const std::string& value = *(ait + 1);
+  for (auto iter = args.begin() + 1; iter != args.end(); iter += 2) {
+    const std::string& prop = *iter;
     if (prop == "VARIABLES") {
-      errors = "Variables and cache variables should be set using SET command";
+      status.SetError(
+        "Variables and cache variables should be set using SET command");
       return false;
     }
     if (prop == "MACROS") {
-      errors = "Commands and macros cannot be set using SET_CMAKE_PROPERTIES";
+      status.SetError(
+        "Commands and macros cannot be set using SET_CMAKE_PROPERTIES");
       return false;
     }
-    mf.SetProperty(prop, value.c_str());
+    status.GetMakefile().SetProperty(prop, (iter + 1)->c_str());
   }
 
   return true;
 }
-}

Source/cmSetSourceFilesPropertiesCommand.cxx

@@ -2,18 +2,17 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmSetSourceFilesPropertiesCommand.h"
 
+#include <algorithm>
+#include <iterator>
+
+#include <cm/string_view>
+#include <cmext/algorithm>
+
 #include "cmExecutionStatus.h"
 #include "cmMakefile.h"
 #include "cmSourceFile.h"
 #include "cmStringAlgorithms.h"
 
-static bool RunCommand(cmMakefile* mf,
-                       std::vector<std::string>::const_iterator filebeg,
-                       std::vector<std::string>::const_iterator fileend,
-                       std::vector<std::string>::const_iterator propbeg,
-                       std::vector<std::string>::const_iterator propend,
-                       std::string& errors);
-
 bool cmSetSourceFilesPropertiesCommand(std::vector<std::string> const& args,
                                        cmExecutionStatus& status)
 {
@@ -23,108 +22,64 @@ bool cmSetSourceFilesPropertiesCommand(std::vector<std::string> const& args,
   }
 
   // break the arguments into source file names and properties
-  int numFiles = 0;
-  std::vector<std::string>::const_iterator j;
-  j = args.begin();
   // old style allows for specifier before PROPERTIES keyword
-  while (j != args.end() && *j != "ABSTRACT" && *j != "WRAP_EXCLUDE" &&
-         *j != "GENERATED" && *j != "COMPILE_FLAGS" &&
-         *j != "OBJECT_DEPENDS" && *j != "PROPERTIES") {
-    numFiles++;
-    ++j;
-  }
+  static const cm::string_view propNames[] = {
+    "ABSTRACT",      "GENERATED",      "WRAP_EXCLUDE",
+    "COMPILE_FLAGS", "OBJECT_DEPENDS", "PROPERTIES"
+  };
+  auto propsBegin = std::find_first_of(
+    args.begin(), args.end(), std::begin(propNames), std::end(propNames));
 
-  cmMakefile& mf = status.GetMakefile();
-
-  // now call the worker function
-  std::string errors;
-  bool ret = RunCommand(&mf, args.begin(), args.begin() + numFiles,
-                        args.begin() + numFiles, args.end(), errors);
-  if (!ret) {
-    status.SetError(errors);
-  }
-  return ret;
-}
-
-static bool RunCommand(cmMakefile* mf,
-                       std::vector<std::string>::const_iterator filebeg,
-                       std::vector<std::string>::const_iterator fileend,
-                       std::vector<std::string>::const_iterator propbeg,
-                       std::vector<std::string>::const_iterator propend,
-                       std::string& errors)
-{
   std::vector<std::string> propertyPairs;
-  bool generated = false;
-  std::vector<std::string>::const_iterator j;
   // build the property pairs
-  for (j = propbeg; j != propend; ++j) {
-    // old style allows for specifier before PROPERTIES keyword
-    if (*j == "ABSTRACT") {
-      propertyPairs.emplace_back("ABSTRACT");
-      propertyPairs.emplace_back("1");
-    } else if (*j == "WRAP_EXCLUDE") {
-      propertyPairs.emplace_back("WRAP_EXCLUDE");
-      propertyPairs.emplace_back("1");
-    } else if (*j == "GENERATED") {
-      generated = true;
-      propertyPairs.emplace_back("GENERATED");
+  for (auto j = propsBegin; j != args.end(); ++j) {
+    // consume old style options
+    if (*j == "ABSTRACT" || *j == "GENERATED" || *j == "WRAP_EXCLUDE") {
+      propertyPairs.emplace_back(*j);
       propertyPairs.emplace_back("1");
     } else if (*j == "COMPILE_FLAGS") {
       propertyPairs.emplace_back("COMPILE_FLAGS");
       ++j;
-      if (j == propend) {
-        errors = "called with incorrect number of arguments "
-                 "COMPILE_FLAGS with no flags";
+      if (j == args.end()) {
+        status.SetError("called with incorrect number of arguments "
+                        "COMPILE_FLAGS with no flags");
         return false;
       }
       propertyPairs.push_back(*j);
     } else if (*j == "OBJECT_DEPENDS") {
       propertyPairs.emplace_back("OBJECT_DEPENDS");
       ++j;
-      if (j == propend) {
-        errors = "called with incorrect number of arguments "
-                 "OBJECT_DEPENDS with no dependencies";
+      if (j == args.end()) {
+        status.SetError("called with incorrect number of arguments "
+                        "OBJECT_DEPENDS with no dependencies");
         return false;
       }
       propertyPairs.push_back(*j);
     } else if (*j == "PROPERTIES") {
-      // now loop through the rest of the arguments, new style
-      ++j;
-      while (j != propend) {
-        propertyPairs.push_back(*j);
-        if (*j == "GENERATED") {
-          ++j;
-          if (j != propend && cmIsOn(*j)) {
-            generated = true;
-          }
-        } else {
-          ++j;
-        }
-        if (j == propend) {
-          errors = "called with incorrect number of arguments.";
-          return false;
-        }
-        propertyPairs.push_back(*j);
-        ++j;
+      // PROPERTIES is followed by new style prop value pairs
+      cmStringRange newStyleProps{ j + 1, args.end() };
+      if (newStyleProps.size() % 2 != 0) {
+        status.SetError("called with incorrect number of arguments.");
+        return false;
       }
-      // break out of the loop because j is already == end
+      // set newStyleProps as is.
+      cm::append(propertyPairs, newStyleProps);
+      // break out of the loop.
       break;
     } else {
-      errors = "called with illegal arguments, maybe missing a "
-               "PROPERTIES specifier?";
+      status.SetError("called with illegal arguments, maybe missing a "
+                      "PROPERTIES specifier?");
       return false;
     }
   }
 
-  // now loop over all the files
-  for (j = filebeg; j != fileend; ++j) {
+  // loop over all the files
+  for (const std::string& sfname : cmStringRange{ args.begin(), propsBegin }) {
     // get the source file
-    cmSourceFile* sf = mf->GetOrCreateSource(*j, generated);
-    if (sf) {
-      // now loop through all the props and set them
-      unsigned int k;
-      for (k = 0; k < propertyPairs.size(); k = k + 2) {
-        sf->SetProperty(propertyPairs[k], propertyPairs[k + 1].c_str());
+    if (cmSourceFile* sf = status.GetMakefile().GetOrCreateSource(sfname)) {
+      // loop through the props and set them
+      for (auto k = propertyPairs.begin(); k != propertyPairs.end(); k += 2) {
+        sf->SetProperty(*k, (k + 1)->c_str());
       }
     }
   }

Source/cmSetTargetPropertiesCommand.cxx

@@ -2,19 +2,14 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmSetTargetPropertiesCommand.h"
 
+#include <algorithm>
 #include <iterator>
 
-#include <cmext/algorithm>
-
 #include "cmExecutionStatus.h"
 #include "cmMakefile.h"
 #include "cmStringAlgorithms.h"
 #include "cmTarget.h"
 
-static bool SetOneTarget(const std::string& tname,
-                         std::vector<std::string>& propertyPairs,
-                         cmMakefile* mf);
-
 bool cmSetTargetPropertiesCommand(std::vector<std::string> const& args,
                                   cmExecutionStatus& status)
 {
@@ -23,61 +18,38 @@ bool cmSetTargetPropertiesCommand(std::vector<std::string> const& args,
     return false;
   }
 
-  // first collect up the list of files
-  std::vector<std::string> propertyPairs;
-  int numFiles = 0;
-  for (auto j = args.begin(); j != args.end(); ++j) {
-    if (*j == "PROPERTIES") {
-      // now loop through the rest of the arguments, new style
-      ++j;
-      if (std::distance(j, args.end()) % 2 != 0) {
-        status.SetError("called with incorrect number of arguments.");
-        return false;
-      }
-      cm::append(propertyPairs, j, args.end());
-      break;
-    }
-    numFiles++;
+  // first identify the properties arguments
+  auto propsIter = std::find(args.begin(), args.end(), "PROPERTIES");
+  if (propsIter == args.end() || propsIter + 1 == args.end()) {
+    status.SetError("called with illegal arguments, maybe missing a "
+                    "PROPERTIES specifier?");
+    return false;
   }
-  if (propertyPairs.empty()) {
-    status.SetError("called with illegal arguments, maybe missing "
-                    "a PROPERTIES specifier?");
+
+  if (std::distance(propsIter, args.end()) % 2 != 1) {
+    status.SetError("called with incorrect number of arguments.");
     return false;
   }
 
   cmMakefile& mf = status.GetMakefile();
 
-  // now loop over all the targets
-  for (int i = 0; i < numFiles; ++i) {
-    if (mf.IsAlias(args[i])) {
+  // loop over all the targets
+  for (const std::string& tname : cmStringRange{ args.begin(), propsIter }) {
+    if (mf.IsAlias(tname)) {
       status.SetError("can not be used on an ALIAS target.");
       return false;
     }
-    bool ret = SetOneTarget(args[i], propertyPairs, &mf);
-    if (!ret) {
+    if (cmTarget* target = mf.FindTargetToUse(tname)) {
+      // loop through all the props and set them
+      for (auto k = propsIter + 1; k != args.end(); k += 2) {
+        target->SetProperty(*k, *(k + 1));
+        target->CheckProperty(*k, &mf);
+      }
+    } else {
       status.SetError(
-        cmStrCat("Can not find target to add properties to: ", args[i]));
+        cmStrCat("Can not find target to add properties to: ", tname));
       return false;
     }
   }
   return true;
 }
-
-static bool SetOneTarget(const std::string& tname,
-                         std::vector<std::string>& propertyPairs,
-                         cmMakefile* mf)
-{
-  if (cmTarget* target = mf->FindTargetToUse(tname)) {
-    // now loop through all the props and set them
-    unsigned int k;
-    for (k = 0; k < propertyPairs.size(); k = k + 2) {
-      target->SetProperty(propertyPairs[k], propertyPairs[k + 1]);
-      target->CheckProperty(propertyPairs[k], mf);
-    }
-  }
-  // if file is not already in the makefile, then add it
-  else {
-    return false;
-  }
-  return true;
-}

Source/cmSetTestsPropertiesCommand.cxx

@@ -2,19 +2,14 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmSetTestsPropertiesCommand.h"
 
+#include <algorithm>
 #include <iterator>
 
-#include <cmext/algorithm>
-
 #include "cmExecutionStatus.h"
 #include "cmMakefile.h"
 #include "cmStringAlgorithms.h"
 #include "cmTest.h"
 
-static bool SetOneTest(const std::string& tname,
-                       std::vector<std::string>& propertyPairs, cmMakefile* mf,
-                       std::string& errors);
-
 bool cmSetTestsPropertiesCommand(std::vector<std::string> const& args,
                                  cmExecutionStatus& status)
 {
@@ -23,61 +18,33 @@ bool cmSetTestsPropertiesCommand(std::vector<std::string> const& args,
     return false;
   }
 
-  cmMakefile& mf = status.GetMakefile();
-
-  // first collect up the list of files
-  std::vector<std::string> propertyPairs;
-  int numFiles = 0;
-  std::vector<std::string>::const_iterator j;
-  for (j = args.begin(); j != args.end(); ++j) {
-    if (*j == "PROPERTIES") {
-      // now loop through the rest of the arguments, new style
-      ++j;
-      if (std::distance(j, args.end()) % 2 != 0) {
-        status.SetError("called with incorrect number of arguments.");
-        return false;
-      }
-      cm::append(propertyPairs, j, args.end());
-      break;
-    }
-    numFiles++;
-  }
-  if (propertyPairs.empty()) {
-    status.SetError("called with illegal arguments, maybe "
-                    "missing a PROPERTIES specifier?");
+  // first identify the properties arguments
+  auto propsIter = std::find(args.begin(), args.end(), "PROPERTIES");
+  if (propsIter == args.end() || propsIter + 1 == args.end()) {
+    status.SetError("called with illegal arguments, maybe missing a "
+                    "PROPERTIES specifier?");
     return false;
   }
 
-  // now loop over all the targets
-  int i;
-  for (i = 0; i < numFiles; ++i) {
-    std::string errors;
-    bool ret = SetOneTest(args[i], propertyPairs, &mf, errors);
-    if (!ret) {
-      status.SetError(errors);
-      return ret;
-    }
+  if (std::distance(propsIter, args.end()) % 2 != 1) {
+    status.SetError("called with incorrect number of arguments.");
+    return false;
   }
 
-  return true;
-}
-
-static bool SetOneTest(const std::string& tname,
-                       std::vector<std::string>& propertyPairs, cmMakefile* mf,
-                       std::string& errors)
-{
-  if (cmTest* test = mf->GetTest(tname)) {
-    // now loop through all the props and set them
-    unsigned int k;
-    for (k = 0; k < propertyPairs.size(); k = k + 2) {
-      if (!propertyPairs[k].empty()) {
-        test->SetProperty(propertyPairs[k], propertyPairs[k + 1].c_str());
+  // loop over all the tests
+  for (const std::string& tname : cmStringRange{ args.begin(), propsIter }) {
+    if (cmTest* test = status.GetMakefile().GetTest(tname)) {
+      // loop through all the props and set them
+      for (auto k = propsIter + 1; k != args.end(); k += 2) {
+        if (!k->empty()) {
+          test->SetProperty(*k, (k + 1)->c_str());
+        }
       }
+    } else {
+      status.SetError(
+        cmStrCat("Can not find test to add properties to: ", tname));
+      return false;
     }
-  } else {
-    errors = cmStrCat("Can not find test to add properties to: ", tname);
-    return false;
   }
-
   return true;
 }