-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
C API: downcase all MP_OBJ_IS_xxx macros and MP_xxx_SLOT_IS_FILLED functions #4446
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
C API: downcase all MP_OBJ_IS_xxx macros and MP_xxx_SLOT_IS_FILLED functions #4446
Conversation
These macros could in principle be (inline) functions so it makes sense to have them lower case, to match the other C API functions. The remaining macros that are upper case are: - MP_OBJ_TO_PTR, MP_OBJ_FROM_PTR - MP_OBJ_NEW_SMALL_INT, MP_OBJ_SMALL_INT_VALUE - MP_OBJ_NEW_QSTR, MP_OBJ_QSTR_VALUE - MP_OBJ_FUN_MAKE_SIG - MP_DECLARE_CONST_xxx - MP_DEFINE_CONST_xxx These must remain macros because they are used when defining const data (at least, MP_OBJ_NEW_SMALL_INT is so it makes sense to have MP_OBJ_SMALL_INT_VALUE also a macro). For those macros that have been made lower case, compatibility macros are provided for the old names so that users do not need to change their code immediately.
I am sure that this will confuse me, because then it is harder to tell, what's a Macro (or define) and what is a function. |
That can be taken care of by a text editor, but I think cases where you really need to know whether something is a function or a macro are rather rare in comparison with when you just use them, so the consistency and readability in this case imo make it worth it. |
At the moment it's not consistent that uppercase names are macros, some are indeed inline functions. And others may be switched to inline functions (or real functions) in the future (see #293 for further discussion about this point). So, given that there's no clear distinction between macro and (inline) function, it seems that the best thing to do is just make everything consistently lowercase. Then you know that it's always just lowercase. (EXCEPT for those which MUST be macros (see above).) |
I'd say that if this was done at all, it should be merged soon. Otherwise this becomes yet another ever-backlogged, hard to maintain refactor. Indeed, there's one conflict already. |
I do intend to merge it soon. But because it's a wide reaching change I wanted to give a good chance for feedback. |
Switch RP2040 flash settings to nvm.toml
This is an attempt to resolve #293 by downcasing most of the uppercase macros/functions, to clean up the C API.
These macros/inline-functions that have been downcased could in principle be (inline) functions so it makes sense to have them lower case, to match the other C API functions.
The remaining macros that are upper case are:
These must remain macros because they are used when defining const data (at least, MP_OBJ_NEW_SMALL_INT is so it makes sense to have MP_OBJ_SMALL_INT_VALUE also a macro).
For those macros that have been made lower case, compatibility macros are provided for the old names so that users do not need to change their code immediately.