◐ Shell
clean mode source ↗

gh-121647: Define _Py_TYPEOF macro on more compilers by fuhsnn · Pull Request #121648 · python/cpython

@fuhsnn

@fuhsnn fuhsnn commented

Jul 12, 2024

edited by bedevere-app Bot

Loading

Extend the _Py_TYPEOF macro to more implementations of __typeof__ and standardized equivalents on C23 and C++11.
On MSVC, __typeof__ is enabled through _MSC_VER check; for others, an autoconf probe is implemented.

@bedevere-app

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-bot

🤖 New build scheduled with the buildbot fleet by @encukou for commit 85e5cbd 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@encukou

Hello! Sorry for the delay in reviewing.
Could you put the standard variants (__STDC_VERSION__, __cplusplus) first? Otherwise, they won't get used at all on compilers we test with.

Ideally, find a way to use Py_SETREF (which _Py_TYPEOF) in Lib/test/test_cext/extension.c -- which tests on different compiler settings.

Also, I'd prefer not adding the configure check. I don't see much benefit over __STDC_VERSION__, and I'd rather avoid Python.h defining another HAVE_* macro. (And removing it will remove the conflict.)

@fuhsnn

Thanks for reviewing, I did the PR to see if it gets a better chance than #121643, however cpython has grown to requiring more patches (__attribute__((section)) usage and an already-reported mimalloc bug #134070), so I'm fine patching cpython for my compiler.

On C23 typeof, I recently came across a peculiar GCC behavior that lead to it to silently remove noreturn from function pointers llvm/llvm-project#176070, so for GCC it's probably safer to stick to __typeof__.

@encukou

Thanks.
Are the patches public? I can't promise a high-priority effort but I'd like to better align CPython with the C standards.

@fuhsnn

Just tested 3.15 master (default and --disable-gil), in addition to #134070 these are enough for slimcc to pass all tests (sans test_gdb, probably caused by lacking DWARF info).

diff --git a/Include/internal/pycore_debug_offsets.h b/Include/internal/pycore_debug_offsets.h
index 66f14e6..fbdc40a 100644
--- a/Include/internal/pycore_debug_offsets.h
+++ b/Include/internal/pycore_debug_offsets.h
@@ -41,7 +41,7 @@ extern "C" {
 #define _GENERATE_DEBUG_SECTION_APPLE(name)
 #endif
 
-#if defined(__linux__) && (defined(__GNUC__) || defined(__clang__))
+#if defined(__linux__) && (defined(__GNUC__) || defined(__clang__) || (_Py__has_attribute(section) && _Py__has_attribute(used)))
 #define _GENERATE_DEBUG_SECTION_LINUX(name) \
    __attribute__((section("." Py_STRINGIFY(name))))               \
    __attribute__((used))
diff --git a/Objects/mimalloc/init.c b/Objects/mimalloc/init.c
index 81b2410..3856a10 100644
--- a/Objects/mimalloc/init.c
+++ b/Objects/mimalloc/init.c
@@ -676,7 +676,7 @@ static void mi_cdecl mi_process_done(void) {
   }
   static bool mi_initialized = _mi_process_init();
 
-#elif defined(__GNUC__) || defined(__clang__)
+#elif defined(__GNUC__) || defined(__clang__) || _Py__has_attribute(constructor)
   // GCC,Clang: use the constructor attribute

@encukou

Do you want to work on this PR, or may I take over?

Does slimcc recognize [[gcc::section, gcc::used]] attributes, with the C23 syntax?
I'm thinking about using those instead, to ensure we're getting GCC semantics rather than blindly applying an attribute called section.

In C23 typeof, I recently came across a peculiar GCC behavior that lead to it to silently remove noreturn from function pointers llvm/llvm-project#176070, so for GCC it's probably safer to stick to typeof.

I don't think this bug is likely to hit CPython, so I'd prefer not working around it.

@fuhsnn

Does slimcc recognize [[gcc::section, gcc::used]] attributes, with the C23 syntax?

Yes, it does! But the corresponding check of C23 style attributes should be __has_c_attribute(gnu::section) instead.

blindly applying an attribute called section

IMO __attribute__((section)) is kind of "owned" by GNU tool chain so it's somewhat expected for compilers reporting __has_attribute(section) to aim for compatible behavior.

Do you want to work on this PR, or may I take over?

Please do! Thank you for taking care in this.

@github-actions

This PR is stale because it has been open for 30 days with no activity.