[C++] Refactor util::Status This change refactors util::Status to have a similar shape as the recently open-sourced absl::Status. This will allow Protobuf to eventually use absl::Status and reduce the codesize. Note that there is more work required before absl::Status can be used.
diff --git a/BUILD b/BUILD index 1124321..9b4b2fb 100644 --- a/BUILD +++ b/BUILD
@@ -183,7 +183,7 @@ "src/google/protobuf/stubs/bytestream.cc", "src/google/protobuf/stubs/common.cc", "src/google/protobuf/stubs/int128.cc", - "src/google/protobuf/stubs/status.cc", + "src/google/protobuf/stubs/internal/status.cc", "src/google/protobuf/stubs/statusor.cc", "src/google/protobuf/stubs/stringpiece.cc", "src/google/protobuf/stubs/stringprintf.cc",
diff --git a/cmake/libprotobuf-lite.cmake b/cmake/libprotobuf-lite.cmake index 6d325d5..bbd3c11 100644 --- a/cmake/libprotobuf-lite.cmake +++ b/cmake/libprotobuf-lite.cmake
@@ -20,7 +20,7 @@ ${protobuf_source_dir}/src/google/protobuf/stubs/bytestream.cc ${protobuf_source_dir}/src/google/protobuf/stubs/common.cc ${protobuf_source_dir}/src/google/protobuf/stubs/int128.cc - ${protobuf_source_dir}/src/google/protobuf/stubs/status.cc + ${protobuf_source_dir}/src/google/protobuf/stubs/internal/status.cc ${protobuf_source_dir}/src/google/protobuf/stubs/statusor.cc ${protobuf_source_dir}/src/google/protobuf/stubs/stringpiece.cc ${protobuf_source_dir}/src/google/protobuf/stubs/stringprintf.cc
diff --git a/src/Makefile.am b/src/Makefile.am index a59971a..207e6c9 100644 --- a/src/Makefile.am +++ b/src/Makefile.am
@@ -185,7 +185,7 @@ google/protobuf/io/io_win32.cc \ google/protobuf/stubs/map_util.h \ google/protobuf/stubs/mathutil.h \ - google/protobuf/stubs/status.cc \ + google/protobuf/stubs/internal/status.cc \ google/protobuf/stubs/status.h \ google/protobuf/stubs/status_macros.h \ google/protobuf/stubs/statusor.cc \
diff --git a/src/google/protobuf/stubs/status.cc b/src/google/protobuf/stubs/internal/status.cc similarity index 73% rename from src/google/protobuf/stubs/status.cc rename to src/google/protobuf/stubs/internal/status.cc index 03b37c3..731e108 100644 --- a/src/google/protobuf/stubs/status.cc +++ b/src/google/protobuf/stubs/internal/status.cc
@@ -37,42 +37,44 @@ namespace google { namespace protobuf { namespace util { -namespace error { -inline std::string CodeEnumToString(error::Code code) { +namespace status_internal { +namespace { + +inline std::string StatusCodeToString(StatusCode code) { switch (code) { - case OK: + case StatusCode::kOk: return "OK"; - case CANCELLED: + case StatusCode::kCancelled: return "CANCELLED"; - case UNKNOWN: + case StatusCode::kUnknown: return "UNKNOWN"; - case INVALID_ARGUMENT: + case StatusCode::kInvalidArgument: return "INVALID_ARGUMENT"; - case DEADLINE_EXCEEDED: + case StatusCode::kDeadlineExceeded: return "DEADLINE_EXCEEDED"; - case NOT_FOUND: + case StatusCode::kNotFound: return "NOT_FOUND"; - case ALREADY_EXISTS: + case StatusCode::kAlreadyExists: return "ALREADY_EXISTS"; - case PERMISSION_DENIED: + case StatusCode::kPermissionDenied: return "PERMISSION_DENIED"; - case UNAUTHENTICATED: + case StatusCode::kUnauthenticated: return "UNAUTHENTICATED"; - case RESOURCE_EXHAUSTED: + case StatusCode::kResourceExhausted: return "RESOURCE_EXHAUSTED"; - case FAILED_PRECONDITION: + case StatusCode::kFailedPrecondition: return "FAILED_PRECONDITION"; - case ABORTED: + case StatusCode::kAborted: return "ABORTED"; - case OUT_OF_RANGE: + case StatusCode::kOutOfRange: return "OUT_OF_RANGE"; - case UNIMPLEMENTED: + case StatusCode::kUnimplemented: return "UNIMPLEMENTED"; - case INTERNAL: + case StatusCode::kInternal: return "INTERNAL"; - case UNAVAILABLE: + case StatusCode::kUnavailable: return "UNAVAILABLE"; - case DATA_LOSS: + case StatusCode::kDataLoss: return "DATA_LOSS"; } @@ -80,18 +82,19 @@ // above switch. return "UNKNOWN"; } -} // namespace error. + +} // namespace const Status Status::OK = Status(); -const Status Status::CANCELLED = Status(error::CANCELLED, ""); -const Status Status::UNKNOWN = Status(error::UNKNOWN, ""); +const Status Status::CANCELLED = Status(StatusCode::kCancelled, ""); +const Status Status::UNKNOWN = Status(StatusCode::kUnknown, ""); -Status::Status() : error_code_(error::OK) { +Status::Status() : error_code_(StatusCode::kOk) { } -Status::Status(error::Code error_code, StringPiece error_message) +Status::Status(StatusCode error_code, StringPiece error_message) : error_code_(error_code) { - if (error_code != error::OK) { + if (error_code != StatusCode::kOk) { error_message_ = error_message.ToString(); } } @@ -112,13 +115,13 @@ } std::string Status::ToString() const { - if (error_code_ == error::OK) { + if (error_code_ == StatusCode::kOk) { return "OK"; } else { if (error_message_.empty()) { - return error::CodeEnumToString(error_code_); + return StatusCodeToString(error_code_); } else { - return error::CodeEnumToString(error_code_) + ":" + + return StatusCodeToString(error_code_) + ":" + error_message_; } } @@ -129,6 +132,7 @@ return os; } +} // namespace status_internal } // namespace util } // namespace protobuf } // namespace google
diff --git a/src/google/protobuf/stubs/internal/status.h b/src/google/protobuf/stubs/internal/status.h new file mode 100644 index 0000000..79c1f80 --- /dev/null +++ b/src/google/protobuf/stubs/internal/status.h
@@ -0,0 +1,119 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#ifndef GOOGLE_PROTOBUF_STUBS_INTERNAL_STATUS_H_ +#define GOOGLE_PROTOBUF_STUBS_INTERNAL_STATUS_H_ + +#include <string> + +#include <google/protobuf/stubs/stringpiece.h> + +#include <google/protobuf/port_def.inc> + +namespace google { +namespace protobuf { +namespace util { +namespace status_internal { + +// These values must match error codes defined in google/rpc/code.proto. +enum class StatusCode : int { + kOk = 0, + kCancelled = 1, + kUnknown = 2, + kInvalidArgument = 3, + kDeadlineExceeded = 4, + kNotFound = 5, + kAlreadyExists = 6, + kPermissionDenied = 7, + kUnauthenticated = 16, + kResourceExhausted = 8, + kFailedPrecondition = 9, + kAborted = 10, + kOutOfRange = 11, + kUnimplemented = 12, + kInternal = 13, + kUnavailable = 14, + kDataLoss = 15, +}; + +class PROTOBUF_EXPORT Status { + public: + // Creates a "successful" status. + Status(); + + // Create a status in the canonical error space with the specified + // code, and error message. If "code == 0", error_message is + // ignored and a Status object identical to Status::kOk is + // constructed. + Status(StatusCode error_code, StringPiece error_message); + Status(const Status&); + Status& operator=(const Status& x); + ~Status() {} + + // Some pre-defined Status objects + static const Status OK; // Identical to 0-arg constructor + static const Status CANCELLED; + static const Status UNKNOWN; + + // Accessor + bool ok() const { + return error_code_ == StatusCode::kOk; + } + StatusCode code() const { + return error_code_; + } + StringPiece message() const { + return error_message_; + } + + bool operator==(const Status& x) const; + bool operator!=(const Status& x) const { + return !operator==(x); + } + + // Return a combination of the error code name and message. + std::string ToString() const; + + private: + StatusCode error_code_; + std::string error_message_; +}; + +// Prints a human-readable representation of 'x' to 'os'. +PROTOBUF_EXPORT std::ostream& operator<<(std::ostream& os, const Status& x); + +} // namespace status_internal +} // namespace util +} // namespace protobuf +} // namespace google + +#include <google/protobuf/port_undef.inc> + +#endif // GOOGLE_PROTOBUF_STUBS_INTERNAL_STATUS_H_
diff --git a/src/google/protobuf/stubs/logging.h b/src/google/protobuf/stubs/logging.h index f37048d..110ccdc 100644 --- a/src/google/protobuf/stubs/logging.h +++ b/src/google/protobuf/stubs/logging.h
@@ -33,6 +33,7 @@ #include <google/protobuf/stubs/macros.h> #include <google/protobuf/stubs/port.h> +#include <google/protobuf/stubs/status.h> #include <google/protobuf/port_def.inc> @@ -64,9 +65,6 @@ }; class StringPiece; -namespace util { -class Status; -} class uint128; namespace internal {
diff --git a/src/google/protobuf/stubs/status.h b/src/google/protobuf/stubs/status.h index bededad..67951ab 100644 --- a/src/google/protobuf/stubs/status.h +++ b/src/google/protobuf/stubs/status.h
@@ -27,99 +27,32 @@ // THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + #ifndef GOOGLE_PROTOBUF_STUBS_STATUS_H_ #define GOOGLE_PROTOBUF_STUBS_STATUS_H_ -#include <iosfwd> -#include <string> - -#include <google/protobuf/stubs/common.h> -#include <google/protobuf/stubs/stringpiece.h> - -#include <google/protobuf/port_def.inc> +#include <google/protobuf/stubs/internal/status.h> namespace google { namespace protobuf { namespace util { + +using ::google::protobuf::util::status_internal::Status; +using ::google::protobuf::util::status_internal::StatusCode; + namespace error { -// These values must match error codes defined in google/rpc/code.proto. -enum Code { - OK = 0, - CANCELLED = 1, - UNKNOWN = 2, - INVALID_ARGUMENT = 3, - DEADLINE_EXCEEDED = 4, - NOT_FOUND = 5, - ALREADY_EXISTS = 6, - PERMISSION_DENIED = 7, - UNAUTHENTICATED = 16, - RESOURCE_EXHAUSTED = 8, - FAILED_PRECONDITION = 9, - ABORTED = 10, - OUT_OF_RANGE = 11, - UNIMPLEMENTED = 12, - INTERNAL = 13, - UNAVAILABLE = 14, - DATA_LOSS = 15, -}; + +// TODO(yannic): Remove these. +constexpr StatusCode OK = StatusCode::kOk; +constexpr StatusCode CANCELLED = StatusCode::kCancelled; +constexpr StatusCode UNKNOWN = StatusCode::kUnknown; +constexpr StatusCode INVALID_ARGUMENT = StatusCode::kInvalidArgument; +constexpr StatusCode NOT_FOUND = StatusCode::kNotFound; +constexpr StatusCode INTERNAL = StatusCode::kInternal; + } // namespace error - -class PROTOBUF_EXPORT Status { - public: - // Creates a "successful" status. - Status(); - - // Create a status in the canonical error space with the specified - // code, and error message. If "code == 0", error_message is - // ignored and a Status object identical to Status::OK is - // constructed. - Status(error::Code error_code, StringPiece error_message); - Status(const Status&); - Status& operator=(const Status& x); - ~Status() {} - - // Some pre-defined Status objects - static const Status OK; // Identical to 0-arg constructor - static const Status CANCELLED; - static const Status UNKNOWN; - - // Accessor - bool ok() const { - return error_code_ == error::OK; - } - int error_code() const { - return error_code_; - } - error::Code code() const { - return error_code_; - } - StringPiece error_message() const { - return error_message_; - } - StringPiece message() const { - return error_message_; - } - - bool operator==(const Status& x) const; - bool operator!=(const Status& x) const { - return !operator==(x); - } - - // Return a combination of the error code name and message. - std::string ToString() const; - - private: - error::Code error_code_; - std::string error_message_; -}; - -// Prints a human-readable representation of 'x' to 'os'. -PROTOBUF_EXPORT std::ostream& operator<<(std::ostream& os, const Status& x); - } // namespace util } // namespace protobuf } // namespace google -#include <google/protobuf/port_undef.inc> - #endif // GOOGLE_PROTOBUF_STUBS_STATUS_H_
diff --git a/src/google/protobuf/stubs/status_test.cc b/src/google/protobuf/stubs/status_test.cc index 8f4398c..ce48680 100644 --- a/src/google/protobuf/stubs/status_test.cc +++ b/src/google/protobuf/stubs/status_test.cc
@@ -39,17 +39,13 @@ namespace { TEST(Status, Empty) { util::Status status; - EXPECT_EQ(util::error::OK, util::Status::OK.error_code()); EXPECT_EQ(util::error::OK, util::Status::OK.code()); EXPECT_EQ("OK", util::Status::OK.ToString()); } TEST(Status, GenericCodes) { - EXPECT_EQ(util::error::OK, util::Status::OK.error_code()); EXPECT_EQ(util::error::OK, util::Status::OK.code()); - EXPECT_EQ(util::error::CANCELLED, util::Status::CANCELLED.error_code()); EXPECT_EQ(util::error::CANCELLED, util::Status::CANCELLED.code()); - EXPECT_EQ(util::error::UNKNOWN, util::Status::UNKNOWN.error_code()); EXPECT_EQ(util::error::UNKNOWN, util::Status::UNKNOWN.code()); } @@ -69,17 +65,17 @@ TEST(Status, ErrorMessage) { util::Status status(util::error::INVALID_ARGUMENT, ""); EXPECT_FALSE(status.ok()); - EXPECT_EQ("", status.error_message().ToString()); + EXPECT_EQ("", status.message().ToString()); EXPECT_EQ("", status.message().ToString()); EXPECT_EQ("INVALID_ARGUMENT", status.ToString()); status = util::Status(util::error::INVALID_ARGUMENT, "msg"); EXPECT_FALSE(status.ok()); - EXPECT_EQ("msg", status.error_message().ToString()); + EXPECT_EQ("msg", status.message().ToString()); EXPECT_EQ("msg", status.message().ToString()); EXPECT_EQ("INVALID_ARGUMENT:msg", status.ToString()); status = util::Status(util::error::OK, "msg"); EXPECT_TRUE(status.ok()); - EXPECT_EQ("", status.error_message().ToString()); + EXPECT_EQ("", status.message().ToString()); EXPECT_EQ("", status.message().ToString()); EXPECT_EQ("OK", status.ToString()); }
diff --git a/src/google/protobuf/stubs/statusor.h b/src/google/protobuf/stubs/statusor.h index c02e89a..c3748ea 100644 --- a/src/google/protobuf/stubs/statusor.h +++ b/src/google/protobuf/stubs/statusor.h
@@ -32,7 +32,7 @@ // object. StatusOr models the concept of an object that is either a // usable value, or an error Status explaining why such a value is // not present. To this end, StatusOr<T> does not allow its Status -// value to be Status::OK. Further, StatusOr<T*> does not allow the +// value to be Status::kOk. Further, StatusOr<T*> does not allow the // contained pointer to be nullptr. // // The primary use-case for StatusOr<T> is as the return value of a @@ -110,8 +110,8 @@ // value, so it is convenient and sensible to be able to do 'return // Status()' when the return type is StatusOr<T>. // - // REQUIRES: status != Status::OK. This requirement is DCHECKed. - // In optimized builds, passing Status::OK here will have the effect + // REQUIRES: status != Status::kOk. This requirement is DCHECKed. + // In optimized builds, passing Status::kOk here will have the effect // of passing PosixErrorSpace::EINVAL as a fallback. StatusOr(const Status& status); // NOLINT @@ -143,7 +143,7 @@ StatusOr& operator=(const StatusOr<U>& other); // Returns a reference to our status. If this contains a T, then - // returns Status::OK. + // returns Status::kOk. const Status& status() const; // Returns this->status().ok() @@ -196,7 +196,8 @@ template<typename T> inline StatusOr<T>::StatusOr(const Status& status) { if (status.ok()) { - status_ = Status(error::INTERNAL, "Status::OK is not a valid argument."); + status_ = + Status(StatusCode::kInternal, "Status::kOk is not a valid argument."); } else { status_ = status; } @@ -205,7 +206,7 @@ template<typename T> inline StatusOr<T>::StatusOr(const T& value) { if (internal::StatusOrHelper::Specialize<T>::IsValueNull(value)) { - status_ = Status(error::INTERNAL, "nullptr is not a valid argument."); + status_ = Status(StatusCode::kInternal, "nullptr is not a valid argument."); } else { status_ = Status::OK; value_ = value;
diff --git a/src/google/protobuf/util/internal/datapiece.cc b/src/google/protobuf/util/internal/datapiece.cc index d3a98e5..feb4ab4 100644 --- a/src/google/protobuf/util/internal/datapiece.cc +++ b/src/google/protobuf/util/internal/datapiece.cc
@@ -48,7 +48,6 @@ namespace converter { using util::Status; -using util::error::Code; namespace {
diff --git a/src/google/protobuf/util/internal/json_stream_parser_test.cc b/src/google/protobuf/util/internal/json_stream_parser_test.cc index 21620c2..5e25a98 100644 --- a/src/google/protobuf/util/internal/json_stream_parser_test.cc +++ b/src/google/protobuf/util/internal/json_stream_parser_test.cc
@@ -139,7 +139,7 @@ }) { util::Status result = RunTest(json, split, setup); EXPECT_EQ(util::error::INVALID_ARGUMENT, result.code()); - StringPiece error_message(result.error_message()); + StringPiece error_message(result.message()); EXPECT_EQ(error_prefix, error_message.substr(0, error_prefix.size())); }
diff --git a/src/google/protobuf/util/internal/protostream_objectsource.cc b/src/google/protobuf/util/internal/protostream_objectsource.cc index 472aa41..8ad51df 100644 --- a/src/google/protobuf/util/internal/protostream_objectsource.cc +++ b/src/google/protobuf/util/internal/protostream_objectsource.cc
@@ -60,11 +60,8 @@ namespace google { namespace protobuf { namespace util { -namespace error { -using util::error::Code; -using util::error::INTERNAL; -} // namespace error namespace converter { + using ::PROTOBUF_NAMESPACE_ID::internal::WireFormat; using ::PROTOBUF_NAMESPACE_ID::internal::WireFormatLite;