Finish StringPiece and unit tests

paddle/strings/stringpiece.cc

@@ -16,7 +16,9 @@
 
 #include "paddle/strings/stringpiece.h"
 
-#include <stddef.h>
+// #include <stddef.h>
+#include <string.h>
+
 #include <algorithm>
 #include <iosfwd>
 
@@ -72,12 +74,14 @@ bool HasSuffix(StringPiece s, StringPiece x) {
 }
 
 StringPiece SkipPrefix(StringPiece s, size_t n) {
-  assert(n <= s.len());
+  if (n > s.len())
+    throw std::invalid_argument("Skip distance larger than StringPiece length");
   return StringPiece(s.data() + n, s.len() - n);
 }
 
 StringPiece SkipSuffix(StringPiece s, size_t n) {
-  assert(size_ >= n);
+  if (n > s.len())
+    throw std::invalid_argument("Skip distance larger than StringPiece length");
   return StringPiece(s.data(), s.len() - n);
 }
 
@@ -124,4 +128,8 @@ StringPiece SubStr(StringPiece s, size_t pos, size_t n) {
   return StringPiece(s.data() + pos, n);
 }
 
+std::ostream& operator<<(std::ostream& o, StringPiece piece) {
+  return o << piece.ToString();
+}
+
 }  // namespace paddle

paddle/strings/stringpiece.h

@@ -17,7 +17,6 @@
 #pragma once
 
 #include <assert.h>
-#include <string.h>
 
 #include <stdexcept>
 #include <string>
@@ -26,7 +25,11 @@ namespace paddle {
 
 // StringPiece points into a std::string object but doesn't own the
 // string.  It is for efficient access to strings.  Like Go's string
-// type.  StringPiece is not thread-safe.
+// type.  Not that StringPiece doesn't mutate the underlying string,
+// so it is thread-safe given that the underlying string doesn't
+// change.  Because StringPiece contains a little data members, and
+// its syntax is simple as it doesn't own/manage the string, it is
+// cheap to construct StringPieces and pass them around.
 class StringPiece {
 public:
   static const size_t npos = static_cast<size_t>(-1);
@@ -37,7 +40,7 @@ public:
   // size_ is 0.
   StringPiece();
   StringPiece(const char* d, size_t n);
-  StringPiece(const char* s);
+  StringPiece(const char* d);
   StringPiece(const std::string& s);
 
   const char* data() const { return data_; }
@@ -55,10 +58,6 @@ public:
   iterator begin() const { return data_; }
   iterator end() const { return data_ + size_; }
 
-  struct Hasher {
-    size_t operator()(StringPiece arg) const;
-  };
-
   // Return a string that contains the copy of the referenced data.
   std::string ToString() const { return std::string(data_, size_); }
 
@@ -69,12 +68,6 @@ private:
   // Intentionally copyable
 };
 
-// Because StringPiece contains a little data members, and without the
-// ownership, it is so cheap to pass StringPieces around, we don't
-// need to define parrameters of the following operations to be
-// references.  Also, it is cheap to construct new StringPieces, so we
-// don't define mutative operations as member functions.
-
 int Compare(StringPiece a, StringPiece b);
 
 bool operator==(StringPiece x, StringPiece y);
@@ -94,9 +87,13 @@ StringPiece SkipSuffix(StringPiece s, size_t n);
 StringPiece TrimPrefix(StringPiece s, StringPiece prefix);
 StringPiece TrimSuffix(StringPiece s, StringPiece suffix);
 
+// Returns if s contains sub.  Any s except for empty s contains an
+// empty sub.
 bool Contains(StringPiece s, StringPiece sub);
 
-// Return the first occurrence of sub in s, or npos.
+// Return the first occurrence of sub in s, or npos.  If both s and
+// sub is empty, it returns npos; otherwise, if only sub is empty, it
+// returns 0.
 size_t Index(StringPiece s, StringPiece sub);
 
 // Return the first occurrence of c in s[pos:end], or npos.
@@ -108,6 +105,6 @@ size_t RFind(StringPiece s, char c, size_t pos);
 StringPiece SubStr(StringPiece s, size_t pos, size_t n);
 
 // allow StringPiece to be logged
-extern std::ostream& operator<<(std::ostream& o, StringPiece piece);
+std::ostream& operator<<(std::ostream& o, StringPiece piece);
 
 }  // namespace paddle