Repository navigation
Move #include directives before extern "C" { in internal headers #115105
Description
Feature or enhancement
Some of our internal headers (e.g., pycore_ceval.h) contain #include directives inside the extern "C" { blocks. This can cause problems if system headers are included within an extern "C" { block and the header is compiled as C++ code. For a similar issue with the public headers, see #110845.
As far as I can tell, nobody has complained about this yet, but I think we might as well address this sooner rather later.
Activity
vstinner commentedon Feb 7, 2024
Yes, it sounds like a good idea.
It took me a while, but I think that I understood the C++20 module issue: #110845 (comment)
encukou commentedon Feb 7, 2024
If someone asks, let's make them happy, but I don't think we need to do this proactively.
Internal headers should be compiled as C.
vstinner commentedon May 20, 2024
I suggest to close this issue in this case.
colesbury commentedon May 20, 2024
@albanD asked me about this a week or two ago in the context of PyTorch. I'm not sure if it's still an issue or if it's been worked around.
encukou commentedon May 21, 2024
@albanD, please speak if this is an issue for you.
I'll close this in a month if no one complains.
jansel commentedon May 21, 2024
In TorchDynamo eval_frame.c was implemented in C (as opposed to C++ like the rest of PyTorch), because CPython headers (internal/pycore_frame.h, internal/pycore_pystate.h, etc) did not work from C++.
This was a few years ago at this point, so I forget the exact error -- but the workaround was to implement that one file in C. We would prefer for that file to be implemented in C++.
albanD commentedon May 21, 2024
Thanks for considering our request.
On top of what @jansel mentioned above, I would add:
As you might imagine, this is fairly brittle (very dependent on cpython core include changes) and prevents us from using c++ as with the rest of our codebase.
vstinner commentedon May 22, 2024
For _PyWeakref_ClearRef, see capi-workgroup/decisions#25
encukou commentedon Jun 21, 2024
Is there anything that should be exposed as public (potentially unstable) API, that we could document, test and support?
If you want the compiled code to match CPython, you will want to compile it as C. Especially if you've already taught your build system to do that. I don't think the
extern "C" {itself is a major problem -- though it's likely to be the first issue in the file.From a quick look at the code, it seems that you're reaching into the kind internals that change as CPython is optimized. I'm afraid I don't have advice for you other than: think about what could be contributed to CPython, and what could be put behind a supported API boundary.
(Also: if you're copying code from CPython, please mind the licence.)
I will close this issue. If you must use internal headers, then I think compiling them as C is the way to go. Discussion about bigger issues than moving
extern "C" {would be better on Discourse.