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

[yoney]

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

• Issue: Enhancement request: make pycore_cell.h compile cleanly under C++ #144490

[picnixz]

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]

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]

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]

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]

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

[yoney]

Created the issue #144490

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

[ZeroIntensity]

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

cpython/Lib/test/test_cppext/extension.cpp

Lines 15 to 19 in b6d8aa4

	#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.

[ZeroIntensity]

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.

[kumaraditya303]

Most other headers use _PyObject_CAST so I think we should use that instead.

[vstinner]

Please also add an #include to test_cppext as well

Just including the header is not enough to reproduce the issue. test_cppext just pass with the following change:

diff --git a/Lib/test/test_cppext/extension.cpp b/Lib/test/test_cppext/extension.cpp
index f95655eccde..881140eb695 100644
--- a/Lib/test/test_cppext/extension.cpp
+++ b/Lib/test/test_cppext/extension.cpp
@@ -15,6 +15,7 @@
 #ifdef TEST_INTERNAL_C_API
    // gh-135906: Check for compiler warnings in the internal C API
 #  include "internal/pycore_backoff.h"
+#  include "internal/pycore_cell.h"
 #  include "internal/pycore_frame.h"
 #endif
 

[vstinner]

LGTM

[colesbury]

Just including the header is not enough to reproduce the issue. test_cppext just pass with the following change:

@vstinner - I think BaseTests in Lib/test/test_cppext/__init__.py is missing handling of TEST_INTERNAL_C_API. It doesn't have the env['TEST_INTERNAL_C_API'] = str(int(self.TEST_INTERNAL_C_API)) line from test_cext

Also, in case it's not clear, the error here is limited to Py_GIL_DISABLED builds.

[yoney]

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

I’m not sure if this is the correct way to run it. When I set TEST_INTERNAL_C_API=1, the test immediately emits many C++ errors. I tried fixing some of them by passing CPPFLAGS, but there are still a lot of errors, so I stopped. Below are a few sample error lines; many seem to originate from mimalloc. It’s also possible that I’m invoking the test incorrectly.

[cpython]$ TEST_INTERNAL_C_API=1 ./python -m test test.test_cppext -u cpu=1
...
      CC env var: 'gcc'
      CFLAGS env var: <missing>
      CPPFLAGS env var: <missing>
      extra_compile_args: ['-Werror', '-DMODULE_NAME=_testcpp11ext', '-std=c++11', '-pedantic-errors', '-Wno-long-long', '-DTEST_INTERNAL_C_API=1']
      running bdist_wheel
      running build
      running build_ext
      building '_testcpp11ext' extension
      creating build/temp.linux-x86_64-cpython-315t-pydebug
      g++ -fno-strict-overflow -Wsign-compare -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -Og -Wall -fPIC -I/home/dev/workspace/cpython/build/test_python_worker_21579æ/tempcwd/env/include -I/home/dev/workspace/cpython/Include -I/home/dev/workspace/cpython -c extension.cpp -o build/temp.linux-x86_64-cpython-315t-pydebug/extension.o -Werror -DMODULE_NAME=_testcpp11ext -std=c++11 -pedantic-errors -Wno-long-long -DTEST_INTERNAL_C_API=1
      In file included from /usr/include/features.h:524,
                       from /usr/include/assert.h:35,
                       from /home/dev/workspace/cpython/Include/Python.h:20,
                       from extension.cpp:13:
      /home/dev/workspace/cpython/Include/internal/mimalloc/mimalloc/types.h:73:33: error: expected unqualified-id before ‘noexcept’
         73 | #define mi_decl_throw           __THROW
            |                                 ^~~~~~~
      /home/dev/workspace/cpython/Include/internal/mimalloc/mimalloc/types.h:611:31: note: in expansion of macro ‘mi_decl_throw’
        611 | mi_decl_noreturn mi_decl_cold mi_decl_throw
            |                               ^~~~~~~~~~~~~
      In file included from /home/dev/workspace/cpython/Include/internal/pycore_mimalloc.h:44,
                       from /home/dev/workspace/cpython/Include/internal/pycore_tstate.h:14,
                       from /home/dev/workspace/cpython/Include/internal/pycore_interp_structs.h:15,
                       from /home/dev/workspace/cpython/Include/internal/pycore_backoff.h:15,
                       from extension.cpp:17:
      /home/dev/workspace/cpython/Include/internal/mimalloc/mimalloc/internal.h: In function ‘bool _mi_is_aligned(void*, size_t)’:
      /home/dev/workspace/cpython/Include/internal/mimalloc/mimalloc/types.h:613:49: error: ‘_mi_assert_fail’ was not declared in this scope; did you mean ‘__assert_fail’?
        613 | #define mi_assert(expr)     ((expr) ? (void)0 : _mi_assert_fail(#expr,__FILE__,__LINE__,__func__))
            |                                                 ^~~~~~~~~~~~~~~
      /home/dev/workspace/cpython/Include/internal/mimalloc/mimalloc/types.h:619:31: note: in expansion of macro ‘mi_assert’
        619 | #define mi_assert_internal    mi_assert
            |                               ^~~~~~~~~
...

Note: This is on the unmodified main branch; I did not add any includes.

[vstinner]

@vstinner - I think BaseTests in Lib/test/test_cppext/init.py is missing handling of TEST_INTERNAL_C_API. It doesn't have the env['TEST_INTERNAL_C_API'] = str(int(self.TEST_INTERNAL_C_API)) line from test_cext

Ooops! I wrote #144536 to fix test_cppext (test the internal C API).

When I set TEST_INTERNAL_C_API=1, the test immediately emits many C++ errors

      /home/dev/workspace/cpython/Include/internal/mimalloc/mimalloc/types.h:73:33: error: expected unqualified-id before ‘noexcept’
         73 | #define mi_decl_throw           __THROW
            |                                 ^~~~~~~

Yeah, I already saw C++ errors/warnings on mimalloc. I'm not sure how to deal with it.