[C++] Refactor StatusOr and StringPiece (#8406)

* [C++] Refactor StatusOr and StringPiece

This change makes `StatusOr` and `StringPiece` more like
`absl::StatusOr` and `{absl,std}::string_view`.

Note that there is more work required before the Abseil types can be
used as drop-in replacement.

Progress on #3688

* Fix more errors

* Fix test

* Remove some asserts

* Delete outdate example

src/google/protobuf/stubs/bytestream.cc

@@ -173,12 +173,8 @@ size_t LimitByteSource::Available() const {
 }
 
 StringPiece LimitByteSource::Peek() {
-  StringPiece piece(source_->Peek());
-  if (piece.size() > limit_) {
-    piece.set(piece.data(), limit_);
-  }
-
-  return piece;
+  StringPiece piece = source_->Peek();
+  return StringPiece(piece.data(), std::min(piece.size(), limit_));
 }
 
 void LimitByteSource::Skip(size_t n) {

src/google/protobuf/stubs/casts.h

@@ -116,8 +116,7 @@ inline To down_cast(From& f) {
 
 template<typename To, typename From>
 inline To bit_cast(const From& from) {
-  GOOGLE_COMPILE_ASSERT(sizeof(From) == sizeof(To),
-                        bit_cast_with_different_sizes);
+  static_assert(sizeof(From) == sizeof(To), "bit_cast_with_different_sizes");
   To dest;
   memcpy(&dest, &from, sizeof(dest));
   return dest;

src/google/protobuf/stubs/common.h

@@ -123,7 +123,6 @@ std::string PROTOBUF_EXPORT VersionString(int version);
 // ===================================================================
 // from google3/util/utf8/public/unilib.h
 
-class StringPiece;
 namespace internal {
 
 // Checks if the buffer contains structurally-valid UTF-8.  Implemented in

src/google/protobuf/stubs/logging.h

@@ -34,6 +34,7 @@
 #include <google/protobuf/stubs/macros.h>
 #include <google/protobuf/stubs/port.h>
 #include <google/protobuf/stubs/status.h>
+#include <google/protobuf/stubs/stringpiece.h>
 
 #include <google/protobuf/port_def.inc>
 
@@ -64,7 +65,6 @@ enum LogLevel {
 #endif
 };
 
-class StringPiece;
 class uint128;
 namespace internal {
 

src/google/protobuf/stubs/macros.h

@@ -31,21 +31,19 @@
 #ifndef GOOGLE_PROTOBUF_MACROS_H__
 #define GOOGLE_PROTOBUF_MACROS_H__
 
-#include <google/protobuf/stubs/port.h>
-
 namespace google {
 namespace protobuf {
 
 #undef GOOGLE_DISALLOW_EVIL_CONSTRUCTORS
 #define GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(TypeName)    \
-  TypeName(const TypeName&);                           \
-  void operator=(const TypeName&)
+  TypeName(const TypeName&) = delete;                  \
+  void operator=(const TypeName&) = delete
 
 #undef GOOGLE_DISALLOW_IMPLICIT_CONSTRUCTORS
 #define GOOGLE_DISALLOW_IMPLICIT_CONSTRUCTORS(TypeName) \
-  TypeName();                                           \
-  TypeName(const TypeName&);                            \
-  void operator=(const TypeName&)
+  TypeName() = delete;                                  \
+  TypeName(const TypeName&) = delete;                   \
+  void operator=(const TypeName&) = delete
 
 // ===================================================================
 // from google3/base/basictypes.h
@@ -89,31 +87,6 @@ namespace protobuf {
   ((sizeof(a) / sizeof(*(a))) / \
    static_cast<size_t>(!(sizeof(a) % sizeof(*(a)))))
 
-// The COMPILE_ASSERT macro can be used to verify that a compile time
-// expression is true. For example, you could use it to verify the
-// size of a static array:
-//
-//   COMPILE_ASSERT(ARRAYSIZE(content_type_names) == CONTENT_NUM_TYPES,
-//                  content_type_names_incorrect_size);
-//
-// or to make sure a struct is smaller than a certain size:
-//
-//   COMPILE_ASSERT(sizeof(foo) < 128, foo_too_large);
-//
-// The second argument to the macro is the name of the variable. If
-// the expression is false, most compilers will issue a warning/error
-// containing the name of the variable.
-
-namespace internal {
-
-template <bool>
-struct CompileAssert {
-};
-
-}  // namespace internal
-
-#define GOOGLE_COMPILE_ASSERT(expr, msg) static_assert(expr, #msg)
-
 }  // namespace protobuf
 }  // namespace google
 

src/google/protobuf/stubs/status_macros.h

@@ -60,7 +60,7 @@ namespace util {
 template<typename T>
 Status DoAssignOrReturn(T& lhs, StatusOr<T> result) {
   if (result.ok()) {
-    lhs = result.ValueOrDie();
+    lhs = result.value();
   }
   return result.status();
 }

src/google/protobuf/stubs/statusor.cc

@@ -35,14 +35,14 @@
 namespace google {
 namespace protobuf {
 namespace util {
-namespace internal {
+namespace statusor_internal {
 
 void StatusOrHelper::Crash(const Status& status) {
   GOOGLE_LOG(FATAL) << "Attempting to fetch value instead of handling error "
                     << status.ToString();
 }
 
-}  // namespace internal
+}  // namespace statusor_internal
 }  // namespace util
 }  // namespace protobuf
 }  // namespace google

src/google/protobuf/stubs/statusor.h

@@ -42,7 +42,7 @@
 //
 //  StatusOr<float> result = DoBigCalculationThatCouldFail();
 //  if (result.ok()) {
-//    float answer = result.ValueOrDie();
+//    float answer = result.value();
 //    printf("Big calculation yielded: %f", answer);
 //  } else {
 //    LOG(ERROR) << result.status();
@@ -52,17 +52,7 @@
 //
 //  StatusOr<Foo*> result = FooFactory::MakeNewFoo(arg);
 //  if (result.ok()) {
-//    std::unique_ptr<Foo> foo(result.ValueOrDie());
-//    foo->DoSomethingCool();
-//  } else {
-//    LOG(ERROR) << result.status();
-//  }
-//
-// Example client usage for a StatusOr<std::unique_ptr<T>>:
-//
-//  StatusOr<std::unique_ptr<Foo>> result = FooFactory::MakeNewFoo(arg);
-//  if (result.ok()) {
-//    std::unique_ptr<Foo> foo = result.ConsumeValueOrDie();
+//    std::unique_ptr<Foo> foo(result.value());
 //    foo->DoSomethingCool();
 //  } else {
 //    LOG(ERROR) << result.status();
@@ -93,17 +83,21 @@
 namespace google {
 namespace protobuf {
 namespace util {
+namespace statusor_internal {
 
 template<typename T>
 class StatusOr {
   template<typename U> friend class StatusOr;
 
  public:
+  using value_type = T;
+
+  // Construct a new StatusOr with Status::UNKNOWN status.
   // Construct a new StatusOr with UnknownError() status.
-  StatusOr();
+  explicit StatusOr();
 
   // Construct a new StatusOr with the given non-ok status. After calling
-  // this constructor, calls to ValueOrDie() will CHECK-fail.
+  // this constructor, calls to value() will CHECK-fail.
   //
   // NOTE: Not explicit - we want to use StatusOr<T> as a return
   // value, so it is convenient and sensible to be able to do 'return
@@ -116,7 +110,7 @@ class StatusOr {
 
   // Construct a new StatusOr with the given value. If T is a plain pointer,
   // value must not be nullptr. After calling this constructor, calls to
-  // ValueOrDie() will succeed, and calls to status() will return OK.
+  // value() will succeed, and calls to status() will return OK.
   //
   // NOTE: Not explicit - we want to use StatusOr<T> as a return type
   // so it is convenient and sensible to be able to do 'return T()'
@@ -149,9 +143,6 @@ class StatusOr {
   bool ok() const;
 
   // Returns a reference to our current value, or CHECK-fails if !this->ok().
-  // If you need to initialize a T object from the stored value,
-  // ConsumeValueOrDie() may be more efficient.
-  const T& ValueOrDie() const;
   const T& value () const;
 
  private:
@@ -162,8 +153,6 @@ class StatusOr {
 ////////////////////////////////////////////////////////////////////////////////
 // Implementation details for StatusOr<T>
 
-namespace internal {
-
 class PROTOBUF_EXPORT StatusOrHelper {
  public:
   // Move type-agnostic error handling to the .cc.
@@ -185,8 +174,6 @@ struct StatusOrHelper::Specialize<T*> {
   static inline bool IsValueNull(const T* t) { return t == nullptr; }
 };
 
-}  // namespace internal
-
 template<typename T>
 inline StatusOr<T>::StatusOr()
     : status_(util::UnknownError("")) {
@@ -203,7 +190,7 @@ inline StatusOr<T>::StatusOr(const Status& status) {
 
 template<typename T>
 inline StatusOr<T>::StatusOr(const T& value) {
-  if (internal::StatusOrHelper::Specialize<T>::IsValueNull(value)) {
+  if (StatusOrHelper::Specialize<T>::IsValueNull(value)) {
     status_ = util::InternalError("nullptr is not a valid argument.");
   } else {
     status_ = util::OkStatus();
@@ -247,21 +234,18 @@ inline bool StatusOr<T>::ok() const {
   return status().ok();
 }
 
-template<typename T>
-inline const T& StatusOr<T>::ValueOrDie() const {
-  if (!status_.ok()) {
-    internal::StatusOrHelper::Crash(status_);
-  }
-  return value_;
-}
-
 template<typename T>
 inline const T& StatusOr<T>::value() const {
   if (!status_.ok()) {
-    internal::StatusOrHelper::Crash(status_);
+    StatusOrHelper::Crash(status_);
   }
   return value_;
 }
+
+}  // namespace statusor_internal
+
+using ::google::protobuf::util::statusor_internal::StatusOr;
+
 }  // namespace util
 }  // namespace protobuf
 }  // namespace google

src/google/protobuf/stubs/statusor_test.cc

@@ -84,15 +84,15 @@ TEST(StatusOr, TestValueCtor) {
   const int kI = 4;
   StatusOr<int> thing(kI);
   EXPECT_TRUE(thing.ok());
-  EXPECT_EQ(kI, thing.ValueOrDie());
+  EXPECT_EQ(kI, thing.value());
 }
 
 TEST(StatusOr, TestCopyCtorStatusOk) {
   const int kI = 4;
   StatusOr<int> original(kI);
   StatusOr<int> copy(original);
   EXPECT_EQ(original.status(), copy.status());
-  EXPECT_EQ(original.ValueOrDie(), copy.ValueOrDie());
+  EXPECT_EQ(original.value(), copy.value());
 }
 
 TEST(StatusOr, TestCopyCtorStatusNotOk) {
@@ -106,7 +106,7 @@ TEST(StatusOr, TestCopyCtorStatusOKConverting) {
   StatusOr<int>    original(kI);
   StatusOr<double> copy(original);
   EXPECT_EQ(original.status(), copy.status());
-  EXPECT_EQ(original.ValueOrDie(), copy.ValueOrDie());
+  EXPECT_EQ(original.value(), copy.value());
 }
 
 TEST(StatusOr, TestCopyCtorStatusNotOkConverting) {
@@ -121,7 +121,7 @@ TEST(StatusOr, TestAssignmentStatusOk) {
   StatusOr<int> target;
   target = source;
   EXPECT_EQ(source.status(), target.status());
-  EXPECT_EQ(source.ValueOrDie(), target.ValueOrDie());
+  EXPECT_EQ(source.value(), target.value());
 }
 
 TEST(StatusOr, TestAssignmentStatusNotOk) {
@@ -137,7 +137,7 @@ TEST(StatusOr, TestAssignmentStatusOKConverting) {
   StatusOr<double> target;
   target = source;
   EXPECT_EQ(source.status(), target.status());
-  EXPECT_DOUBLE_EQ(source.ValueOrDie(), target.ValueOrDie());
+  EXPECT_DOUBLE_EQ(source.value(), target.value());
 }
 
 TEST(StatusOr, TestAssignmentStatusNotOkConverting) {
@@ -158,13 +158,13 @@ TEST(StatusOr, TestStatus) {
 TEST(StatusOr, TestValue) {
   const int kI = 4;
   StatusOr<int> thing(kI);
-  EXPECT_EQ(kI, thing.ValueOrDie());
+  EXPECT_EQ(kI, thing.value());
 }
 
 TEST(StatusOr, TestValueConst) {
   const int kI = 4;
   const StatusOr<int> thing(kI);
-  EXPECT_EQ(kI, thing.ValueOrDie());
+  EXPECT_EQ(kI, thing.value());
 }
 
 TEST(StatusOr, TestPointerDefaultCtor) {
@@ -183,15 +183,15 @@ TEST(StatusOr, TestPointerValueCtor) {
   const int kI = 4;
   StatusOr<const int*> thing(&kI);
   EXPECT_TRUE(thing.ok());
-  EXPECT_EQ(&kI, thing.ValueOrDie());
+  EXPECT_EQ(&kI, thing.value());
 }
 
 TEST(StatusOr, TestPointerCopyCtorStatusOk) {
   const int kI = 0;
   StatusOr<const int*> original(&kI);
   StatusOr<const int*> copy(original);
   EXPECT_EQ(original.status(), copy.status());
-  EXPECT_EQ(original.ValueOrDie(), copy.ValueOrDie());
+  EXPECT_EQ(original.value(), copy.value());
 }
 
 TEST(StatusOr, TestPointerCopyCtorStatusNotOk) {
@@ -205,8 +205,7 @@ TEST(StatusOr, TestPointerCopyCtorStatusOKConverting) {
   StatusOr<Derived*> original(&derived);
   StatusOr<Base2*>   copy(original);
   EXPECT_EQ(original.status(), copy.status());
-  EXPECT_EQ(static_cast<const Base2*>(original.ValueOrDie()),
-            copy.ValueOrDie());
+  EXPECT_EQ(static_cast<const Base2*>(original.value()), copy.value());
 }
 
 TEST(StatusOr, TestPointerCopyCtorStatusNotOkConverting) {
@@ -221,7 +220,7 @@ TEST(StatusOr, TestPointerAssignmentStatusOk) {
   StatusOr<const int*> target;
   target = source;
   EXPECT_EQ(source.status(), target.status());
-  EXPECT_EQ(source.ValueOrDie(), target.ValueOrDie());
+  EXPECT_EQ(source.value(), target.value());
 }
 
 TEST(StatusOr, TestPointerAssignmentStatusNotOk) {
@@ -237,8 +236,7 @@ TEST(StatusOr, TestPointerAssignmentStatusOKConverting) {
   StatusOr<Base2*>   target;
   target = source;
   EXPECT_EQ(source.status(), target.status());
-  EXPECT_EQ(static_cast<const Base2*>(source.ValueOrDie()),
-            target.ValueOrDie());
+  EXPECT_EQ(static_cast<const Base2*>(source.value()), target.value());
 }
 
 TEST(StatusOr, TestPointerAssignmentStatusNotOkConverting) {
@@ -259,13 +257,13 @@ TEST(StatusOr, TestPointerStatus) {
 TEST(StatusOr, TestPointerValue) {
   const int kI = 0;
   StatusOr<const int*> thing(&kI);
-  EXPECT_EQ(&kI, thing.ValueOrDie());
+  EXPECT_EQ(&kI, thing.value());
 }
 
 TEST(StatusOr, TestPointerValueConst) {
   const int kI = 0;
   const StatusOr<const int*> thing(&kI);
-  EXPECT_EQ(&kI, thing.ValueOrDie());
+  EXPECT_EQ(&kI, thing.value());
 }
 
 }  // namespace