Page MenuHomeFreeBSD

Document two new conventions:
Needs RevisionPublic

Authored by imp on Thu, Jan 9, 10:19 PM.



C99 initializers have been encouraged for a while.
_Static_assert is to be preferred to CTASSERT.

Diff Detail

Lint OK
No Unit Test Coverage
Build Status
Buildable 28583
Build 26625: arc lint + arc unit

Event Timeline

imp created this revision.Thu, Jan 9, 10:19 PM
julian requested changes to this revision.Thu, Jan 9, 10:21 PM
julian added a subscriber: julian.
julian added inline comments.

.. e.g. (example)..
so people don't have to go look it up to see what it is.

This revision now requires changes to proceed.Thu, Jan 9, 10:21 PM
brooks added a subscriber: brooks.Thu, Jan 9, 10:57 PM

I'd rather see use switch to static_assert since that's in C11 and C++11. It's a bit unfortunate that only C++17 has the single argument version though.

rlibby added a subscriber: rlibby.Thu, Jan 9, 11:07 PM
rlibby added inline comments.

Just my opinion: _Static_assert() is longer, uglier, and the mandatory second argument is often not useful (since it's just a string, and not e.g. a printf format). It feels weird to have regular code directly invoking reserved identifiers (underscore capital), like directly using _Bool. I'd suggest we consider just leaving CTASSERT() alone and maybe adding a new CTASSERT1() wrapper.

melifaro added inline comments.Thu, Jan 9, 11:17 PM

I'd second this: _Static_assert() looks weird as it feels like reserved macro. Additionally, having a macro able to provide an error explanation directly without encoding it in the comments is pretty useful. I'd prefer either sticking with static_assert() or introducing CTASSERT1() wrapper.

jhb added a comment.Fri, Jan 10, 1:25 AM

From here: it seems that the intention is that _Static_assert should be used publicly as static_assert() from <assert.h>. (Our assert.h defines it). This is indeed similar to _Bool vs bool where a header is used in userland (<stdbool.h>) to give the C++ name in C. What we have one for 'bool' in the kernel is to go ahead and define 'bool' in <sys/types.h> under #ifdef _KERNEL. I think something similar is probably ok in in <sys/systm.h> where we define 'static_assert' to '_Static_assert' and fix existing kernel code to use the C++ spelling.

I agree with @jhb, we should prefer static_assert using assert.h