MP_STATIC_ASSERT: Use _Static_assert or c++ static_assert

[jepler]

Summary

When _Static_assert can be used, the diagnostic quality is improved:

../py/objint.c: In function ‘mp_obj_int_make_new’:
../py/misc.h:67:32: error: static assertion failed: "37 == 42"
../py/objint.c:45:5: note: in expansion of macro ‘MP_STATIC_ASSERT’

As compared to a diagnostic about

../py/misc.h:71:50: error: size of unnamed array is negative

Related: #18116

Testing

I built the unix coverage build locally. Since this is a compile-time only test, CI will be a good test.

Trade-offs and Alternatives

Testing on godbolt indicated that this actually works back to gcc 4.5, but it's easier to use GNUC >= 5 as the test; Hypothetical users of 4.5, 4.6, or 4.7 will just get slightly worse diagnostics.

For "nonconstant" static asserts, the array trick still needs to be used; _Static_assert rejects these expressions just like C++ static_assert does.

[github-actions]

Code size report:

  mpy-cross:    +0 +0.000% 
   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.38%. Comparing base (adf6319) to head (7077916).
⚠️ Report is 61 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #18145   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files         171      171           
  Lines       22287    22287           
=======================================
  Hits        21928    21928           
  Misses        359      359           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[dpgeorge]

For "nonconstant" static asserts, the array trick still needs to be used; _Static_assert rejects these expressions just like C++ static_assert does.

Hmm, so this probably won't fix #18116.

Maybe you can bring in #18129 to this PR just to test if it does or does not fix the clang 17 issue?

[jepler]

Based on some testing in godbolt, the pragma to disable the warning flag is accepted without complaint (under -Werror) even in extremely old clang versions.

[jepler]

