Skip to content

C API: downcase all MP_OBJ_IS_xxx macros and MP_xxx_SLOT_IS_FILLED functions - #4446

Closed
dpgeorge wants to merge 4 commits into
micropython:masterfrom
dpgeorge:py-downcase-mp-obj-is-macros
Closed

dpgeorge wants to merge 4 commits into
micropython:masterfrom
dpgeorge:py-downcase-mp-obj-is-macros

Conversation

@dpgeorge

Copy link
Copy Markdown
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
Copy Markdown
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

stinos commented Jan 30, 2019

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

pfalcon commented Feb 5, 2019

Copy link
Copy Markdown
Contributor

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

dpgeorge commented Feb 6, 2019

Copy link
Copy Markdown
Member Author

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
Copy Markdown
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