Merge topic 'cmstrcat-move-first-arg'

beba50bd61 cmStrCat(): optimize when first argument is an rvalue string
d6f2a7ab4b cmStringCommand: remove use of cmCatViews()

Acked-by: Kitware Robot <kwrobot@kitware.com>
Tested-by: buildbot <buildbot@kitware.com>
Merge-request: !7969

4 files changed, 95 insertions, 44 deletions

diff --git a/Source/cmStringAlgorithms.cxx b/Source/cmStringAlgorithms.cxx
index f73c854..e559cfa 100644
--- a/Source/cmStringAlgorithms.cxx
+++ b/Source/cmStringAlgorithms.cxx
@@ -203,15 +203,23 @@ cmAlphaNum::cmAlphaNum(double val)
   MakeDigits(this->View_, this->Digits_, "%g", val);
 }
 
-std::string cmCatViews(std::initializer_list<cm::string_view> views)
+std::string cmCatViews(cm::optional<std::string>&& first,
+                       std::initializer_list<cm::string_view> views)
 {
-  std::size_t total_size = 0;
+  std::size_t totalSize = 0;
   for (cm::string_view const& view : views) {
-    total_size += view.size();
+    totalSize += view.size();
   }
 
-  std::string result(total_size, '\0');
-  std::string::iterator sit = result.begin();
+  std::string result;
+  std::string::size_type initialLen = 0;
+  if (first) {
+    totalSize += first->length();
+    initialLen = first->length();
+    result = std::move(*first);
+  }
+  result.resize(totalSize);
+  std::string::iterator sit = result.begin() + initialLen;
   for (cm::string_view const& view : views) {
     sit = std::copy_n(view.data(), view.size(), sit);
   }
diff --git a/Source/cmStringAlgorithms.h b/Source/cmStringAlgorithms.h
index 83938bc..bff2eda 100644
--- a/Source/cmStringAlgorithms.h
+++ b/Source/cmStringAlgorithms.h
@@ -12,6 +12,7 @@
 #include <utility>
 #include <vector>
 
+#include <cm/optional>
 #include <cm/string_view>
 
 #include "cmRange.h"
@@ -146,7 +147,8 @@ std::vector<std::string> cmExpandedLists(InputIt first, InputIt last)
 }
 
 /** Concatenate string pieces into a single string.  */
-std::string cmCatViews(std::initializer_list<cm::string_view> views);
+std::string cmCatViews(cm::optional<std::string>&& first,
+                       std::initializer_list<cm::string_view> views);
 
 /** Utility class for cmStrCat.  */
 class cmAlphaNum
@@ -189,13 +191,38 @@ private:
   char Digits_[32];
 };
 
+template <typename A, typename B, typename... AV>
+class cmStrCatHelper
+{
+public:
+  static std::string Compute(cmAlphaNum const& a, cmAlphaNum const& b,
+                             AV const&... args)
+  {
+    return cmCatViews(
+      cm::nullopt,
+      { a.View(), b.View(), static_cast<cmAlphaNum const&>(args).View()... });
+  }
+};
+
+template <typename B, typename... AV>
+class cmStrCatHelper<std::string, B, AV...>
+{
+public:
+  static std::string Compute(std::string&& a, cmAlphaNum const& b,
+                             AV const&... args)
+  {
+    return cmCatViews(
+      std::move(a),
+      { b.View(), static_cast<cmAlphaNum const&>(args).View()... });
+  }
+};
+
 /** Concatenate string pieces and numbers into a single string.  */
-template <typename... AV>
-inline std::string cmStrCat(cmAlphaNum const& a, cmAlphaNum const& b,
-                            AV const&... args)
+template <typename A, typename B, typename... AV>
+inline std::string cmStrCat(A&& a, B&& b, AV&&... args)
 {
-  return cmCatViews(
-    { a.View(), b.View(), static_cast<cmAlphaNum const&>(args).View()... });
+  return cmStrCatHelper<A, B, AV...>::Compute(
+    std::forward<A>(a), std::forward<B>(b), std::forward<AV>(args)...);
 }
 
 /** Joins wrapped elements of a range with separator into a single string.  */
@@ -207,7 +234,9 @@ std::string cmWrap(cm::string_view prefix, Range const& rng,
     return std::string();
   }
   return cmCatViews(
-    { prefix, cmJoin(rng, cmCatViews({ suffix, sep, prefix })), suffix });
+    cm::nullopt,
+    { prefix, cmJoin(rng, cmCatViews(cm::nullopt, { suffix, sep, prefix })),
+      suffix });
 }
 
 /** Joins wrapped elements of a range with separator into a single string.  */
