Skip to content

Conversation

AJMansfield
Copy link
Contributor

@AJMansfield AJMansfield commented Oct 3, 2025

Summary

This PR updates the definition of MP_DEFINE_CONST_OBJ_TYPE to generate a compile-time error on code that attempts to define a slot function with an incompatible function signature, and rewrites the code generation script that produces its internal MP_DEFINE_CONST_OBJ_TYPE_NARGS_* macros to make it easier to read and modify in this way.

The addition of the explicit (const void *) casts also resolves an issue compiling with IAR, where an implicit cast from a function pointer to a void pointer is disallowed, based on comments from @hammondeggs in the MicroPython discord from this conversation.

Testing

I've built and run the test suite against the Unix and RP2 ports with these patches applied, with all tests passing.

To verify that this generates the expected error, I've also tested building a defective version of MicroPython where I've added a bool_attr method to the objbool type with the mismatched declaration static void bool_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest, mp_obj_t extra). Applied to master, this defect produces no compilation warnings or test failures! With this PR, the build produces the following error, correctly identifying the problem:

In file included from ../../py/mpstate.h:35,
                 from ../../py/runtime.h:29,
                 from ../../py/objbool.c:29:
../../py/obj.h:771:37: error: cast between incompatible function types from ‘void (*)(void *, qstr,  void **, void *)’ {aka ‘void (*)(void *, long unsigned int,  void **, void *)’} to ‘void (*)(void *, qstr,  void **)’ {aka ‘void (*)(void *, long unsigned int,  void **)’} [-Werror=cast-function-type]
  771 | #define _MP_OBJ_TYPE_SLOT_TYPE_attr (mp_attr_fun_t)
      |                                     ^
../../py/obj.h:821:44: note: in definition of macro ‘MP_DEFINE_CONST_OBJ_TYPE_EXPAND’
  821 | #define MP_DEFINE_CONST_OBJ_TYPE_EXPAND(x) x
      |                                            ^
../../py/obj.h:811:533: note: in expansion of macro ‘_MP_OBJ_TYPE_SLOT_TYPE_attr’
  811 | #define MP_DEFINE_CONST_OBJ_TYPE_NARGS_5(_struct_type, _typename, _name, _flags, f1, v1, f2, v2, f3, v3, f4, v4, f5, v5) const _struct_type _typename = { .base = { &mp_type_type }, .flags = _flags, .name = _name, .slot_index_##f1 = 1, .slot_index_##f2 = 2, .slot_index_##f3 = 3, .slot_index_##f4 = 4, .slot_index_##f5 = 5, .slots = { (const void *)_MP_OBJ_TYPE_SLOT_TYPE_##f1 v1, (const void *)_MP_OBJ_TYPE_SLOT_TYPE_##f2 v2, (const void *)_MP_OBJ_TYPE_SLOT_TYPE_##f3 v3, (const void *)_MP_OBJ_TYPE_SLOT_TYPE_##f4 v4, (const void *)_MP_OBJ_TYPE_SLOT_TYPE_##f5 v5 } }
      |                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                     ^~~~~~~~~~~~~~~~~~~~~~~
../../py/obj.h:825:179: note: in expansion of macro ‘MP_DEFINE_CONST_OBJ_TYPE_NARGS_5’
  825 | #define MP_DEFINE_CONST_OBJ_TYPE_NARGS(_1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, _14, _15, _16, _17, _18, _19, _20, _21, _22, _23, _24, _25, _26, _27, _28, N, ...) MP_DEFINE_CONST_OBJ_TYPE_NARGS_##N
      |                                                                                                                                                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../py/obj.h:834:71: note: in expansion of macro ‘MP_DEFINE_CONST_OBJ_TYPE_NARGS’
  834 | #define MP_DEFINE_CONST_OBJ_TYPE(...) MP_DEFINE_CONST_OBJ_TYPE_EXPAND(MP_DEFINE_CONST_OBJ_TYPE_NARGS(__VA_ARGS__, _INV, 12, _INV, 11, _INV, 10, _INV, 9, _INV, 8, _INV, 7, _INV, 6, _INV, 5, _INV, 4, _INV, 3, _INV, 2, _INV, 1, _INV, 0)(mp_obj_type_t, __VA_ARGS__))
      |                                                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../py/objbool.c:91:1: note: in expansion of macro ‘MP_DEFINE_CONST_OBJ_TYPE’
   91 | MP_DEFINE_CONST_OBJ_TYPE(
      | ^~~~~~~~~~~~~~~~~~~~~~~~
CC ../../py/objgenerator.c
cc1: all warnings being treated as errors

This is perhaps a bit wordier than ideal, but for an error generated deep within a nested macro expansion it's still pretty readable.

I lack the infrastructure to test IAR builds myself, but @hammondeggs has reported that this PR does resolve their issue.

Trade-offs and Alternatives

This does increase the source code's bulk and makes the macros themselves even wordier, but with the refactored generator script it's still an improvement to clarity.

The original from 3ac8b58 was an
impressive code golf, but it's not exactly easy to understand or
modify. There's really no need for it, see also micropython#14238.

Signed-off-by: Anson Mansfield <amansfield@mantaro.com>
Signed-off-by: Anson Mansfield <amansfield@mantaro.com>
This resolves an issue compiling with IAR, where an implicit cast from a
function pointer to a void pointer is disallowed.

Based on comments from @hammondeggs in the MicroPython discord:
https://discord.com/channels/574275045187125269/1012726673709469818/1423683501374308392

Signed-off-by: Anson Mansfield <amansfield@mantaro.com>
@AJMansfield AJMansfield changed the title py/obj: Explicit cast MP_DEFINE_CONST_OBJ_TYPE slot values. py/obj: Explicitly cast MP_DEFINE_CONST_OBJ_TYPE slot values. Oct 3, 2025
Copy link

codecov bot commented Oct 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.39%. Comparing base (a79e1fd) to head (71832a1).
⚠️ Report is 45 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #18201   +/-   ##
=======================================
  Coverage   98.39%   98.39%           
=======================================
  Files         171      171           
  Lines       22276    22276           
=======================================
  Hits        21918    21918           
  Misses        358      358           

☔ 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.

Copy link

github-actions bot commented Oct 3, 2025

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

@AJMansfield
Copy link
Contributor Author

AJMansfield commented Oct 3, 2025

unix/qemu_arm test failure on 568fbb4 is spurious (thread/stress_heap.py).

@AJMansfield AJMansfield marked this pull request as ready for review October 3, 2025 17:20
By explicitly casting to the slot type before casting to a void pointer,
this ensures that MP_DEFINE_CONST_OBJ_TYPE definitions can only define
function slots with functions of the correct signature, or produce a
compile-time error otherwise.

Signed-off-by: Anson Mansfield <amansfield@mantaro.com>
@AJMansfield AJMansfield force-pushed the ungolf-const-obj-type branch from 568fbb4 to 71832a1 Compare October 3, 2025 17:50
@hammondeggs
Copy link

This has fixed this issue that I ran into when attempting to compile in IAR. Thank you!

@dpgeorge dpgeorge added the py-core Relates to py/ directory in source label Oct 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

py-core Relates to py/ directory in source

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants