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 46 matching lines @@
 
   // Starting position in the string.
   size_t offset;
 };
 
 static bool CompareParameter(const ReplacementOffset& elem1,
                              const ReplacementOffset& elem2) {
   return elem1.parameter < elem2.parameter;
 }
 
+// Overloaded function to append one string onto the end of another. Having a
+// separate overload for |source| as both stringpiece and string allows for more

[danakj, 2017/02/22 15:10:45]

StringPiece

[Matt Giuca, 2017/02/22 22:39:36]

I was trying to consistently use lowercase "stringpiece" to refer to "any type
of stringpiece (StringPiece or StringPiece16)". I guess that isn't clear though.
Should I use "BasicStringPiece" instead? Or just "StringPiece" by itself, with
SP16 implied?

[danakj, 2017/02/22 22:40:30]

SP16 implied imo. lowercase breaks case-sensitive regex/search so I don't like.
BasicStringPiece would be more confusing to read.

[Matt Giuca, 2017/02/23 04:48:06]

Done.

+// efficient usage from functions templated to work with either type (avoiding a
+// redundant call to the BasicStringPiece constructor in both cases).
+template <typename STR>
+inline void AppendToString(STR* target, const STR& source) {
+  target->append(source);
+}
+template <typename STR>

[danakj, 2017/02/22 15:10:45]

style: blank line between functions

[Matt Giuca, 2017/02/23 04:48:06]

Done.

+inline void AppendToString(STR* target, const BasicStringPiece<STR>& source) {
+  source.AppendToString(target);
+}
+
 // Assuming that a pointer is the size of a "machine word", then
 // uintptr_t is an integer type that is also a machine word.
 typedef uintptr_t MachineWord;
 const uintptr_t kMachineWordAlignmentMask = sizeof(MachineWord) - 1;
 
 inline bool IsAlignedToMachineWord(const void* pointer) {
   return !(reinterpret_cast<MachineWord>(pointer) & kMachineWordAlignmentMask);
 }
 
 template<typename T> inline T* AlignToMachineWord(T* pointer) {
@@ skipping 769 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>
-static STR JoinStringT(const std::vector<STR>& parts,
-                       BasicStringPiece<STR> sep) {
-  if (parts.empty())
+// Generic version for all JoinString overloads. |LIST| must be a sequence
+// (std::vector or std::initializer_list) of strings/stringpieces (std::string,
+// string16, StringPiece or StringPiece16). |STR| is either std::string or
+// string16.
+template <typename LIST, typename STR>

[danakj, 2017/02/22 15:10:45]

It's not normal in chromium for template types to be ALLCAPS like this. I see
that STR came this way, but maybe we can stop doing that now.

[Matt Giuca, 2017/02/23 04:48:06]

Done.

+static STR JoinStringT(const LIST& parts, BasicStringPiece<STR> sep) {
+  auto iter = parts.begin();
+  // Early-exit to avoid bad access to the first element.
+  if (iter == parts.end())
     return STR();
 
-  STR result(parts[0]);
-  auto iter = parts.begin();
+  // Begin constructing the result from the first element.
+  STR result(iter->data(), iter->size());
   ++iter;
 
   for (; iter != parts.end(); ++iter) {
     sep.AppendToString(&result);
-    result += *iter;
+    // Using the overloaded AppendToString allows this template function to work
+    // on both strings and stringpieces without creating an intermediate

[danakj, 2017/02/22 15:10:46]

StringPieces

[Matt Giuca, 2017/02/23 04:48:06]

Done.

+    // stringpiece object.

[danakj, 2017/02/22 15:10:45]

StringPiece

[Matt Giuca, 2017/02/23 04:48:06]

Done.

+    AppendToString(&result, *iter);
   }
 
   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 JoinStringT(parts, separator);
+}
+
+string16 JoinString(const std::vector<StringPiece16>& parts,
+                    StringPiece16 separator) {
+  return JoinStringT(parts, separator);
+}
+
+std::string JoinString(const std::initializer_list<StringPiece>& parts,
+                       StringPiece separator) {
+  return JoinStringT(parts, separator);
+}
+
+string16 JoinString(const std::initializer_list<StringPiece16>& parts,
+                    StringPiece16 separator) {
+  return JoinStringT(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