From cfcf0059926589e26f318e29df8733e5a09c2928 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Mon, 9 Sep 2024 23:41:34 +0000 Subject: [PATCH] pw_assert: Update PW_CHECK_OK() to handle any expr convertible to Status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: 357682413 Bug: 365592494 Change-Id: Ie6a08a9c743eef386ece545cfcd5871fd9ffe45f Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/234820 Lint: Lint 🤖 Commit-Queue: Jonathon Reinhart Reviewed-by: Wyatt Hepler --- pw_assert/BUILD.bazel | 1 + pw_assert/BUILD.gn | 1 + pw_assert/assert_facade_test.cc | 24 ++++++++++++ pw_assert/docs.rst | 10 +++++ .../public/pw_assert/internal/check_impl.h | 38 +++++++++++-------- 5 files changed, 59 insertions(+), 15 deletions(-) diff --git a/pw_assert/BUILD.bazel b/pw_assert/BUILD.bazel index d1717e930f..b761c9080f 100644 --- a/pw_assert/BUILD.bazel +++ b/pw_assert/BUILD.bazel @@ -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", diff --git a/pw_assert/BUILD.gn b/pw_assert/BUILD.gn index b4978597f1..999b9bb59c 100644 --- a/pw_assert/BUILD.gn +++ b/pw_assert/BUILD.gn @@ -226,6 +226,7 @@ pw_test("assert_facade_test") { ] deps = [ ":pw_assert", + dir_pw_result, dir_pw_span, dir_pw_status, dir_pw_string, diff --git a/pw_assert/assert_facade_test.cc b/pw_assert/assert_facade_test.cc index f1224a83cb..0afc8eee35 100644 --- a/pw_assert/assert_facade_test.cc +++ b/pw_assert/assert_facade_test.cc @@ -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 { @@ -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 result = 123; + PW_CHECK_OK(result); +} + +TEST_F(AssertFailTest, ResultNotOK) { + pw::Result 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(); } diff --git a/pw_assert/docs.rst b/pw_assert/docs.rst index 8a102f1882..f402ffc630 100644 --- a/pw_assert/docs.rst +++ b/pw_assert/docs.rst @@ -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``. +------------------------------------+-------------------------------------+ | **Do NOT do this** | **Do this instead** | @@ -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``. + The ``DCHECK`` variants only run if ``PW_ASSERT_ENABLE_DEBUG`` is defined; otherwise, the entire statement is removed (and the expression not evaluated). @@ -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 works. + pw::Result result = GetSomething(); + PW_CHECK_OK(result); + // C works too. pw_Status c_status = DoMoreThings(); PW_CHECK_OK(c_status, "System state: %s", SystemState()); diff --git a/pw_assert/public/pw_assert/internal/check_impl.h b/pw_assert/public/pw_assert/internal/check_impl.h index 2f0dd473fb..fe117191b1 100644 --- a/pw_assert/public/pw_assert/internal/check_impl.h +++ b/pw_assert/public/pw_assert/internal/check_impl.h @@ -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(...) \