Skip to content

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

Closed

Conversation

dpgeorge
Copy link
Member

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:

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

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.
@robert-hh
Copy link
Contributor

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.

@stinos
Copy link
Contributor

stinos commented Jan 30, 2019

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.

@dpgeorge
Copy link
Member Author

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.

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

@pfalcon
Copy link
Contributor

pfalcon commented Feb 5, 2019

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.

@dpgeorge
Copy link
Member Author

dpgeorge commented Feb 6, 2019

I'd say that if this was done at all, it should be merged soon.

I do intend to merge it soon. But because it's a wide reaching change I wanted to give a good chance for feedback.

@dpgeorge
Copy link
Member Author

Ok, merged in eee1e88 through 6e30f96

@dpgeorge dpgeorge closed this Feb 12, 2019
@dpgeorge dpgeorge deleted the py-downcase-mp-obj-is-macros branch February 12, 2019 03:58
tannewt pushed a commit to tannewt/circuitpython that referenced this pull request Mar 23, 2021
Switch RP2040 flash settings to nvm.toml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFC: Cleaning up macro naming, possibly switch to inline functions
4 participants