gh-128629: Add Py_PACK_VERSION and Py_PACK_FULL_VERSION#128630
gh-128629: Add Py_PACK_VERSION and Py_PACK_FULL_VERSION#128630encukou merged 2 commits intopython:mainfrom
Conversation
Modules/_testlimitedcapi/version.c
Outdated
| @@ -0,0 +1,74 @@ | |||
| /* Test version macros in the limited API */ | |||
|
|
|||
| #define Py_LIMITED_API 0x030E0000 // Added in 3.14 | |||
There was a problem hiding this comment.
You need more complicated code for Free Threading, something like:
#include "pyconfig.h" // Py_GIL_DISABLED
#ifndef Py_GIL_DISABLED
# define Py_LIMITED_API 0x030e0000 // Added in 3.14
#endif
| result = _testlimitedcapi.pack_version(*args) | ||
| self.assertEqual(result, expected) | ||
|
|
||
| def test_pack_full_version_ctypes(self): |
There was a problem hiding this comment.
What's the purpose of testing the API through ctypes? It should be the same as testing _testlimitedcapi, no?
There was a problem hiding this comment.
It tests the exported function.
(Unlike most similar API, these are macros even in limited API. This tests the case of not using the C headers at all.)
There was a problem hiding this comment.
The _testcapi module tests Py_INCREF() macro and function this way:
#define TEST_REFCOUNT() \
do { \
PyObject *obj = PyList_New(0); \
if (obj == NULL) { \
return NULL; \
} \
assert(Py_REFCNT(obj) == 1); \
\
/* test Py_NewRef() */ \
PyObject *ref = Py_NewRef(obj); \
assert(ref == obj); \
assert(Py_REFCNT(obj) == 2); \
Py_DECREF(ref); \
\
/* test Py_XNewRef() */ \
PyObject *xref = Py_XNewRef(obj); \
assert(xref == obj); \
assert(Py_REFCNT(obj) == 2); \
Py_DECREF(xref); \
\
assert(Py_XNewRef(NULL) == NULL); \
\
Py_DECREF(obj); \
Py_RETURN_NONE; \
} while (0)
// Test Py_NewRef() and Py_XNewRef() macros
static PyObject*
test_refcount_macros(PyObject *self, PyObject *Py_UNUSED(ignored))
{
TEST_REFCOUNT();
}
#undef Py_NewRef
#undef Py_XNewRef
// Test Py_NewRef() and Py_XNewRef() functions, after undefining macros.
static PyObject*
test_refcount_funcs(PyObject *self, PyObject *Py_UNUSED(ignored))
{
TEST_REFCOUNT();
}Maybe you can do something similar to avoid ctypes.
There was a problem hiding this comment.
The C tests do something similar, with #undef Py_PACK_FULL_VERSION.
(But if I had misspelled that #undef, the test would quietly be ineffective.)
Also, I don't particularly want to avoid ctypes :)
| @@ -1,4 +1,5 @@ | |||
|
|
|||
| #ifndef _Py_PATCHLEVEL_H | |||
There was a problem hiding this comment.
Why not just Py_PATCHLEVEL_H, as done in other header files?
There was a problem hiding this comment.
Because I don't want to add this define to public API. It's a private detail, users don't need this; we should be able to, for example, merge this code into another header and remove patchlevel.h.
|
Thank you for the reviews! |
📚 Documentation preview 📚: https://cpython-previews--128630.org.readthedocs.build/