Added StringPiece overloads for base::JoinString.

base/strings/string_util.cc

Index: base/strings/string_util.cc
diff --git a/base/strings/string_util.cc b/base/strings/string_util.cc
index cb668ed7fff4f60f76291a0a19d478fb9faaacd5..4c7f69493443c99424eb8481443ea3bff303cc45 100644
--- a/base/strings/string_util.cc
+++ b/base/strings/string_util.cc
@@ -853,7 +853,7 @@ 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())
@@ -871,6 +871,43 @@ static STR JoinStringT(const std::vector<STR>& parts,
   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);
@@ -881,6 +918,26 @@ string16 JoinString(const std::vector<string16>& parts,
   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,