Added StringPiece overloads for base::JoinString.

base/strings/string_util.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/strings/string_util.h"
 
 #include <ctype.h>
 #include <errno.h>
 #include <math.h>
 #include <stdarg.h>
@@ skipping 835 matching lines @@
 }
 
 char* WriteInto(std::string* str, size_t length_with_null) {
   return WriteIntoT(str, length_with_null);
 }
 
 char16* WriteInto(string16* str, size_t length_with_null) {
   return WriteIntoT(str, length_with_null);
 }
 
-template<typename STR>
+template <typename STR>
 static STR JoinStringT(const std::vector<STR>& parts,
                        BasicStringPiece<STR> sep) {
   if (parts.empty())
     return STR();
 
   STR result(parts[0]);
   auto iter = parts.begin();
   ++iter;
 
   for (; iter != parts.end(); ++iter) {
     sep.AppendToString(&result);
     result += *iter;
   }
 
   return result;
 }
 
+template <typename STR>
+static STR JoinStringPieceT(const std::vector<BasicStringPiece<STR>>& parts,
+                            BasicStringPiece<STR> sep) {
+  if (parts.empty())
+    return STR();
+
+  STR result(parts[0].data(), parts[0].size());
+  auto iter = parts.begin();
+  ++iter;
+
+  for (; iter != parts.end(); ++iter) {
+    sep.AppendToString(&result);
+    iter->AppendToString(&result);
+  }
+
+  return result;
+}
+
+template <typename STR>
+static STR JoinStringPieceT(
+    const std::initializer_list<BasicStringPiece<STR>>& parts,

[Matt Giuca, 2017/02/17 09:51:22]

Upon further thought, I can probably template this and avoid duplicating. I'll
try this on Monday (sorry for so many revisions).

[Matt Giuca, 2017/02/20 23:55:41]

Done.

I managed to combine all 3 into one version you can see in Patch Set 6 (casting
each item to a StringPiece for appending -- this allows it to work on either
StringPieces or strings). I had a brief look at the generated assembly code (on
Linux) and it seems a bit inefficient as it has to call the StringPiece
constructor and AppendToString method, and those won't be inlined without
link-time optimization.

So I would rather not rely on LTO for something in base, and just go with two
separate versions -- Patch Set 7. WDYT?

[danakj, 2017/02/21 23:47:15]

I would say let's avoid temporary objects. You could do this by making an
overloaded helper append function and still keep the rest in a template, for
instance (since string and StringPiece don't share apis).

[Matt Giuca, 2017/02/22 00:10:15]

What temporary object do you mean? (Are you saying my current version has a
temporary somewhere I'm not seeing? Or are you referring to Patch Set 6?)
Nice suggestion. Will address this later but just sending this out now to
confirm: you're suggesting a way to make Patch Set 6 efficient (while keeping
the single implementation), as opposed to saying there's some inefficiency in
Patch Set 7?

[Matt Giuca, 2017/02/22 06:20:13]

Done.

+    BasicStringPiece<STR> sep) {
+  if (parts.size() == 0)
+    return STR();
+
+  auto iter = parts.begin();
+  STR result(iter->data(), iter->size());
+  ++iter;
+
+  for (; iter != parts.end(); ++iter) {
+    sep.AppendToString(&result);
+    iter->AppendToString(&result);
+  }
+
+  return result;
+}
+
 std::string JoinString(const std::vector<std::string>& parts,
                        StringPiece separator) {
   return JoinStringT(parts, separator);
 }
 
 string16 JoinString(const std::vector<string16>& parts,
                     StringPiece16 separator) {
   return JoinStringT(parts, separator);
 }
 
+std::string JoinString(const std::vector<StringPiece>& parts,
+                       StringPiece separator) {
+  return JoinStringPieceT(parts, separator);
+}
+
+string16 JoinString(const std::vector<StringPiece16>& parts,
+                    StringPiece16 separator) {
+  return JoinStringPieceT(parts, separator);
+}
+
+std::string JoinString(const std::initializer_list<StringPiece>& parts,
+                       StringPiece separator) {
+  return JoinStringPieceT(parts, separator);
+}
+
+string16 JoinString(const std::initializer_list<StringPiece16>& parts,
+                    StringPiece16 separator) {
+  return JoinStringPieceT(parts, separator);
+}
+
 template<class FormatStringType, class OutStringType>
 OutStringType DoReplaceStringPlaceholders(
     const FormatStringType& format_string,
     const std::vector<OutStringType>& subst,
     std::vector<size_t>* offsets) {
   size_t substitutions = subst.size();
   DCHECK_LT(substitutions, 10U);
 
   size_t sub_length = 0;
   for (const auto& cur : subst)
@@ skipping 94 matching lines @@
 }  // namespace
 
 size_t strlcpy(char* dst, const char* src, size_t dst_size) {
   return lcpyT<char>(dst, src, dst_size);
 }
 size_t wcslcpy(wchar_t* dst, const wchar_t* src, size_t dst_size) {
   return lcpyT<wchar_t>(dst, src, dst_size);
 }
 
 }  // namespace base