diff --git a/Source/cmStringCommand.cxx b/Source/cmStringCommand.cxx
index c12d1fe..5a64588 100644
--- a/Source/cmStringCommand.cxx
+++ b/Source/cmStringCommand.cxx
@@ -9,7 +9,6 @@
 #include <cctype>
 #include <cstdio>
 #include <cstdlib>
-#include <initializer_list>
 #include <limits>
 #include <memory>
 #include <stdexcept>
@@ -950,9 +949,9 @@ struct Args : cmRange<typename std::vector<std::string>::const_iterator>
 class json_error : public std::runtime_error
 {
 public:
-  json_error(std::initializer_list<cm::string_view> message,
+  json_error(const std::string& message,
              cm::optional<Args> errorPath = cm::nullopt)
-    : std::runtime_error(cmCatViews(message))
+    : std::runtime_error(message)
     , ErrorPath{
       std::move(errorPath) // NOLINT(performance-move-const-arg)
     }
@@ -964,7 +963,7 @@ public:
 const std::string& Args::PopFront(cm::string_view error)
 {
   if (this->empty()) {
-    throw json_error({ error });
+    throw json_error(std::string(error));
   }
   const std::string& res = *this->begin();
   this->advance(1);
@@ -974,7 +973,7 @@ const std::string& Args::PopFront(cm::string_view error)
 const std::string& Args::PopBack(cm::string_view error)
 {
   if (this->empty()) {
-    throw json_error({ error });
+    throw json_error(std::string(error));
   }
   const std::string& res = *(this->end() - 1);
   this->retreat(1);
@@ -999,7 +998,7 @@ cm::string_view JsonTypeToString(Json::ValueType type)
     case Json::ValueType::objectValue:
       return "OBJECT"_s;
   }
-  throw json_error({ "invalid JSON type found"_s });
+  throw json_error("invalid JSON type found");
 }
 
 int ParseIndex(
@@ -1008,14 +1007,14 @@ int ParseIndex(
 {
   unsigned long lindex;
   if (!cmStrToULong(str, &lindex)) {
-    throw json_error({ "expected an array index, got: '"_s, str, "'"_s },
+    throw json_error(cmStrCat("expected an array index, got: '"_s, str, "'"_s),
                      progress);
   }
   Json::ArrayIndex index = static_cast<Json::ArrayIndex>(lindex);
   if (index >= max) {
     cmAlphaNum sizeStr{ max };
-    throw json_error({ "expected an index less than "_s, sizeStr.View(),
-                       " got '"_s, str, "'"_s },
+    throw json_error(cmStrCat("expected an index less than "_s, sizeStr.View(),
+                              " got '"_s, str, "'"_s),
                      progress);
   }
   return index;
@@ -1036,16 +1035,16 @@ Json::Value& ResolvePath(Json::Value& json, Args path)
     } else if (search->isObject()) {
       if (!search->isMember(field)) {
         const auto progressStr = cmJoin(progress, " "_s);
-        throw json_error({ "member '"_s, progressStr, "' not found"_s },
+        throw json_error(cmStrCat("member '"_s, progressStr, "' not found"_s),
                          progress);
       }
       search = &(*search)[field];
     } else {
       const auto progressStr = cmJoin(progress, " "_s);
       throw json_error(
-        { "invalid path '"_s, progressStr,
-          "', need element of OBJECT or ARRAY type to lookup '"_s, field,
-          "' got "_s, JsonTypeToString(search->type()) },
+        cmStrCat("invalid path '"_s, progressStr,
+                 "', need element of OBJECT or ARRAY type to lookup '"_s,
+                 field, "' got "_s, JsonTypeToString(search->type())),
         progress);
     }
   }
@@ -1061,7 +1060,7 @@ Json::Value ReadJson(const std::string& jsonstr)
   std::string error;
   if (!jsonReader->parse(jsonstr.data(), jsonstr.data() + jsonstr.size(),
                          &json, &error)) {
-    throw json_error({ "failed parsing json string: "_s, error });
+    throw json_error(cmStrCat("failed parsing json string: "_s, error));
   }
   return json;
 }
@@ -1101,9 +1100,9 @@ bool HandleJSONCommand(std::vector<std::string> const& arguments,
         mode != "LENGTH"_s && mode != "REMOVE"_s && mode != "SET"_s &&
         mode != "EQUAL"_s) {
       throw json_error(
-        { "got an invalid mode '"_s, mode,
-          "', expected one of GET, TYPE, MEMBER, LENGTH, REMOVE, SET, "
-          " EQUAL"_s });
+        cmStrCat("got an invalid mode '"_s, mode,
+                 "', expected one of GET, TYPE, MEMBER, LENGTH, REMOVE, SET, "
+                 " EQUAL"_s));
     }
 
     const auto& jsonstr = args.PopFront("missing json string argument"_s);
@@ -1127,10 +1126,11 @@ bool HandleJSONCommand(std::vector<std::string> const& arguments,
       const auto& indexStr = args.PopBack("missing member index"_s);
       const auto& value = ResolvePath(json, args);
       if (!value.isObject()) {
-        throw json_error({ "MEMBER needs to be called with an element of "
-                           "type OBJECT, got "_s,
-                           JsonTypeToString(value.type()) },
-                         args);
+        throw json_error(
+          cmStrCat("MEMBER needs to be called with an element of "
+                   "type OBJECT, got "_s,
+                   JsonTypeToString(value.type())),
+          args);
       }
       const auto index = ParseIndex(
         indexStr, Args{ args.begin(), args.end() + 1 }, value.size());
@@ -1140,9 +1140,9 @@ bool HandleJSONCommand(std::vector<std::string> const& arguments,
     } else if (mode == "LENGTH"_s) {
       const auto& value = ResolvePath(json, args);
       if (!value.isArray() && !value.isObject()) {
-        throw json_error({ "LENGTH needs to be called with an "
-                           "element of type ARRAY or OBJECT, got "_s,
-                           JsonTypeToString(value.type()) },
+        throw json_error(cmStrCat("LENGTH needs to be called with an "
+                                  "element of type ARRAY or OBJECT, got "_s,
+                                  JsonTypeToString(value.type())),
                          args);
       }
 
@@ -1165,9 +1165,9 @@ bool HandleJSONCommand(std::vector<std::string> const& arguments,
         value.removeMember(toRemove, &removed);
 
       } else {
-        throw json_error({ "REMOVE needs to be called with an "
-                           "element of type ARRAY or OBJECT, got "_s,
-                           JsonTypeToString(value.type()) },
+        throw json_error(cmStrCat("REMOVE needs to be called with an "
+                                  "element of type ARRAY or OBJECT, got "_s,
+                                  JsonTypeToString(value.type())),
                          args);
       }
       makefile.AddDefinition(*outputVariable, WriteJson(json));
@@ -1189,9 +1189,9 @@ bool HandleJSONCommand(std::vector<std::string> const& arguments,
           value.append(newValue);
         }
       } else {
-        throw json_error({ "SET needs to be called with an "
-                           "element of type OBJECT or ARRAY, got "_s,
-                           JsonTypeToString(value.type()) });
+        throw json_error(cmStrCat("SET needs to be called with an "
+                                  "element of type OBJECT or ARRAY, got "_s,
+                                  JsonTypeToString(value.type())));
       }
 
       makefile.AddDefinition(*outputVariable, WriteJson(json));
@@ -1207,7 +1207,7 @@ bool HandleJSONCommand(std::vector<std::string> const& arguments,
     if (outputVariable && e.ErrorPath) {
       const auto errorPath = cmJoin(*e.ErrorPath, "-");
       makefile.AddDefinition(*outputVariable,
-                             cmCatViews({ errorPath, "-NOTFOUND"_s }));
+                             cmStrCat(errorPath, "-NOTFOUND"_s));
     } else if (outputVariable) {
       makefile.AddDefinition(*outputVariable, "NOTFOUND"_s);
     }
@@ -1215,7 +1215,7 @@ bool HandleJSONCommand(std::vector<std::string> const& arguments,
     if (errorVariable) {
       makefile.AddDefinition(*errorVariable, e.what());
     } else {
-      status.SetError(cmCatViews({ "sub-command JSON "_s, e.what(), "."_s }));
+      status.SetError(cmStrCat("sub-command JSON "_s, e.what(), "."_s));
       success = false;
     }
   }
diff --git a/Tests/CMakeLib/testStringAlgorithms.cxx b/Tests/CMakeLib/testStringAlgorithms.cxx
index 1e6b611..f73e62a 100644
--- a/Tests/CMakeLib/testStringAlgorithms.cxx
+++ b/Tests/CMakeLib/testStringAlgorithms.cxx
@@ -6,6 +6,8 @@
 #include <iostream>
 #include <sstream>
 #include <string>
+#include <type_traits>
+#include <utility>
 #include <vector>
 
 #include <cm/string_view>
@@ -144,6 +146,18 @@ int testStringAlgorithms(int /*unused*/, char* /*unused*/ [])
     d -= val;
     assert_ok((d < div) && (d > -div), "cmStrCat double");
   }
+  {
+    std::string val;
+    std::string expect;
+    val.reserve(120 * cmStrLen("cmStrCat move"));
+    auto data = val.data();
+    for (int i = 0; i < 100; i++) {
+      val = cmStrCat(std::move(val), "cmStrCat move");
+      expect += "cmStrCat move";
+    }
+    assert_ok((val.data() == data), "cmStrCat move");
+    assert_string(val, expect, "cmStrCat move");
+  }
 
   // ----------------------------------------------------------------------
   // Test cmWrap