Skip to content

gh-144490: Fix C++ compatibility in pycore_cell.h#144482

Open
yoney wants to merge 1 commit intopython:mainfrom
yoney:pycore_cell
Open

gh-144490: Fix C++ compatibility in pycore_cell.h#144482
yoney wants to merge 1 commit intopython:mainfrom
yoney:pycore_cell

Conversation

@yoney
Copy link
Contributor

@yoney yoney commented Feb 4, 2026

The header already includes an extern "C" guard, suggesting it is intended to be C++ compatible. Adding a cast allows it to compile without -fpermissive.

cc: @colesbury

@picnixz
Copy link
Member

picnixz commented Feb 4, 2026

Do we do this kind of stuff for every calls? do we support -fpermissive flags? should we do it? I would prefer having an issue (maybe an old one) for other places that could be affected by this.

@colesbury
Copy link
Contributor

The internal headers (pycore_xxx.h) are often not C++ compatible, even though they all have extern "C" { guards (often in the wrong places).

The change itself seems fine, but I'm curious how this came up. What's including pycore_cell.h from C++?

@colesbury
Copy link
Contributor

To answer @picnixz, we generally do this sort of thing when people ask, especially when the changes are tiny, but we don't proactively make the internal header files C++ compatible.

@yoney
Copy link
Contributor Author

yoney commented Feb 4, 2026

What's including pycore_cell.h from C++?

This came up while I’m adapting CinderX to FT-Python: C++ codegen path for LOAD_DEREF/STORE_DEREF ends up including pycore_cell.h and calling the internal helpers as a good starting point (vs. generating the atomic-swap sequence right away). I initially worked around it with wrappers on our side, but since the header already has extern "C" guards and the change is tiny, I wanted to check whether it’s okay to make this compile cleanly from C++.

Also agreed that pycore_* headers aren’t guaranteed to be C++-compatible and may change; in CinderX we already adapt to internal changes per version.

@colesbury
Copy link
Contributor

Ok, make sense. Can you open an issue like @picnixz asked? Also edit the PR title to associate it with the issue.

@yoney yoney changed the title Fix C++ compatibility in _PyCell_GetStackRef gh-144490: Fix C++ compatibility in pycore_cell.h Feb 4, 2026
@yoney
Copy link
Contributor Author

yoney commented Feb 4, 2026

Created the issue #144490

@picnixz, @colesbury: Thank you so much for the review.

Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add an #include to test_cppext as well, like this:

#ifdef TEST_INTERNAL_C_API
// gh-135906: Check for compiler warnings in the internal C API
# include "internal/pycore_backoff.h"
# include "internal/pycore_frame.h"
#endif

That'll make sure we don't break this again someday.

PyObject *value;
#ifdef Py_GIL_DISABLED
value = _Py_atomic_load_ptr(&cell->ob_ref);
value = (PyObject *)_Py_atomic_load_ptr(&cell->ob_ref);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you, but I believe we've used _Py_STATIC_CAST for things like this in the past. That will expand to static_cast<type>(expr) in C++, and then the usual (type)(expr) in C.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants