pw_status: Introduce pw::OkStatus()
Status::Ok() is too similar to Status::ok(); having both status.ok() and
status.Ok() compile but mean different things is confusing.
This CL introduces a pw::OkStatus() free function that is an alias for
pw::Status(). This is intended to be a concise, readable way to create
a status with code PW_STATUS_OK.
Change-Id: I85d2ea1d14662c6c4d455219cfeced90c5f92218
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/28900
Reviewed-by: Ewout van Bekkum <ewout@google.com>
Commit-Queue: Auto-Submit <auto-submit@pigweed.google.com.iam.gserviceaccount.com>
Pigweed-Auto-Submit: Wyatt Hepler <hepler@google.com>
diff --git a/pw_status/docs.rst b/pw_status/docs.rst
index a2de75f..0f01ea8 100644
--- a/pw_status/docs.rst
+++ b/pw_status/docs.rst
@@ -20,26 +20,31 @@
) and `gRPC <https://grpc.io>`_ (`doc/statuscodes.md
<https://github.com/grpc/grpc/blob/master/doc/statuscodes.md>`_).
-A ``Status`` is created with a ``static constexpr`` member function
-corresponding to the code.
+An OK ``Status`` is created by the ``pw::OkStatus`` function or by the default
+``Status`` constructor. Non-OK ``Status`` is created with a static member
+function that corresponds with the status code.
.. code-block:: cpp
// Ok (gRPC code "OK") does not indicate an error; this value is returned on
// success. It is typical to check for this value before proceeding on any
// given call across an API or RPC boundary. To check this value, use the
- // `Status::ok()` member function rather than inspecting the raw code.
- Status::Ok()
+ // `status.ok()` member function rather than inspecting the raw code.
+ //
+ // OkStatus() is provided as a free function, rather than a static member
+ // function like the error statuses to avoid conflicts with the ok() member
+ // function. Status::Ok() would be too similar to Status::ok().
+ pw::OkStatus()
// Cancelled (gRPC code "CANCELLED") indicates the operation was cancelled,
// typically by the caller.
- Status::Cancelled()
+ pw::Status::Cancelled()
// Unknown (gRPC code "UNKNOWN") indicates an unknown error occurred. In
// general, more specific errors should be raised, if possible. Errors raised
// by APIs that do not return enough error information may be converted to
// this error.
- Status::Unknown()
+ pw::Status::Unknown()
// InvalidArgument (gRPC code "INVALID_ARGUMENT") indicates the caller
// specified an invalid argument, such a malformed filename. Note that such
@@ -47,14 +52,14 @@
// arguments themselves. Errors with validly formed arguments that may cause
// errors with the state of the receiving system should be denoted with
// `FailedPrecondition` instead.
- Status::InvalidArgument()
+ pw::Status::InvalidArgument()
// DeadlineExceeded (gRPC code "DEADLINE_EXCEEDED") indicates a deadline
// expired before the operation could complete. For operations that may change
// state within a system, this error may be returned even if the operation has
// completed successfully. For example, a successful response from a server
// could have been delayed long enough for the deadline to expire.
- Status::DeadlineExceeded()
+ pw::Status::DeadlineExceeded()
// NotFound (gRPC code "NOT_FOUND") indicates some requested entity (such as
// a file or directory) was not found.
@@ -63,11 +68,11 @@
// users, such as during a gradual feature rollout or undocumented allow list.
// If, instead, a request should be denied for specific sets of users, such as
// through user-based access control, use `PermissionDenied` instead.
- Status::NotFound()
+ pw::Status::NotFound()
// AlreadyExists (gRPC code "ALREADY_EXISTS") indicates the entity that a
// caller attempted to create (such as file or directory) is already present.
- Status::AlreadyExists()
+ pw::Status::AlreadyExists()
// PermissionDenied (gRPC code "PERMISSION_DENIED") indicates that the caller
// does not have permission to execute the specified operation. Note that this
@@ -79,12 +84,12 @@
// some resource. Instead, use `ResourceExhausted` for those errors.
// `PermissionDenied` must not be used if the caller cannot be identified.
// Instead, use `Unauthenticated` for those errors.
- Status::PermissionDenied()
+ pw::Status::PermissionDenied()
// ResourceExhausted (gRPC code "RESOURCE_EXHAUSTED") indicates some resource
// has been exhausted, perhaps a per-user quota, or perhaps the entire file
// system is out of space.
- Status::ResourceExhausted()
+ pw::Status::ResourceExhausted()
// FailedPrecondition (gRPC code "FAILED_PRECONDITION") indicates that the
// operation was rejected because the system is not in a state required for
@@ -103,7 +108,7 @@
// fails because the directory is non-empty, `FailedPrecondition`
// should be returned since the client should not retry unless
// the files are deleted from the directory.
- Status::FailedPrecondition()
+ pw::Status::FailedPrecondition()
// Aborted (gRPC code "ABORTED") indicates the operation was aborted,
// typically due to a concurrency issue such as a sequencer check failure or a
@@ -111,7 +116,7 @@
//
// See the guidelines above for deciding between `FailedPrecondition`,
// `Aborted`, and `Unavailable`.
- Status::Aborted()
+ pw::Status::Aborted()
// OutOfRange (gRPC code "OUT_OF_RANGE") indicates the operation was
// attempted past the valid range, such as seeking or reading past an
@@ -129,17 +134,17 @@
// error) when it applies so that callers who are iterating through
// a space can easily look for an `OutOfRange` error to detect when
// they are done.
- Status::OutOfRange()
+ pw::Status::OutOfRange()
// Unimplemented (gRPC code "UNIMPLEMENTED") indicates the operation is not
// implemented or supported in this service. In this case, the operation
// should not be re-attempted.
- Status::Unimplemented()
+ pw::Status::Unimplemented()
// Internal (gRPC code "INTERNAL") indicates an internal error has occurred
// and some invariants expected by the underlying system have not been
// satisfied. This error code is reserved for serious errors.
- Status::Internal()
+ pw::Status::Internal()
// Unavailable (gRPC code "UNAVAILABLE") indicates the service is currently
// unavailable and that this is most likely a transient condition. An error
@@ -148,17 +153,17 @@
//
// See the guidelines above for deciding between `FailedPrecondition`,
// `Aborted`, and `Unavailable`.
- Status::Unavailable()
+ pw::Status::Unavailable()
// DataLoss (gRPC code "DATA_LOSS") indicates that unrecoverable data loss or
// corruption has occurred. As this error is serious, proper alerting should
// be attached to errors such as this.
- Status::DataLoss()
+ pw::Status::DataLoss()
// Unauthenticated (gRPC code "UNAUTHENTICATED") indicates that the request
// does not have valid authentication credentials for the operation. Correct
// the authentication and try again.
- Status::Unauthenticated()
+ pw::Status::Unauthenticated()
.. attention::
@@ -199,7 +204,7 @@
.. code-block:: cpp
// An OK StatusWithSize with a size of 123.
- StatusWithSize::Ok(123)
+ StatusWithSize(123)
// A NOT_FOUND StatusWithSize with a size of 0.
StatusWithSize::NotFound()
diff --git a/pw_status/public/pw_status/status.h b/pw_status/public/pw_status/status.h
index 2842797..f9227d7 100644
--- a/pw_status/public/pw_status/status.h
+++ b/pw_status/public/pw_status/status.h
@@ -32,7 +32,7 @@
// success. It is typical to check for this value before proceeding on any
// given call across an API or RPC boundary. To check this value, use the
// `Status::ok()` member function rather than inspecting the raw code.
- PW_STATUS_OK = 0, // Use Status::Ok() in C++
+ PW_STATUS_OK = 0, // Use OkStatus() in C++
// Cancelled (gRPC code "CANCELLED") indicates the operation was cancelled,
// typically by the caller.
@@ -215,14 +215,14 @@
namespace pw {
// The Status class is a thin, zero-cost abstraction around the pw_Status enum.
-// It initializes to Status::Ok() by default and adds ok() and str() methods.
+// It initializes to OkStatus() by default and adds ok() and str() methods.
// Implicit conversions are permitted between pw_Status and pw::Status.
class Status {
public:
using Code = pw_Status;
// All of the pw_Status codes are available in the Status class as, e.g.
- // pw::Status::Ok() or pw::Status::OutOfRange().
+ // pw::OkStatus() or pw::Status::OutOfRange().
//
// These aliases are DEPRECATED -- prefer using the helper functions below.
// For example, change Status::CANCELLED to Status::Cancelled().
@@ -310,7 +310,7 @@
// Returns the Status::Code (pw_Status) for this Status.
constexpr Code code() const { return code_; }
- // True if the status is Status::Ok().
+ // True if the status is OK.
[[nodiscard]] constexpr bool ok() const { return code_ == PW_STATUS_OK; }
// Functions for checking which status this is.
@@ -370,6 +370,11 @@
Code code_;
};
+// Returns an OK status. Equivalent to Status() or Status(PW_STATUS_OK). This
+// function is used instead of a Status::Ok() function, which would be too
+// similar to Status::ok().
+[[nodiscard]] constexpr Status OkStatus() { return Status(); }
+
constexpr bool operator==(const Status& lhs, const Status& rhs) {
return lhs.code() == rhs.code();
}
diff --git a/pw_status/py/pw_status/update_style.py b/pw_status/py/pw_status/update_style.py
index 4e67941..ff59115 100755
--- a/pw_status/py/pw_status/update_style.py
+++ b/pw_status/py/pw_status/update_style.py
@@ -26,7 +26,6 @@
from pw_presubmit import git_repo
_REMAP = {
- 'OK': 'Ok',
'CANCELLED': 'Cancelled',
'UNKNOWN': 'Unknown',
'INVALID_ARGUMENT': 'InvalidArgument',
@@ -62,6 +61,9 @@
def _remap_codes(match) -> bytes:
status, code = (g.decode() for g in match.groups())
+ if status == 'OK':
+ return b'OkStatus()'
+
return f'{status}::{_REMAP[code]}()'.encode()
diff --git a/pw_status/status_test.cc b/pw_status/status_test.cc
index 4812b9d..eb45758 100644
--- a/pw_status/status_test.cc
+++ b/pw_status/status_test.cc
@@ -61,7 +61,7 @@
TEST(Status, Ok_OkIsTrue) {
static_assert(Status().ok());
static_assert(Status(PW_STATUS_OK).ok());
- static_assert(Status::Ok().ok());
+ static_assert(OkStatus().ok());
}
TEST(Status, NotOk_OkIsFalse) {
@@ -72,7 +72,7 @@
TEST(Status, Code) {
// clang-format off
static_assert(PW_STATUS_OK == Status().code());
- static_assert(PW_STATUS_OK == Status::Ok().code());
+ static_assert(PW_STATUS_OK == OkStatus().code());
static_assert(PW_STATUS_CANCELLED == Status::Cancelled().code());
static_assert(PW_STATUS_UNKNOWN == Status::Unknown().code());
static_assert(PW_STATUS_INVALID_ARGUMENT == Status::InvalidArgument().code());
@@ -94,7 +94,7 @@
TEST(Status, EqualCodes) {
static_assert(PW_STATUS_OK == Status());
- static_assert(PW_STATUS_OK == Status::Ok());
+ static_assert(PW_STATUS_OK == OkStatus());
static_assert(PW_STATUS_CANCELLED == Status::Cancelled());
static_assert(PW_STATUS_UNKNOWN == Status::Unknown());
static_assert(PW_STATUS_INVALID_ARGUMENT == Status::InvalidArgument());
@@ -133,27 +133,27 @@
}
TEST(Status, IsNotError) {
- static_assert(!Status::Ok().IsCancelled());
- static_assert(!Status::Ok().IsUnknown());
- static_assert(!Status::Ok().IsInvalidArgument());
- static_assert(!Status::Ok().IsDeadlineExceeded());
- static_assert(!Status::Ok().IsNotFound());
- static_assert(!Status::Ok().IsAlreadyExists());
- static_assert(!Status::Ok().IsPermissionDenied());
- static_assert(!Status::Ok().IsUnauthenticated());
- static_assert(!Status::Ok().IsResourceExhausted());
- static_assert(!Status::Ok().IsFailedPrecondition());
- static_assert(!Status::Ok().IsAborted());
- static_assert(!Status::Ok().IsOutOfRange());
- static_assert(!Status::Ok().IsUnimplemented());
- static_assert(!Status::Ok().IsInternal());
- static_assert(!Status::Ok().IsUnavailable());
- static_assert(!Status::Ok().IsDataLoss());
+ static_assert(!OkStatus().IsCancelled());
+ static_assert(!OkStatus().IsUnknown());
+ static_assert(!OkStatus().IsInvalidArgument());
+ static_assert(!OkStatus().IsDeadlineExceeded());
+ static_assert(!OkStatus().IsNotFound());
+ static_assert(!OkStatus().IsAlreadyExists());
+ static_assert(!OkStatus().IsPermissionDenied());
+ static_assert(!OkStatus().IsUnauthenticated());
+ static_assert(!OkStatus().IsResourceExhausted());
+ static_assert(!OkStatus().IsFailedPrecondition());
+ static_assert(!OkStatus().IsAborted());
+ static_assert(!OkStatus().IsOutOfRange());
+ static_assert(!OkStatus().IsUnimplemented());
+ static_assert(!OkStatus().IsInternal());
+ static_assert(!OkStatus().IsUnavailable());
+ static_assert(!OkStatus().IsDataLoss());
}
TEST(Status, Strings) {
EXPECT_STREQ("OK", Status().str());
- EXPECT_STREQ("OK", Status::Ok().str());
+ EXPECT_STREQ("OK", OkStatus().str());
EXPECT_STREQ("CANCELLED", Status::Cancelled().str());
EXPECT_STREQ("UNKNOWN", Status::Unknown().str());
EXPECT_STREQ("INVALID_ARGUMENT", Status::InvalidArgument().str());
@@ -215,7 +215,7 @@
EXPECT_EQ(Status::Unknown(), PassStatusFromC(Status::Unknown()));
EXPECT_EQ(Status::NotFound(), PassStatusFromC(PW_STATUS_NOT_FOUND));
- EXPECT_EQ(Status::Ok(), PassStatusFromC(Status::Ok()));
+ EXPECT_EQ(OkStatus(), PassStatusFromC(OkStatus()));
}
TEST(StatusCLinkage, TestStatusFromC) { EXPECT_EQ(0, TestStatusFromC()); }
