Browse Source

cmStrCat(): allow any argument to be an rvalue string

This will allow us to re-use any rvalue allocation that is
available, not just from the first argument.
Kyle Edwards 2 years ago
parent
commit
1cca051470
3 changed files with 75 additions and 50 deletions
  1. 30 10
      Source/cmStringAlgorithms.cxx
  2. 31 36
      Source/cmStringAlgorithms.h
  3. 14 4
      Tests/CMakeLib/testStringAlgorithms.cxx

+ 30 - 10
Source/cmStringAlgorithms.cxx

@@ -203,25 +203,45 @@ cmAlphaNum::cmAlphaNum(double val)
   MakeDigits(this->View_, this->Digits_, "%g", val);
 }
 
-std::string cmCatViews(cm::optional<std::string>&& first,
-                       std::initializer_list<cm::string_view> views)
+std::string cmCatViews(
+  std::initializer_list<std::pair<cm::string_view, std::string*>> views)
 {
   std::size_t totalSize = 0;
-  for (cm::string_view const& view : views) {
-    totalSize += view.size();
+  std::string* rvalueString = nullptr;
+  std::size_t rvalueStringLength = 0;
+  std::size_t rvalueStringOffset = 0;
+  for (auto const& view : views) {
+    // Find the rvalue string with the largest capacity.
+    if (view.second &&
+        (!rvalueString ||
+         view.second->capacity() > rvalueString->capacity())) {
+      rvalueString = view.second;
+      rvalueStringLength = rvalueString->length();
+      rvalueStringOffset = totalSize;
+    }
+    totalSize += view.first.size();
   }
 
   std::string result;
   std::string::size_type initialLen = 0;
-  if (first) {
-    totalSize += first->length();
-    initialLen = first->length();
-    result = std::move(*first);
+  if (rvalueString && rvalueString->capacity() >= totalSize) {
+    result = std::move(*rvalueString);
+  } else {
+    rvalueString = nullptr;
   }
   result.resize(totalSize);
+  if (rvalueString && rvalueStringOffset > 0) {
+    std::copy_backward(result.begin(), result.begin() + rvalueStringLength,
+                       result.begin() + rvalueStringOffset +
+                         rvalueStringLength);
+  }
   std::string::iterator sit = result.begin() + initialLen;
-  for (cm::string_view const& view : views) {
-    sit = std::copy_n(view.data(), view.size(), sit);
+  for (auto const& view : views) {
+    if (rvalueString && view.second == rvalueString) {
+      sit += rvalueStringLength;
+    } else {
+      sit = std::copy_n(view.first.data(), view.first.size(), sit);
+    }
   }
   return result;
 }

+ 31 - 36
Source/cmStringAlgorithms.h

@@ -12,7 +12,6 @@
 #include <utility>
 #include <vector>
 
-#include <cm/optional>
 #include <cm/string_view>
 
 #include "cmRange.h"
@@ -147,8 +146,8 @@ std::vector<std::string> cmExpandedLists(InputIt first, InputIt last)
 }
 
 /** Concatenate string pieces into a single string.  */
-std::string cmCatViews(cm::optional<std::string>&& first,
-                       std::initializer_list<cm::string_view> views);
+std::string cmCatViews(
+  std::initializer_list<std::pair<cm::string_view, std::string*>> views);
 
 /** Utility class for cmStrCat.  */
 class cmAlphaNum
@@ -162,6 +161,10 @@ public:
     : View_(str)
   {
   }
+  cmAlphaNum(std::string&& str)
+    : RValueString_(&str)
+  {
+  }
   cmAlphaNum(const char* str)
     : View_(str)
   {
@@ -184,45 +187,34 @@ public:
   {
   }
 
-  cm::string_view View() const { return this->View_; }
+  cm::string_view View() const
+  {
+    if (this->RValueString_) {
+      return *this->RValueString_;
+    }
+    return this->View_;
+  }
+
+  std::string* RValueString() const { return this->RValueString_; }
 
 private:
+  std::string* RValueString_ = nullptr;
   cm::string_view View_;
   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 A, typename B, typename... AV>
 inline std::string cmStrCat(A&& a, B&& b, AV&&... args)
 {
-  return cmStrCatHelper<A, B, AV...>::Compute(
-    std::forward<A>(a), std::forward<B>(b), std::forward<AV>(args)...);
+  static auto const makePair =
+    [](const cmAlphaNum& arg) -> std::pair<cm::string_view, std::string*> {
+    return { arg.View(), arg.RValueString() };
+  };
+
+  return cmCatViews({ makePair(std::forward<A>(a)),
+                      makePair(std::forward<B>(b)),
+                      makePair(std::forward<AV>(args))... });
 }
 
 /** Joins wrapped elements of a range with separator into a single string.  */
@@ -233,10 +225,13 @@ std::string cmWrap(cm::string_view prefix, Range const& rng,
   if (rng.empty()) {
     return std::string();
   }
-  return cmCatViews(
-    cm::nullopt,
-    { prefix, cmJoin(rng, cmCatViews(cm::nullopt, { suffix, sep, prefix })),
-      suffix });
+  return cmCatViews({ { prefix, nullptr },
+                      { cmJoin(rng,
+                               cmCatViews({ { suffix, nullptr },
+                                            { sep, nullptr },
+                                            { prefix, nullptr } })),
+                        nullptr },
+                      { suffix, nullptr } });
 }
 
 /** Joins wrapped elements of a range with separator into a single string.  */

+ 14 - 4
Tests/CMakeLib/testStringAlgorithms.cxx

@@ -149,13 +149,23 @@ int testStringAlgorithms(int /*unused*/, char* /*unused*/ [])
   {
     std::string val;
     std::string expect;
-    val.reserve(120 * cmStrLen("cmStrCat move"));
+    val.reserve(50 * cmStrLen("cmStrCat move ") + 1);
     auto data = val.data();
+    auto capacity = val.capacity();
+    bool moved = true;
     for (int i = 0; i < 100; i++) {
-      val = cmStrCat(std::move(val), "cmStrCat move");
-      expect += "cmStrCat move";
+      if (i % 2 == 0) {
+        val = cmStrCat(std::move(val), "move ");
+        expect += "move ";
+      } else {
+        val = cmStrCat("cmStrCat ", std::move(val));
+        expect = "cmStrCat " + std::move(expect);
+      }
+      if (val.data() != data || val.capacity() != capacity) {
+        moved = false;
+      }
     }
-    assert_ok((val.data() == data), "cmStrCat move");
+    assert_ok(moved, "cmStrCat move");
     assert_string(val, expect, "cmStrCat move");
   }