[the commit message check is failing because there's the merge commit for #18129 testing]

[bikeNomad]

This fixes #18116 for me.

[projectgus]

Thanks @jepler, I started down the same path as you but got stuck on what to do about the MP_STATIC_ASSERT_NONCONSTEXPR case. I think this is the right way forward.

I wonder if we should leave macos-26 in place, presumably at some point this will become macos-latest anyway?

[AJMansfield]

There's also a few other tests that should probably be added --- for the static_assert spelling, you can add in C23, clang __has_feature(cxx_static_assert), and the assert.h macro of that name that existed for C11; and C++ should be limited to specifically C++11 or later. For the _Static_assert spelling you can also add in clang feature __has_feature(c_static_assert).

I did something very similar to this for #18148, with the C23 alignas and alignof keywords which are in a very similar state --- albeit even more fragmented.

[AJMansfield]

Suggested change

	#if __cplusplus
	#if (defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 202311L)) || (defined(__cplusplus) && (__cplusplus >= 201103L)) || __has_feature(cxx_static_assert) || defined(static_assert)
	// C23 keyword: https://en.cppreference.com/w/c/language/static_assert.html
	// C++11 declaration: https://en.cppreference.com/w/cpp/language/static_assert.html
	// Clang feature: https://clang.llvm.org/docs/LanguageExtensions.html#c-11-static-assert
	// assert.h macro: https://en.cppreference.com/w/c/error/static_assert.html

[AJMansfield]

Suggested change

	#elif __GNUC__ >= 5 || __STDC_VERSION__ >= 201112L
	#elif (defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L)) || (defined(__GNUC__) && __GNUC__ >= 5) || __has_feature(c_static_assert)
	// C11 keyword: https://en.cppreference.com/w/c/language/static_assert.html
	// GCC feature: https://www.gnu.org/software/c-intro-and-ref/manual/html_node/Static-Assertions.html
	// Clang feature: https://clang.llvm.org/docs/LanguageExtensions.html#c11-static-assert

[AJMansfield]

I'll also note that MSVC has the macro _STATIC_ASSERT from <crtdbg.h> that could potentially be added for pre-C11 MSVC compatibility --- though I'd conjecture that that actually just expands to more-or-less the same thing anyway.

[jepler]

@AJMansfield I gently disagree with your proposed change to the enabling condition, but I also feel like we've discussed it before and I don't mean to rehash old settled topics. If this rings a bell do you recall specifics or have a link to the old discussion? I only found #17735 (review) in my search.

gcc (actually cpp) documentation says that "__has_feature and __has_extension are not recommended for use in new code".

As an extension, gcc has long offered support for _Static_assert in C99 mode, at least since gcc 4.5, but until recent versions (gcc 14?) does not support __has_feature. So, concretely, I think your proposed change would prevent use of _Static_assert in gcc -std=c99 before gcc 14 including most of the CI unix builds. (but read on, I think it never works)

Similar to how we ended up using version checks for MP_GCC_HAS_BUILTIN_OVERFLOW (but supplemented it with some __has_builtin calls, which maybe improves clang compat?) I think we can't purely use __has_feature or STDC_VERSION checks if we want the widest use of _Static_assert. Do you see value in adding the __has_feature check rather than entirely replacing the __GNUC__ check?

I'm also not sure if __has_feature is even the right check for gcc.

To check my theories I tried this on godbolt across a range of gcc versions with -std=c99:

// like py/misc.h
#ifndef __has_feature
#define __has_feature(x) (0)
#endif

#ifndef __has_extension
#define __has_extension(x) (0)
#endif

#if __has_feature(c_static_assert)
#warning "has c_static_assert (feature)"
#endif


#if __has_extension(c_static_assert)
#warning "has c_static_assert (extension)"
#endif

_Static_assert(1, "it is thus");

gcc 4.7, 5.1, ..., 12.5, 13.4 build with no diagnostics.
gcc 14.3 says <source>:17:2: warning: #warning "has c_static_assert (extension)" [-Wcpp].
clang all the way back to 3.0.0 says <source>:17:2: warning: #warning "has c_static_assert (extension)" [-W#warnings]

So, I actually didn't find any compiler versions where the has_feature check would enable use of static assert.

The other positive thing I see your version doing is fixing how things work under -Wundef. This is probably a good idea; circuitpython, at least, likes to build with -Werror=undef. Has requiring -Wundef clean builds been considered in micropython?

[AJMansfield]

@AJMansfield I gently disagree with your proposed change to the enabling condition, but I also feel like we've discussed it before and I don't mean to rehash old settled topics. If this rings a bell do you recall specifics or have a link to the old discussion? I only found #17735 (review) in my search.

I don't believe it's a conversation I've had --- glad for the reference on how MicroPython usually conducts things though.

gcc (actually cpp) documentation says that "__has_feature and __has_extension are not recommended for use in new code".

As an extension, gcc has long offered support for _Static_assert in C99 mode, at least since gcc 4.5, but until recent versions (gcc 14?) does not support __has_feature.

Indeed, my proposal was a bit premature, I should've spent a bit more time reading and testing things first 🙃. The only (possible) benefit there might be to do with clang support.

Also useful note on the __has_feature vs __has_extension situation, I'll have to see if I can make a similar swap to extend my alignas and alignof macros further back.

The other positive thing I see your version doing is fixing how things work under -Wundef. This is probably a good idea; circuitpython, at least, likes to build with -Werror=undef. Has requiring -Wundef clean builds been considered in micropython?

Agreed --- and it's not that I'd expect anyone with a stupid-mode compiler that actually breaks on undefined macros, but if there's anywhere to do belt-and-braces on that it's the macros designed to bridge compatibility gaps between different compiler versions.

[dpgeorge]

I think what's in this PR at the moment is fine: it works, is an improvement on the diagnostics for failing static assertions, and fixes issue #18116.

Further improvements can be made later if needed.

[dpgeorge]

Has requiring -Wundef clean builds been considered in micropython?

Yes, there was a discussion about this somewhere. I'd be happy if we were able to turn that warning on (although I guess it might require quite a few changes...).

[dpgeorge]

Rebased and merged in 099991f, 33cf1ab and d7cb54b

[dhalbert]

We also had to do this in CircuitPython: adafruit#10656. But #if __cplusplus and #if __clang__ fail when warnings are more strict if they are not defined. So had to make these #if defined(...) instead.