Skip to content

Commit

Permalink
pw_assert: Update PW_CHECK_OK() to handle any expr convertible to Status
Browse files Browse the repository at this point in the history
Bug: 357682413
Bug: 365592494
Change-Id: Ie6a08a9c743eef386ece545cfcd5871fd9ffe45f
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/234820
Lint: Lint 🤖 <[email protected]>
Commit-Queue: Jonathon Reinhart <[email protected]>
Reviewed-by: Wyatt Hepler <[email protected]>
  • Loading branch information
JonathonReinhart authored and CQ Bot Account committed Sep 9, 2024
1 parent 48d6e60 commit cfcf005
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 15 deletions.
1 change: 1 addition & 0 deletions pw_assert/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ pw_cc_test(
"//pw_assert",
"//pw_compilation_testing:negative_compilation_testing",
"//pw_preprocessor",
"//pw_result",
"//pw_span",
"//pw_status",
"//pw_string",
Expand Down
1 change: 1 addition & 0 deletions pw_assert/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ pw_test("assert_facade_test") {
]
deps = [
":pw_assert",
dir_pw_result,
dir_pw_span,
dir_pw_status,
dir_pw_string,
Expand Down
24 changes: 24 additions & 0 deletions pw_assert/assert_facade_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
// clang-format on

#include "pw_compilation_testing/negative_compilation.h"
#include "pw_result/result.h"
#include "pw_status/status.h"
#include "pw_status/status_with_size.h"
#include "pw_unit_test/framework.h"

namespace {
Expand Down Expand Up @@ -530,6 +532,28 @@ TEST_F(AssertFailTest, StatusNotOKMessageArguments) {
EXPECT_MESSAGE("Check failed: status (=UNKNOWN) == OkStatus() (=OK). msg: 5");
}

TEST_F(AssertPassTest, StatusWithSizeOK) {
pw::StatusWithSize status_size = pw::StatusWithSize(123);
PW_CHECK_OK(status_size);
}

TEST_F(AssertFailTest, StatusWithSizeNotOK) {
pw::StatusWithSize status_size = pw::StatusWithSize::Unknown();
PW_CHECK_OK(status_size);
EXPECT_MESSAGE("Check failed: status_size (=UNKNOWN) == OkStatus() (=OK). ");
}

TEST_F(AssertPassTest, ResultOK) {
pw::Result<int> result = 123;
PW_CHECK_OK(result);
}

TEST_F(AssertFailTest, ResultNotOK) {
pw::Result<int> result = pw::Status::Unknown();
PW_CHECK_OK(result);
EXPECT_MESSAGE("Check failed: result (=UNKNOWN) == OkStatus() (=OK). ");
}

// Example expression for the test below.
pw::Status DoTheThing() { return pw::Status::ResourceExhausted(); }

Expand Down
10 changes: 10 additions & 0 deletions pw_assert/docs.rst
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ invoke to assert. These macros are found in the ``pw_assert/check.h`` header.
Additionally, use ``PW_CHECK_OK(status)`` when checking for an OK status,
since it enables showing a human-readable status string rather than an
integer (e.g. ``status == RESOURCE_EXHAUSTED`` instead of ``status == 5``.
This works with any expression convertible to ``pw::Status``, including
``pw::StatusWithString`` and ``pw::Result<T>``.

+------------------------------------+-------------------------------------+
| **Do NOT do this** | **Do this instead** |
Expand Down Expand Up @@ -371,6 +373,10 @@ invoke to assert. These macros are found in the ``pw_assert/check.h`` header.
``PW_STATUS_OK`` (in C). Optionally include a message with arguments to
report.

``status`` can be a ``pw::Status``, or (in C++ only) any expression
convertible to ``pw::Status``, including ``pw::StatusWithString`` and
``pw::Result<T>``.

The ``DCHECK`` variants only run if ``PW_ASSERT_ENABLE_DEBUG`` is defined;
otherwise, the entire statement is removed (and the expression not evaluated).

Expand All @@ -382,6 +388,10 @@ invoke to assert. These macros are found in the ``pw_assert/check.h`` header.
// Any expression that evaluates to a pw::Status or pw_Status works.
PW_CHECK_OK(DoTheThing(), "System state: %s", SystemState());
// pw::Result<T> works.
pw::Result<int> result = GetSomething();
PW_CHECK_OK(result);
// C works too.
pw_Status c_status = DoMoreThings();
PW_CHECK_OK(c_status, "System state: %s", SystemState());
Expand Down
38 changes: 23 additions & 15 deletions pw_assert/public/pw_assert/internal/check_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,27 +144,35 @@

// clang-format on

// PW_CHECK_OK - If condition does not evaluate to PW_STATUS_OK, crash. Message
// optional.
#define PW_CHECK_OK(expression, ...) \
do { \
const _PW_CHECK_OK_STATUS _pw_assert_check_ok_status = (expression); \
if (_pw_assert_check_ok_status != PW_STATUS_OK) { \
_PW_CHECK_BINARY_ARG_HANDLER( \
#expression, \
pw_StatusString(_pw_assert_check_ok_status), \
"==", \
"OkStatus()", \
"OK", \
"%s", \
"" __VA_ARGS__); \
} \
// PW_CHECK_OK - If expression does not evaluate to PW_STATUS_OK, crash.
// Message optional.
//
// In C++, expression must evaluate to a value convertible to Status via
// pw::internal::ConvertToStatus().
//
// In C, expression must evaluate to a pw_Status value.
#define PW_CHECK_OK(expression, ...) \
do { \
const _PW_CHECK_OK_STATUS _pw_assert_check_ok_status = \
_PW_CHECK_OK_TO_STATUS(expression); \
if (_pw_assert_check_ok_status != PW_STATUS_OK) { \
_PW_CHECK_BINARY_ARG_HANDLER( \
#expression, \
pw_StatusString(_pw_assert_check_ok_status), \
"==", \
"OkStatus()", \
"OK", \
"%s", \
"" __VA_ARGS__); \
} \
} while (0)

#ifdef __cplusplus
#define _PW_CHECK_OK_STATUS ::pw::Status
#define _PW_CHECK_OK_TO_STATUS(expr) ::pw::internal::ConvertToStatus(expr)
#else
#define _PW_CHECK_OK_STATUS pw_Status
#define _PW_CHECK_OK_TO_STATUS(expr) (expr)
#endif // __cplusplus

#define PW_DCHECK_OK(...) \
Expand Down

0 comments on commit cfcf005

Please sign in to comment.