Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (master@255d74b). Learn more about missing BASE report.

@@            Coverage Diff            @@
##             master   #17843   +/-   ##
=========================================
  Coverage          ?   98.38%           
=========================================
  Files             ?      171           
  Lines             ?    22283           
  Branches          ?        0           
=========================================
  Hits              ?    21924           
  Misses            ?      359           
  Partials          ?        0           

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

Code size report:

   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

I'm happy to merge this, but it feels like a layering violation. Over time this file has gained things that depend on MP configuration variables (eg float, error message compression), and perhaps that was the wrong direction to go in. I would like misc.h to be the bottom layer, independent of everything else in py/.

Do you think that's a good idea/goal? At least that way there's a good metric as to what can go in misc.h.

Or maybe that's not really possible. In that case we should remove the comment at the top of misc.h: "a mini library of useful types and functions".

I'm happy to merge this, but it feels like a layering violation. Over time this file has gained things that depend on MP configuration variables (eg float, error message compression), and perhaps that was the wrong direction to go in.

I think it's potentially a good goal (and actually I thought this was the case before I got a compiler error and realised it wasn't true!)

I guess the question is where the parts which do depend on config go. Quite a lot would have to be moved - one of the first sections has all of the Python heap allocator functions - some of these depend on MICROPY_MALLOC_USES_ALLOCATED_SIZE, MICROPY_TRACKED_ALLOC, and MICROPY_MEM_STATS. Next there are the string functions which partially depend on MICROPY_PY_BUILTINS_STR_UNICODE, etc, etc...

(Those dependencies aren't as obvious as the ones which fail to compile if config macros are missing, but they're still dependencies - if you put py/misc.h first in the include order then you'll currently get missing or incorrect prototypes for some functions.)

Thinking a bit more, we could potentially split this into something like:

• py/c_compiler.h - has the "polyfills" for non-uniform C compiler functions like static asserts, overflow detection, etc. Depends on compiler flags but not config headers.
• py/py_environ.h - Library of utility functions for the Python runtime environment (internal heap, float ops, basic string ops, etc). Depends on config headers.

(Names I've come up with here are not good, but I can't think of better ones right now.)

I'm not sure whether such a restructure has much long-term benefit, though.

Thinking a bit more, we could potentially split this into something like:
...
I'm not sure whether such a restructure has much long-term benefit, though.

It is a bit of effort and churn for seemingly little gain, although at the moment misc.h is a bit of a grab-bag of definitions.

Is this a blocking issue? Can we discuss it later?

Is this a blocking issue? Can we discuss it later?

Definitely can be left to later, but suggest we merge this now in the meantime as it makes the existing implicit relationship explicit.