mp_obj_new_bool() vs MP_BOOL() #1494
Comments
|
I'd say we use the inline implementation of MP_BOOL and rename it mp_obj_new_bool. |
|
This wouldn't follow pattern we use for other simple types - we use upper case for "inline" versions, e.g. MP_OBJ_NEW_SMALL_INT(). So, the most consistent change would be to make it MP_OBJ_NEW_BOOL(). Of course, this is the "biggest" change, and any other change would be ok too (even just removing mp_obj_new_bool() declaration), but I'd like to be sure there's consistency in it. |
|
But small int and qstr are upper case because they deal with underlying representation of the object as a word. Construction of a bool is more like construction of a proper type than of an int. It's unfortunate that we have (do we need to have?) upper and lower case names with a blurry border between what is and is not upper case. Whether something is implemented as a macro or inline function is an implementation detail and arguably should not leak into the API naming scheme. For example, soon I'll implement float stuffing and NaN boxing and then mp_obj_float_get and mp_obj_new_float become macros/inline functions, and are in the same class as small int and qstr handling functions. But I wouldn't want to change their case. |
|
I thought they're upper-case because they are macros and just follow standard C naming conventions of having macros in upper-case. In current situation on its own isn't that bad, as it will become when we start thinking about public API for external modules. So, if you think that MP_BOOL() should be renamed to mp_obj_new_bool() throughout the code, I'll go for it, especially that's how I used it in modjni.c (which is now broken in mainline thus, and the right direction of fixing it what brought this question). |
|
Yes, the reason (originally) for upper case is to stay with time-honoured C tradition: upper case = macro = possibly nasty side effects. The difficulty with our situation is 1) we want a clean API; 2) whether a function is macro/inline/normal-function depends on the object representation and so is not fixed. And when changing the implementation we shouldn't change the name. Ideal solution: switch everything to lower case and use inline functions. We tried already to go to inline functions which seems totally sensible, but that didn't work (see #293). Well, maybe with latest gcc version things have improved. If not we could try a decent compromise: inline function only when argument(s) are evaluated more than once, otherwise macro. Then everything can be lower case. We should decide on a long term goal and gradually update the functions when appropriate, so as to not need to rename things over and over. |
|
Fixed by #1502. |
add displayio to TG-Techie's first board
mp_obj_new_bool() is declared, but not defined. MP_OBJ_NEW_BOOL() which would conform to naming conventions, doesn't exist. Instead, there's MP_BOOL(). Should it be like that?
The text was updated successfully, but these errors were encountered: