gh-118124: Use static_assert() in Py_BUILD_ASSERT() on C11#118398
gh-118124: Use static_assert() in Py_BUILD_ASSERT() on C11#118398vstinner merged 1 commit intopython:mainfrom
Conversation
|
See also PR gh-118125. |
Include/pymacro.h
Outdated
| #define Py_BUILD_ASSERT_EXPR(cond) \ | ||
| #if __STDC_VERSION__ >= 201112L | ||
| # define Py_BUILD_ASSERT_EXPR(cond) \ | ||
| (sizeof(struct { _Static_assert(cond, #cond); int dummy; }), \ |
There was a problem hiding this comment.
Defining structs in expression is not valid C++. And _Static_assert is not a keyword in C++.
There was a problem hiding this comment.
Is __STDC_VERSION__ defined in C++?
There was a problem hiding this comment.
For C++, we already have this code:
// gh-91782: On FreeBSD 12, if the _POSIX_C_SOURCE and _XOPEN_SOURCE macros are
// defined, <sys/cdefs.h> disables C11 support and <assert.h> does not define
// the static_assert() macro.
// https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=255290
//
// macOS <= 10.10 doesn't define static_assert in assert.h at all despite
// having C11 compiler support.
//
// static_assert is defined in glibc from version 2.16. Compiler support for
// the C11 _Static_assert keyword is in gcc >= 4.6.
//
// MSVC makes static_assert a keyword in C11-17, contrary to the standards.
//
// In C++11 and C2x, static_assert is a keyword, redefining is undefined
// behaviour. So only define if building as C (if __STDC_VERSION__ is defined),
// not C++, and only for C11-17.
#if !defined(static_assert) && (defined(__GNUC__) || defined(__clang__)) \
&& defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L \
&& __STDC_VERSION__ <= 201710L
# define static_assert _Static_assert
#endifThere was a problem hiding this comment.
I updated the PR to test explicitly for defined(__STDC_VERSION__).
There was a problem hiding this comment.
Is
__STDC_VERSION__defined in C++?
Ah, no. Missed that.
|
Do you prefer leaving the macros unchanged for all C++ and C99, for reducing complexity? Since you're working on this, can/should I close my PR? |
_Static_assert() was added to C11. I don't think that we can expect it to be available on C99. Is it available on C++? |
No. VLA was added to C99 (optional since C11) and is often available as compiler extension for older C versions and most C++ versions. There're tricks to also fix the two macros for them but would introduce some complexity in code, at least not as clean as your PR. |
I updated the PR for C++. It now uses static_assert() on C++11 and newer. |
|
The Docs CI job failed with: |
Use static_assert() in Py_BUILD_ASSERT() and Py_BUILD_ASSERT_EXPR() on C11 and newer and C++11 and newer. Add tests to test_cext and test_cppext.
I rebased my PR on the main branch to retrieve a fix. I also squashed my commits. |
|
@namniav: So what do you think of this approach? |
|
It looks good to me. I'm not sure whether we should fix Py_BUILD_ASSERT_EXPR for older C versions and C++, I'm ok with either though. |
|
I meant to leave a comment yesterday but somehow started a review. There's no option to remove myself from reviewers, so I clicked "approved" to "finish the review". |
An alternative is to remove it, but it seems like it's used outside by 3rd party projects. |
…hon#118398) Use static_assert() in Py_BUILD_ASSERT() and Py_BUILD_ASSERT_EXPR() on C11 and newer and C++11 and newer. Add tests to test_cext and test_cppext.
Uh oh!
There was an error while loading. Please reload this page.