-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
Move to guarded includes #1022
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
Comments
Regarding the underscores: some would say it's reserved for system headers only but I'm not sure it's worth caring about. Also probably What would be the usecase for 4? It goes against the standard practice of including only what is strictly required (well, unless one requires everything of course), and is an extra maintainance burden. |
I thought someone would mention this :) We don't need them, since
Yes, good point. Having MICROPY in the macro already gives us a unique namespace, but we still need to distinguish extmod from py directories. |
To make it easy for someone to embed uPy by included 1 header that has everything you need. Guided by Lua, we want the barrier to entry to embedding uPy to be as low as possible. |
Makes sense. |
There's some numbering issues. I guess we should go for 1,1,2 (i.e. first thress) asap. 4 is nice, yes. Renaming a dir can wait for decision until after #704 imho ;-). |
1, 1 and 2 have been fully implemented in a series of commits starting with 51dfcb4. |
- Use a single file env.props for defining the main directories used when building. env.props resolves the base directory and defines overridable output directories, and is used by all other build files. - Fix the build currently failing, basically because the preprocessing command for generating qstrdefs uses different include directories than the build itself does. (specifically, qstrdefs.h uses #include "py/mpconfig.h" since the fixes for micropython#1022 in 51dfcb4, so we need to use the base directory as include directory, not the py dir itself). So define a single variable containing the include directories instead and use it where needed.
nrf: Fix BLE on nRF52840 after adding the USB functionality
It was inevitable that we would have to move away from Plan 9 include style to standard guarded includes :) It was good while it lasted, and lasted longer than expected.
Proposal for the change:
__MICROPY_INCLUDED_xxx_H__
, and pattern:where
xxx
is the filename in upper case without the .h. The leading and trailing double underscore are just my preference/habit and we may not want them.#include
'ing a uPy header we need to prefix it with the uPy directory, like:#include "py/obj.h"
. This is because we have very generic header names (misc.h, obj.h, bc.h, runtime.h, lexer.h) that can easily clash with other C code. By doing "py/obj.h" we don't need the py/ directory in the list of include search directories (we just need the root dir), and so the uPy headers won't clash with external headers.The text was updated successfully, but these errors were encountered: