pw_assert: Add design discussion to docs
This enhances the assert documentation with information about tradeoffs
considered while designing the module. Additionally, provides some
guidance to be more explicit that PW_CHECK(a < b) shouldn't be used, and
instead PW_CHECK_INT_LT(a, b) should be used.
Change-Id: I421700bfa85cb34c874c999245c201cb3337ebb4
diff --git a/pw_assert/docs.rst b/pw_assert/docs.rst
index 97790db..5608043 100644
--- a/pw_assert/docs.rst
+++ b/pw_assert/docs.rst
@@ -42,10 +42,6 @@
PW_CHECK_INT_LE(ItemCount(), 100);
PW_CHECK_INT_LE(ItemCount(), 100, "System state: %s", GetStateStr());
-Compatibility
--------------
-The facade is compatible with C and C++.
-
Example
-------
@@ -79,6 +75,62 @@
// The functions ItemCount() and GetStateStr() are never called.
PW_DCHECK_INT_LE(ItemCount(), 100, "System state: %s", GetStateStr());
+Design discussion
+-----------------
+The Pigweed assert API was designed taking into account the needs of several
+past projects the team members were involved with. Based on those experiences,
+the following were key requirements for the API:
+
+1. **C compatibility** - Since asserts are typically invoked from arbitrary
+ contexts, including from vendor or third party code, the assert system must
+ have a C-compatible API. Some API functions working only in C++ is
+ acceptable, as long as the key functions work in C.
+2. **Capturing both expressions and values** - Since asserts can trigger in
+ ways that are not repeatable, it is important to capture rich diagnostic
+ information to help identifying the root cause of the fault. For asserts,
+ this means including the failing expression text, and optionally also
+ capturing failing expression values. For example, instead of capturing an
+ error with the expression (``x < y``), capturing an error with the
+ expression and values(``x < y, with x = 10, y = 0``).
+3. **Tokenization compatible** - It's important that the assert expressions
+ support tokenization; both the expression itself (e.g. ``a < b``) and the
+ message attached to the expression. For example: ``PW_CHECK(ItWorks(), "Ruh
+ roh: %d", some_int)``.
+4. **Customizable assert handling** - Most products need to support custom
+ handling of asserts. In some cases, an assert might trigger printing out
+ details to a UART; in other cases, it might trigger saving a log entry to
+ flash. The assert system must support this customization.
+
+The combination of #1, #2, and #3 led to the structure of the API. In
+particular, the need to support tokenized asserts and the need to support
+capturing values led to the choice of having ``PW_CHECK_INT_LE(a, b)`` instead
+of ``PW_CHECK(a <= b)``. Needing to support tokenization is what drove the
+facade & backend arrangement, since the backend must provide the raw macros for
+asserting in that case, rather than terminating at a C-style API.
+
+Why isn't there a ``PW_CHECK_LE``? Why is the type (e.g. ``INT``) needed?
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+The problem with asserts like ``PW_CHECK_LE(a, b)`` instead of
+``PW_CHECK_INT_LE(a, b)`` or ``PW_CHECK_FLOAT_LE(a, b)`` is that to capture the
+arguments with the tokenizer, we need to know the types. Using the
+preprocessor, it is impossible to dispatch based on the types of ``a`` and
+``b``, so unfortunately having a separate macro for each of the types commonly
+asserted on is necessary.
+
+How should objects be asserted against or compared?
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Unfortunatly, there is no native mechanism for this, and instead the way to
+assert object states or comparisons is with the normal ``PW_CHECK_*`` macros
+that operate on booleans, ints, and floats.
+
+This is due to the requirement of supporting C and also tokenization. It may be
+possible support rich object comparions by defining a convention for
+stringifying objects; however, this hasn't been added yet. Additionally, such a
+mechanism would not work well with tokenization. In particular, it would
+require runtime stringifying arguments and rendering them with ``%s``, which
+leads to binary bloat even with tokenization. So it is likely that a rich
+object assert API won't be added.
+
---------------------------
Assert facade API reference
---------------------------
@@ -112,10 +164,32 @@
The ``DCHECK`` variants only run if ``NDEBUG`` is defined; otherwise, the
entire statement is removed (and the expression not evaluated).
+ Example:
+
.. code-block:: cpp
PW_CHECK(StartTurbines());
PW_CHECK(StartWarpDrive(), "Oddly warp drive couldn't start; ruh-roh!");
+ PW_CHECK(RunSelfTest(), "Failure in self test; try %d", TestAttempts());
+
+ .. attention::
+
+ Don't use use ``PW_CHECK`` for binary comparisons!
+
+ 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)`` |
+ +---------------------------------+-------------------------------------+
.. cpp:function:: PW_CHECK_NOTNULL(ptr)
.. cpp:function:: PW_CHECK_NOTNULL(ptr, format, ...)
@@ -283,31 +357,72 @@
like sys_io and halt in a while loop waiting for a debugger. In other cases,
the backend could store crash details like the current thread's stack to flash.
-This module does not provide a backend; see ``pw_assert_basic`` for a basic
-implementation (which we do not advise using in production).
+This facade module (``pw_assert``) does not provide a backend. See
+:ref:`chapter-pw-assert-basic` for a basic implementation.
-Here are the macros the backend must provide:
+The backend must provide the header
+
+``pw_assert_backend/backend.h``
+
+and that header must define the following macros:
.. cpp:function:: PW_HANDLE_CRASH(message, ...)
- The backend should trigger a system crash or halt, and if possible, deliver
- the specified message and arguments to the user or developer.
+ Trigger a system crash or halt, and if possible, deliver the specified
+ message and arguments to the user or developer.
.. cpp:function:: PW_HANDLE_ASSERT_FAILURE(condition_str, message, ...)
- This macro is invoked from ``PW_CHECK`` if condition is false. The
- application should crash with the given message and specified format
- arguments, and may optionally include the stringified condition provided in
- ``condition_str``.
+ Trigger a system crash or halt, and if possible, deliver the condition string
+ (indicating what expression was false) and the message with format arguments,
+ to the user or developer.
+
+ This macro is invoked from the ``PW_CHECK`` facade macro if condition is
+ false.
.. cpp:function:: PW_HANDLE_ASSERT_BINARY_COMPARE_FAILURE( \
a_str, a_val, op_str, b_str, b_val, type_fmt, message, ...)
- This macro is invoked from the ``PW_CHECK_*_*`` macros if the condition
- ``a_val op b_val`` is false. The facade API macros have already evaluated and
- stringified the arguments, so the backend is free to report the details as
- needed.
+ Trigger a system crash or halt for a failed binary comparison assert (e.g.
+ any of the ``PW_CHECK_<type>_<op>`` macros). The handler should combine the
+ assert components into a useful message for the user; though in some cases
+ this may not be possible.
- The backend is expected to report the failure to the user or developer in a
- useful way, potentially capturing the string and values of the binary
- comparison operands.
+ Consider the following example:
+
+ .. code-block:: cpp
+
+ int temp = 16;
+ int max_temp = 15;
+ PW_CHECK_INT_LE(temp, MAX_TEMP, "Got too hot; state: %s", GetSystemState());
+
+ In this block, the assert will trigger, which will cause the facade to invoke
+ the handler macro. Below is the meaning of the arguments, referencing to the
+ example:
+
+ - ``a_str`` - Stringified first operand. In the example: ``"temp"``.
+ - ``a_val`` - The value of the first operand. In the example: ``16``.
+ - ``op_str`` - The string version of the operator. In the example: "<=".
+ - ``b_str`` - Stringified second operand. In the example: ``"max_temp"``.
+ - ``b_val`` - The value of the second operand. In the example: ``15``.
+ - ``type_fmt`` - The format code for the type. In the example: ``"%d"``.
+ - ``message, ...`` - A formatted message to go with the assert. In the
+ example: ``"Got too hot; state: %s", "ON_FIRE"``.
+
+ .. tip::
+
+ See :ref:`chapter-pw-assert-basic` for one way to combine these arguments
+ into a meaningful error message.
+
+.. attention::
+
+ The facade macros (``PW_CRASH`` and related) are expected to behave like they
+ have the ``[[ noreturn ]]`` attribute set. This implies that the backend
+ handler functions, ``PW_HANDLE_*`` defined by the backend, must not return.
+
+ In other words, the device must reboot.
+
+-------------
+Compatibility
+-------------
+The facade is compatible with C and C++.
diff --git a/pw_assert_basic/docs.rst b/pw_assert_basic/docs.rst
index c930b79..97967f5 100644
--- a/pw_assert_basic/docs.rst
+++ b/pw_assert_basic/docs.rst
@@ -1,9 +1,9 @@
-.. _chapter-pw-assert-basic:
-
.. default-domain:: cpp
.. highlight:: cpp
+.. _chapter-pw-assert-basic:
+
===============
pw_assert_basic
===============
