pw_assert: PW_CHECK_OK() macro for Status
This adds PW_CHECK_OK() for both C and C++ to assert that a Status
object or enum is OK.
Change-Id: I85e963bffc39510769feeb93e0b331f044557e7e
diff --git a/pw_assert/BUILD.gn b/pw_assert/BUILD.gn
index 0fac58d..63b61b6 100644
--- a/pw_assert/BUILD.gn
+++ b/pw_assert/BUILD.gn
@@ -48,6 +48,7 @@
"public/pw_assert/internal/assert_impl.h",
"pw_assert_test/fake_backend.h",
]
+ deps = [ dir_pw_status ]
}
pw_test("assert_backend_compile_test") {
@@ -55,6 +56,7 @@
deps = [
":pw_assert",
dir_pw_assert_backend,
+ dir_pw_status,
]
sources = [
"assert_backend_compile_test.c",
diff --git a/pw_assert/assert_backend_compile_test.c b/pw_assert/assert_backend_compile_test.c
index 599fa93..fb81a94 100644
--- a/pw_assert/assert_backend_compile_test.c
+++ b/pw_assert/assert_backend_compile_test.c
@@ -24,6 +24,7 @@
#define PW_ASSERT_USE_SHORT_NAMES 1
#include "pw_assert/assert.h"
+#include "pw_status/status.h"
#ifdef __cplusplus
#error "This file must be compiled as plain C to verify C compilation works."
@@ -182,4 +183,13 @@
CHECK_INT_LE(Add3(1, 2, 3), Add3(1, 2, 3), "INT: " FAIL_IF_DISPLAYED);
CHECK_INT_LE(x_int, y_int, "INT: " FAIL_IF_DISPLAYED_ARGS, z);
}
+
+ { // Compile tests for PW_ASSERT_OK().
+ PW_CHECK_OK(PW_STATUS_OK);
+ PW_CHECK_OK(PW_STATUS_OK, "msg");
+ PW_CHECK_OK(PW_STATUS_OK, "msg: %d", 5);
+ PW_DCHECK_OK(PW_STATUS_OK);
+ PW_DCHECK_OK(PW_STATUS_OK, "msg");
+ PW_DCHECK_OK(PW_STATUS_OK, "msg: %d", 5);
+ }
}
diff --git a/pw_assert/assert_backend_compile_test.cc b/pw_assert/assert_backend_compile_test.cc
index 47df056..372810b 100644
--- a/pw_assert/assert_backend_compile_test.cc
+++ b/pw_assert/assert_backend_compile_test.cc
@@ -40,6 +40,7 @@
#include "pw_assert/assert.h"
// clang-format on
+#include "pw_status/status.h"
#include "gtest/gtest.h"
// This is a global constant to feed into the formatter for tests.
@@ -197,6 +198,32 @@
CHECK_INT_LE(x_int, y_int, "INT: " FAIL_IF_DISPLAYED_ARGS, z);
}
+pw::Status MakeStatus(pw::Status status) { return status; }
+
+TEST(Check, CheckOkMacrosCompile) {
+ MAYBE_SKIP_TEST;
+ pw::Status status = pw::Status::UNKNOWN;
+
+ // Typical case with long names.
+ PW_CHECK_OK(status);
+ PW_CHECK_OK(status, "msg");
+ PW_CHECK_OK(status, "msg: %d", 5);
+
+ // Short names.
+ CHECK_OK(status);
+ CHECK_OK(status, "msg");
+ CHECK_OK(status, "msg: %d", 5);
+
+ // Status from a literal.
+ PW_CHECK_OK(pw::Status::OK);
+
+ // Status from a function.
+ PW_CHECK_OK(MakeStatus(pw::Status::OK));
+
+ // Status from C enums.
+ PW_CHECK_OK(PW_STATUS_OK);
+}
+
// These are defined in assert_test.c, to test C compatibility.
extern "C" void AssertBackendCompileTestsInC();
diff --git a/pw_assert/assert_facade_test.cc b/pw_assert/assert_facade_test.cc
index c4b0890..a234562 100644
--- a/pw_assert/assert_facade_test.cc
+++ b/pw_assert/assert_facade_test.cc
@@ -420,6 +420,65 @@
CHECK_INT_LE(1, 2, "msg: %d", 5);
}
+// Verify PW_CHECK_OK, including message handling.
+TEST_F(AssertFail, StatusNotOK) {
+ pw::Status status = pw::Status::UNKNOWN;
+ PW_CHECK_OK(status);
+ EXPECT_MESSAGE("Check failed: status (=UNKNOWN) == Status::OK (=OK). ");
+}
+
+TEST_F(AssertFail, StatusNotOKMessageNoArguments) {
+ pw::Status status = pw::Status::UNKNOWN;
+ PW_CHECK_OK(status, "msg");
+ EXPECT_MESSAGE("Check failed: status (=UNKNOWN) == Status::OK (=OK). msg");
+}
+
+TEST_F(AssertFail, StatusNotOKMessageArguments) {
+ pw::Status status = pw::Status::UNKNOWN;
+ PW_CHECK_OK(status, "msg: %d", 5);
+ EXPECT_MESSAGE("Check failed: status (=UNKNOWN) == Status::OK (=OK). msg: 5");
+}
+
+// Example expression for the test below.
+pw::Status DoTheThing() { return pw::Status::RESOURCE_EXHAUSTED; }
+
+TEST_F(AssertFail, NonTrivialExpression) {
+ PW_CHECK_OK(DoTheThing());
+ EXPECT_MESSAGE(
+ "Check failed: DoTheThing() (=RESOURCE_EXHAUSTED) == Status::OK (=OK). ");
+}
+
+// Note: This function seems pointless but it is not, since pw::Status::FOO
+// constants are not actually status objects, but code objects. This way we can
+// ensure the macros work with both real status objects and literals.
+pw::Status MakeStatus(pw::Status status) { return status; }
+TEST_F(AssertPass, Constant) { PW_CHECK_OK(pw::Status::OK); }
+TEST_F(AssertPass, Dynamic) { PW_CHECK_OK(MakeStatus(pw::Status::OK)); }
+TEST_F(AssertPass, Enum) { PW_CHECK_OK(PW_STATUS_OK); }
+TEST_F(AssertFail, Constant) { PW_CHECK_OK(pw::Status::UNKNOWN); }
+TEST_F(AssertFail, Dynamic) { PW_CHECK_OK(MakeStatus(pw::Status::UNKNOWN)); }
+TEST_F(AssertFail, Enum) { PW_CHECK_OK(PW_STATUS_UNKNOWN); }
+
+#if PW_ASSERT_ENABLE_DCHECK
+
+// In debug mode, the asserts should check their arguments.
+TEST_F(AssertPass, DCheckConstant) { PW_DCHECK_OK(pw::Status::OK); }
+TEST_F(AssertPass, DCheckDynamic) { PW_DCHECK_OK(MakeStatus(pw::Status::OK)); }
+TEST_F(AssertFail, DCheckConstant) { PW_DCHECK_OK(pw::Status::UNKNOWN); }
+TEST_F(AssertFail, DCheckDynamic) {
+ PW_DCHECK_OK(MakeStatus(pw::Status::UNKNOWN));
+}
+#else // PW_ASSERT_ENABLE_DCHECK
+
+// In release mode, all the asserts should pass.
+TEST_F(AssertPass, DCheckConstant) { PW_DCHECK_OK(pw::Status::OK); }
+TEST_F(AssertPass, DCheckDynamic) { PW_DCHECK_OK(MakeStatus(pw::Status::OK)); }
+TEST_F(AssertPass, DCheckConstant) { PW_DCHECK_OK(pw::Status::UNKNOWN); }
+TEST_F(AssertPass, DCheckDynamic) {
+ PW_DCHECK_OK(MakeStatus(pw::Status::UNKNOWN));
+}
+#endif // PW_ASSERT_ENABLE_DCHECK
+
// TODO: Figure out how to run some of these tests is C.
} // namespace
diff --git a/pw_assert/docs.rst b/pw_assert/docs.rst
index 5608043..b3c6452 100644
--- a/pw_assert/docs.rst
+++ b/pw_assert/docs.rst
@@ -174,22 +174,29 @@
.. attention::
- Don't use use ``PW_CHECK`` for binary comparisons!
+ Don't use use ``PW_CHECK`` for binary comparisons or status checks!
Instead, use the ``PW_CHECK_<TYPE>_<OP>`` macros. These macros enable
capturing the value of the operands, and also tokenizing them if using a
tokenizing assert backend. For example, if ``x`` and ``b`` are integers,
use instead ``PW_CHECK_INT_LT(x, b)``.
- +---------------------------------+-------------------------------------+
- | **Do NOT do this** | **Do this instead** |
- +---------------------------------+-------------------------------------+
- | ``PW_CHECK(a_int < b_int)`` | ``PW_CHECK_INT_LT(a_int, b_int)`` |
- +---------------------------------+-------------------------------------+
- | ``PW_CHECK(a_ptr <= b_ptr)`` | ``PW_CHECK_PTR_LE(a_ptr, b_ptr)`` |
- +---------------------------------+-------------------------------------+
- | ``PW_CHECK(Temp() <= 10.0)`` | ``PW_CHECK_FLOAT_LE(Temp(), 10.0)`` |
- +---------------------------------+-------------------------------------+
+ Additionally, use ``PW_CHECK_OK(status)`` when checking for a
+ ``Status::OK``, since it enables showing a human-readable status string
+ rather than an integer (e.g. ``status == RESOURCE_EXHAUSTED`` instead of
+ ``status == 5``.
+
+ +------------------------------------+-------------------------------------+
+ | **Do NOT do this** | **Do this instead** |
+ +------------------------------------+-------------------------------------+
+ | ``PW_CHECK(a_int < b_int)`` | ``PW_CHECK_INT_LT(a_int, b_int)`` |
+ +------------------------------------+-------------------------------------+
+ | ``PW_CHECK(a_ptr <= b_ptr)`` | ``PW_CHECK_PTR_LE(a_ptr, b_ptr)`` |
+ +------------------------------------+-------------------------------------+
+ | ``PW_CHECK(Temp() <= 10.0)`` | ``PW_CHECK_FLOAT_LE(Temp(), 10.0)`` |
+ +------------------------------------+-------------------------------------+
+ | ``PW_CHECK(Foo() == Status::OK)`` | ``PW_CHECK_OK(Foo())`` |
+ +------------------------------------+-------------------------------------+
.. cpp:function:: PW_CHECK_NOTNULL(ptr)
.. cpp:function:: PW_CHECK_NOTNULL(ptr, format, ...)
@@ -207,7 +214,7 @@
Foo* foo = GetTheFoo()
PW_CHECK_NOTNULL(foo);
- Bar* bar = GetSomeBar()
+ Bar* bar = GetSomeBar();
PW_CHECK_NOTNULL(bar, "Weirdly got NULL bar; state: %d", MyState());
.. cpp:function:: PW_CHECK_TYPE_OP(a, b)
@@ -348,6 +355,37 @@
| PW_DCHECK_FLOAT_NE | float | a != b | %f |
+--------------------+--------------+-----------+-----------------------+
+.. cpp:function:: PW_CHECK_OK(status)
+.. cpp:function:: PW_CHECK_OK(status, format, ...)
+.. cpp:function:: PW_DCHECK_OK(status)
+.. cpp:function:: PW_DCHECK_OK(status, format, ...)
+
+ Assert that ``status`` evaluates to ``pw::Status::OK`` (in C++) or
+ ``PW_STATUS_OK`` (in C). Optionally include a message with arguments to
+ report.
+
+ The ``DCHECK`` variants only run if ``NDEBUG`` is defined; otherwise, the
+ entire statement is removed (and the expression not evaluated).
+
+ .. code-block:: cpp
+
+ pw::Status operation_status = DoSomeOperation();
+ PW_CHECK_OK(operation_status);
+
+ // Any expression that evaluates to a pw::Status or pw_Status works.
+ PW_CHECK_OK(DoTheThing(), "System state: %s", SystemState());
+
+ // C works too.
+ pw_Status c_status = DoMoreThings();
+ PW_CHECK_OK(c_status, "System state: %s", SystemState());
+
+ .. note::
+
+ Using ``PW_CHECK_OK(status)`` instead of ``PW_CHECK(status == Status::OK)``
+ enables displaying an error message with a string version of the error
+ code; for example ``status == RESOURCE_EXHAUSTED`` instead of ``status ==
+ 5``.
+
----------------------------
Assert backend API reference
----------------------------
diff --git a/pw_assert/public/pw_assert/internal/assert_impl.h b/pw_assert/public/pw_assert/internal/assert_impl.h
index a03c1d1..7814340 100644
--- a/pw_assert/public/pw_assert/internal/assert_impl.h
+++ b/pw_assert/public/pw_assert/internal/assert_impl.h
@@ -128,6 +128,21 @@
// clang-format on
+// PW_CHECK - If condition evaluates to false, crash. Message optional.
+#define PW_CHECK_OK(status, ...) \
+ do { \
+ if (status != PW_STATUS_OK) { \
+ _PW_CHECK_OK_SELECT_MACRO(#status, \
+ pw_StatusString(status), \
+ PW_HAS_ARGS(__VA_ARGS__), \
+ __VA_ARGS__); \
+ } \
+ } while (0)
+
+#define PW_DCHECK_OK(...) \
+ if (PW_ASSERT_ENABLE_DCHECK) \
+ PW_CHECK_OK(__VA_ARGS__)
+
// =========================================================================
// Implementation for PW_CHECK
@@ -223,8 +238,6 @@
//
// The macro avoids evaluating the arguments multiple times at the cost of some
// macro complexity.
-//
-// TODO: Concat names with __LINE__; requires an extra layer of macros.
#define _PW_CHECK_BINARY_CMP_IMPL( \
argument_a, comparison_op, argument_b, type_decl, type_fmt, ...) \
do { \
@@ -242,14 +255,45 @@
} \
} while (0)
+// =========================================================================
+// Implementation for PW_CHECK_OK
+
+// TODO: Explain why we must expand another time.
+#define _PW_CHECK_OK_SELECT_MACRO( \
+ status_expr_str, status_value_str, has_args, ...) \
+ _PW_CHECK_OK_SELECT_MACRO_EXPANDED( \
+ status_expr_str, status_value_str, has_args, __VA_ARGS__)
+
+// Delegate to the macro
+#define _PW_CHECK_OK_SELECT_MACRO_EXPANDED( \
+ status_expr_str, status_value_str, has_args, ...) \
+ _PW_CHECK_OK_HAS_MSG_##has_args( \
+ status_expr_str, status_value_str, __VA_ARGS__)
+
+// PW_CHECK_OK version 1: No message or args
+#define _PW_CHECK_OK_HAS_MSG_0(status_expr_str, status_value_str, ignored_arg) \
+ PW_HANDLE_ASSERT_BINARY_COMPARE_FAILURE( \
+ status_expr_str, status_value_str, "==", "Status::OK", "OK", "%s", "")
+
+// PW_CHECK_OK version 2: With message (and maybe args)
+#define _PW_CHECK_OK_HAS_MSG_1(status_expr_str, status_value_str, ...) \
+ PW_HANDLE_ASSERT_BINARY_COMPARE_FAILURE(status_expr_str, \
+ status_value_str, \
+ "==", \
+ "Status::OK", \
+ "OK", \
+ "%s", \
+ __VA_ARGS__)
+
// Define short, usable names if requested. Note that the CHECK() macro will
// conflict with Google Log, which expects stream style logs.
-//
-// TODO(pwbug/17): Convert this to the config system when available.
#ifndef PW_ASSERT_USE_SHORT_NAMES
#define PW_ASSERT_USE_SHORT_NAMES 0
#endif
+// =========================================================================
+// Short name definitions (optional)
+
// clang-format off
#if PW_ASSERT_USE_SHORT_NAMES
@@ -281,6 +325,7 @@
#define CHECK_FLOAT_GT PW_CHECK_FLOAT_GT
#define CHECK_FLOAT_EQ PW_CHECK_FLOAT_EQ
#define CHECK_FLOAT_NE PW_CHECK_FLOAT_NE
+#define CHECK_OK PW_CHECK_OK
// Checks that are disabled if NDEBUG is not defined.
#define DCHECK PW_DCHECK
@@ -309,6 +354,7 @@
#define DCHECK_FLOAT_GT PW_DCHECK_FLOAT_GT
#define DCHECK_FLOAT_EQ PW_DCHECK_FLOAT_EQ
#define DCHECK_FLOAT_NE PW_DCHECK_FLOAT_NE
+#define DCHECK_OK PW_DCHECK_OK
#endif // PW_ASSERT_SHORT_NAMES
// clang-format on
diff --git a/pw_assert/pw_assert_test/fake_backend.h b/pw_assert/pw_assert_test/fake_backend.h
index c059056..ee7f3be 100644
--- a/pw_assert/pw_assert_test/fake_backend.h
+++ b/pw_assert/pw_assert_test/fake_backend.h
@@ -67,9 +67,13 @@
// clang-format off
// This is too hairy for clang format to handle and retain readability.
-#define PW_HANDLE_ASSERT_BINARY_COMPARE_FAILURE( \
- arg_a_str, arg_a_val, comparison_op_str, arg_b_str, arg_b_val, \
- type_fmt, message, ...) \
+#define PW_HANDLE_ASSERT_BINARY_COMPARE_FAILURE(arg_a_str, \
+ arg_a_val, \
+ comparison_op_str, \
+ arg_b_str, \
+ arg_b_val, \
+ type_fmt, \
+ message, ...) \
pw_CaptureAssert(__FILE__, \
__LINE__, \
__func__, \
diff --git a/pw_assert_basic/public/pw_assert_basic/assert_basic.h b/pw_assert_basic/public/pw_assert_basic/assert_basic.h
index fa27cbb..f2186c2 100644
--- a/pw_assert_basic/public/pw_assert_basic/assert_basic.h
+++ b/pw_assert_basic/public/pw_assert_basic/assert_basic.h
@@ -58,9 +58,13 @@
// clang-format off
// This is too hairy for clang format to handle and retain readability.
-#define PW_HANDLE_ASSERT_BINARY_COMPARE_FAILURE( \
- arg_a_str, arg_a_val, comparison_op_str, arg_b_str, arg_b_val, \
- type_fmt, message, ...) \
+#define PW_HANDLE_ASSERT_BINARY_COMPARE_FAILURE(arg_a_str, \
+ arg_a_val, \
+ comparison_op_str, \
+ arg_b_str, \
+ arg_b_val, \
+ type_fmt, \
+ message, ...) \
pw_Crash(__FILE__, \
__LINE__, \
__func__, \
