Codecov Report

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

@@           Coverage Diff           @@
##           master   #18148   +/-   ##
=======================================
  Coverage   98.38%   98.39%           
=======================================
  Files         171      171           
  Lines       22287    22276   -11     
=======================================
- Hits        21928    21918   -10     
+ Misses        359      358    -1     

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

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

The added define logic here is quite complex (although well documented with comments!). In light of the related discussion in #18145, can things be simplified here?

And since these macros aren't actually used anywhere (yet), is it really worth adding them?

The added define logic here is quite complex (although well documented with comments!). In light of the related discussion in #18145, can things be simplified here?

Probably at least a bit! If we're not aiming for -Wundef-clean, the conditions can definitely at least be made shorter.

And actually, one other reference it's only now occurred to me to check, the the implementation of BOOST_ALIGNMENT from boost::config do seem to find the alignas(x) and __declspec(align(x)) cases adequate for every C++ compiler they support (even if their scope is only C++).

An important difference between this and MP_STATIC_ASSERT, though, is what the fallback behavior means for program correctness. Using ((void)sizeof(char[1 - 2 * !(cond)])) as our static assert just means a less-helpful error message, it's not incorrect to use even if for some particular compiler _Static_assert or static_assert could have been used. Whereas, alignment not being correct could actually be a correctness bug.

Maybe a reason this should actually not provide a base case? I wrote it with one originally as my original usage was just for a correctness-noncritical performance tweak, but perhaps it's still better to emit a warning/error.

And since these macros aren't actually used anywhere (yet), is it really worth adding them?

Definitely an open question! There's a few other PRs like #18194 that would want to use this, and it'll at least make it more easily available for experiments like some of the early versions of #18147.

I'll also note that there's several usages across existing code of __attribute__((aligned(...))), etc --- I'll have to see about adding on another commit with a refactor for as many as I can find. (i.e. to help signpost these macros as the future preferred way to do that; and to help demonstrate/verify there's no missing corner cases, at least in CI.)