py/obj: Add comments explaining the slot index scheme. #9373
Conversation
|
@stinos I have also updated the wiki with some notes on using these macros. https://github.com/micropython/micropython/wiki/Build-Troubleshooting I'm curious, what was the scenario where you needed to use |
Codecov Report
@@ Coverage Diff @@
## master #9373 +/- ##
=======================================
Coverage 98.35% 98.35%
=======================================
Files 156 156
Lines 20506 20506
=======================================
Hits 20169 20169
Misses 337 337
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
looks good!
I'm building types dynamically, sort of like in
Yeah I almost added it myself but then reasoned it might not see much usage. On the other hand, having to repeat the index is a pretty good recipe for bugs.. |
py/obj.h
Outdated
| #define MP_OBJ_TYPE_HAS_SLOT(t, f) ((t)->slot_index_##f) | ||
| #define MP_OBJ_TYPE_GET_SLOT(t, f) (_MP_OBJ_TYPE_SLOT_TYPE_##f(t)->slots[(t)->slot_index_##f - 1]) | ||
| #define MP_OBJ_TYPE_GET_SLOT_OR_NULL(t, f) (_MP_OBJ_TYPE_SLOT_TYPE_##f(MP_OBJ_TYPE_HAS_SLOT(t, f) ? MP_OBJ_TYPE_GET_SLOT(t, f) : NULL)) | ||
| #define MP_OBJ_TYPE_SET_SLOT(t, f, v, n) ((t)->slot_index_##f = (n) + 1, (t)->slots[(t)->slot_index_##f - 1] = (void *)v) | ||
| #define MP_OBJ_TYPE_SET_SLOT(t, f, v, n) ((t)->slot_index_##f = (n) + 1, (t)->slots[n] = (void *)v) |
There was a problem hiding this comment.
I don't think it matters functionally but for consistency use slots[(n)] ?
There was a problem hiding this comment.
Yes, good to always wrap macro args in parenthesis.
Also, does this make any functional change? The macro arg n is now evaluated twice.
There was a problem hiding this comment.
It is always invoked with a literal.
There was a problem hiding this comment.
OK. But still worth wrapping n in parenthesis, if only for consistency with other macros.
Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
ffafb1f to
cd5137a
Compare
|
Thanks, merged in d75c7e8 |
Based on suggestions from @stinos in #8813 (comment)