Skip to content

assert: add "c-stdaux-assert.h" with c_more_assert() #16

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

thom311
Copy link
Contributor

@thom311 thom311 commented Aug 31, 2023

The standard assert() and c_assert() macros are enabled by default, allowing
debugging assertions that can be disabled by defining NDEBUG. Notably,
c_assert() always evaluates the condition, whereas assert() expects
side-effect-free conditions.

However, these macros are often active in most builds, as many projects do not
define NDEBUG, even for release builds. While assertions are essential for
debugging and testing, they do add runtime overhead. Using different levels of
assertion can help manage this. Some assertions check conditions are almost
certain to hold, such as function argument validation where code review easily
confirms invariants. In such cases, always-on assertions may not be justified,
and developers may prefer to avoid assertions that are still active in release
builds.

This is where c_more_assert() fits in. By setting C_MORE_ASSERT to 2 or
higher, the user can enable these assertions. Otherwise, they are disabled by
default, minimizing runtime cost. c_more_assert_with() extends this
functionality by allowing selective enablement at specific assertion levels
through C_MORE_ASSERT.

Compared to assert(), these macros other advantages beside being disabled by
default:

  • The compiler sees the expression regardless of NDEBUG or assertion level,
    catching potential compile errors in expressions and avoiding certain
    compiler warnings that otherwise depend on whether the assertion is enabled.
  • Constant expressions that fail always mark the code path as unreachable,
    reducing warnings on unreachable paths.

Also, as c_assert() evaluates all expressions, unreachable paths are now
recognized even with NDEBUG defined. That may allow aggressive optimization as
the compiler can reason that the assertion holds. This is what the user asks
for with NDEBUG.

Similar to <assert.h>, <c-stdaux-assert.h> supports multiple inclusions and
reconfiguration through NDEBUG and C_MORE_ASSERT.

Assertion failures now print only critical information (e.g., file:line),
omitting less useful strings unless C_MORE_ASSERT >= 2, to reduce binary size.

Additionally, C_MORE_ASSERT_LEVEL represents the effective assertion level and
can be used both as a macro and a runtime constant, allowing conditionally
compiled code:

if (C_MORE_ASSERT_LEVEL >= 5) {
    /* Elaborate checking of invariant. The compiler validates this code
     * while optimizing out the entire block due to the constant expression. */
}

… are async-signal-safe

They are very fundamental. It's important that we can use them at all places.
Document it.
@thom311 thom311 force-pushed the th/more-asserts branch 6 times, most recently from 8e1ff4f to 0f3c4c4 Compare July 12, 2024 13:22
@thom311 thom311 marked this pull request as draft November 13, 2024 11:10
@thom311 thom311 changed the title add "c-stdaux-assert.h" with more assertions assert: add "c-stdaux-assert.h" with c_more_assert() Nov 14, 2024
@thom311 thom311 marked this pull request as ready for review November 14, 2024 08:19
@thom311
Copy link
Contributor Author

thom311 commented Nov 14, 2024

@dvdhrm hi. How do you like this?

(yes, there is still a test failure, I'd address that after initial feedback).

@dvdhrm
Copy link
Member

dvdhrm commented Nov 15, 2024

My main issue is that I don't think assertion categories (or debugging categories) can be listed linearly. So instead of having debug_level = [0..N], I would prefer if you have distinct debugging groups.

As an example, I very much love lockdep in the linux-kernel, but it is so slow that you cannot enable it in a production kernel. But I like that I can enable it conditionally for development when I need it. But I wouldn't be able to decide on which debug_level to place it.

So what exactly do you envision these assertions to be used for? You could surely leave it up to each application to decide which level to use for which category of assertion, but then I would just move these macros into the individual projects, rather than having them here?

@thom311
Copy link
Contributor Author

thom311 commented Nov 15, 2024

but then I would just move these macros into the individual projects, rather than having them here?

Everything in c-stdaux can be reimplement in the individual project. It's here because it's IMO a major feature, that I would rather have at one place, where we also invest effort to get all corner cases well. As an example, a major feature is that the compiler sees the expression with c_more_assert() and thereby we can avoid certain compiler warnings. And it's not entirely trivial. I'd guess many projects that use plain assert() cannot use compiler warnings with NDEBUG because they get flooded by them. That is a problem.


Example usage:

int will_call_foo(int argument1) {
      internal_foo(argument1, 3);
      internal_foo(argument1, 5);
}

static void internal_foo(int argument1, int argument2) {

    /* argument1 is NOT obviously valid because we cannot see all callers. Always have this
     * assert enabled, even in most release builds. Maybe the user would disable it for extra
     * optimization with NDEBUG, but that is the exception. */
    assert(argument1 > 0);

   /* internal_foo() is a internal function with few callers. We can easily verify by review that
    * argument2 is valid. Don't pay the overhead in production. It's still useful to have extra
    * assertion to make the invariant visible to the reviewer, and also guard
    * against future changes of the callers. */
    c_more_assert(argument2 > 0);

    /* Maybe even more kinds of assertions, that are have higher performance overhead to evaluate.
     * We wouldn't want to slow down all debug builds (only those that want heavy assertions
     * enabled). */
    c_more_assert_with(10, expensive_check(argument2));

   /* Or for assertions that  are even more obviously correct, where a review can see it even
    * easier (e.g. it's just a consequence of a few lines earlier).
    * It still gives a peace of mind, that some CI checks this obviously correct assert.
    * 
    * It's a mix of how expensive is the check, vs. how useful is the assertion to check. */
   int arg2_quarter = argument2 / 4;
   c_more_assert_with(5, is_even(arg2_quarter));
}

All assertions are expected/required to hold. But some hold more obviously than others.

As assertions have some overhead, we may be tempted to not write assertions that "obviously" hold or are less useful. It discourages to just add a lot of assertions. While in some programming styles assertions may not be favored, IMO assertions are amazing. I want to plaster the code with assertions, but I don't want to pay the overhead in normal builds.

Assertions to me are also a documentation in code.


My main issue is that I don't think assertion categories (or debugging categories) can be listed linearly. So instead of having debug_level = [0..N], I would prefer if you have distinct debugging groups.

In NetworkManager, c_more_assert() are used all over the place (called nm_assert()). There are some uses of c_more_assert_with() (called if (NM_MORE_ASSERTS > 5) nm_assert();).

The point of C_MORE_ASSERT is to avoid the overhead. When I already run a debug build in testing, I want to enable all assertions I can affort (except those that are too expensive for my test).

As an example, I very much love lockdep in the linux-kernel, but it is so slow that you cannot enable it in a production kernel. But I like that I can enable it conditionally for development when I need it. But I wouldn't be able to decide on which debug_level to place it.

Makes sense. So you have expensive assertions, and depending on which aspect you debug/test, enable only that domain. The individual project can still do that, for specific uses.

NetworkManager does not care to micro-optimizing levels vs. damains. It just has the convention that NM_MORE_ASSERT<=10 does not hurt performance too much, and NM_MORE_ASSERT=10000 enables all levels.


The main benefit already comes by having just a the separate level (C_MORE_ASSERT>=2) that is disabled by default. Having additional levels on top of that (any C_MORE_ASSERT level) is arguably less important. But having many levels is a small step once you have already the the necessary levels 0 (aka NDEBUG), 1 (default), and >2 (disabled by default).

@thom311 thom311 force-pushed the th/more-asserts branch 2 times, most recently from cb95bfb to a9045b4 Compare November 15, 2024 14:42
The standard assert() and c_assert() macros are enabled by default, allowing
debugging assertions that can be disabled by defining NDEBUG. Notably,
c_assert() always evaluates the condition, whereas assert() expects
side-effect-free conditions.

However, these macros are often active in most builds, as many projects do not
define NDEBUG, even for release builds. While assertions are essential for
debugging and testing, they do add runtime overhead. Using different levels of
assertion can help manage this. Some assertions check conditions are almost
certain to hold, such as function argument validation where code review easily
confirms invariants. In such cases, always-on assertions may not be justified,
and developers may prefer to avoid assertions that are still active in release
builds.

This is where `c_more_assert()` fits in. By setting C_MORE_ASSERT to 2 or
higher, the user can enable these assertions. Otherwise, they are disabled by
default, minimizing runtime cost. `c_more_assert_with()` extends this
functionality by allowing selective enablement at specific assertion levels
through C_MORE_ASSERT.

Compared to assert(), these macros other advantages beside being disabled by
default:
- The compiler sees the expression regardless of NDEBUG or assertion level,
  catching potential compile errors in expressions and avoiding certain
  compiler warnings that otherwise depend on whether the assertion is enabled.
- Constant expressions that fail always mark the code path as unreachable,
  reducing warnings on unreachable paths.

Also, as c_assert() evaluates all expressions, unreachable paths are now
recognized even with NDEBUG defined. That may allow aggressive optimization as
the compiler can reason that the assertion holds. This is what the user asks
for with NDEBUG.

Similar to <assert.h>, <c-stdaux-assert.h> supports multiple inclusions and
reconfiguration through NDEBUG and C_MORE_ASSERT.

Assertion failures now print only critical information (e.g., file:line),
omitting less useful strings unless C_MORE_ASSERT >= 2, to reduce binary size.

Additionally, C_MORE_ASSERT_LEVEL represents the effective assertion level and
can be used both as a macro and a runtime constant, allowing conditionally
compiled code:

    if (C_MORE_ASSERT_LEVEL >= 5) {
        /* Elaborate checking of invariant. The compiler validates this code
         * while optimizing out the entire block due to the constant expression. */
    }

One important difference might be that c_assert() is now a do{}while(0)
statement, previously it was an expression. I don't think anybody should
care about the difference, especially since the expression previously
also returned void.
@thom311
Copy link
Contributor Author

thom311 commented Nov 15, 2024

In the branch, c_assert() changes from being an expression ( (cond) ? assert(true) : assert(false) ) to a do { ... } while(0) statement. Was it important that c_assert() was an expression? I think it shouldn't matter for most purposes, because the expression also returned void.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants