gh-86542: New C-APIs to simplify module attribute declaration by tiran · Pull Request #23286 · python/cpython · GitHub
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 145 additions & 0 deletions Doc/c-api/module.rst
16 changes: 16 additions & 0 deletions Doc/data/refcounts.dat
Original file line number Diff line number Diff line change
Expand Up @@ -1325,6 +1325,10 @@ PyMethod_New:PyObject*:class:0:
PyMethod_Self:PyObject*::0:
PyMethod_Self:PyObject*:im:0:

PyModule_AddConstants:int:::
PyModule_AddConstants:PyObject*:module:0:
PyModule_AddConstants:PyModuleConst_Def*:def::

PyModule_AddFunctions:int:::
PyModule_AddFunctions:PyObject*:module:0:
PyModule_AddFunctions:PyMethodDef*:functions::
Expand All @@ -1343,6 +1347,18 @@ PyModule_AddObject:PyObject*:module:0:
PyModule_AddObject:const char*:name::
PyModule_AddObject:PyObject*:value:+1:

PyModule_AddNewException:PyObject*::+1:
PyModule_AddNewException:PyObject*:module:0:
PyModule_AddNewException:const char*:name::
PyModule_AddNewException:const char*:doc::
PyModule_AddNewException:PyObject*:base:0:
PyModule_AddNewException:PyObject*:dict:0:

PyModule_AddNewTypeFromSpec:PyObject*::+1:
PyModule_AddNewTypeFromSpec:PyObject*:module:0:
PyModule_AddNewTypeFromSpec:PyType_spec*:spec::
PyModule_AddNewTypeFromSpec:PyObject*:base:0:

PyModule_AddStringConstant:int:::
PyModule_AddStringConstant:PyObject*:module:0:
PyModule_AddStringConstant:const char*:name::
Expand Down
9 changes: 9 additions & 0 deletions Include/modsupport.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,15 @@ PyAPI_FUNC(int) PyModule_AddType(PyObject *module, PyTypeObject *type);
#define PyModule_AddIntMacro(m, c) PyModule_AddIntConstant(m, #c, c)
#define PyModule_AddStringMacro(m, c) PyModule_AddStringConstant(m, #c, c)

#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03100000
/* New in 3.9 */
PyAPI_FUNC(PyTypeObject *) PyModule_AddNewTypeFromSpec(
PyObject *module, PyType_Spec *spec, PyObject *base);
PyAPI_FUNC(PyObject *) PyModule_AddNewException(
PyObject *module, const char *name, const char *doc,
PyObject *base, PyObject *dict);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you start by adding these two functions in a separated PR, to make this PR shorter?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't really think that's necessary; the PR is not that big.

#endif

#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03050000
/* New in 3.5 */
PyAPI_FUNC(int) PyModule_SetDocString(PyObject *, const char *);
Expand Down
47 changes: 47 additions & 0 deletions Include/moduleobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,53 @@ typedef struct PyModuleDef_Slot{

#endif /* New in 3.5 */

struct PyModuleConst_Def;
#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03100000
/* New in 3.10 */
enum _PyModuleConst_type {
_PyModuleConst_none_type = 1,
_PyModuleConst_long_type = 2,
_PyModuleConst_ulong_type = 3,
_PyModuleConst_bool_type = 4,
_PyModuleConst_double_type = 5,
_PyModuleConst_string_type = 6,
_PyModuleConst_call_type = 7,
};

typedef struct PyModuleConst_Def {
const char *name;
enum _PyModuleConst_type type;
union {
const char *m_str;
long m_long;
unsigned long m_ulong;
double m_double;
PyObject* (*m_call)(PyObject *module);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure how to make this member future-proof in term of stable ABI.

We should try to put a max_align_t inside, but this type requires C11. GCC defines it with:

typedef struct {
  long long __max_align_ll __attribute__((__aligned__(__alignof__(long long))));
  long double __max_align_ld __attribute__((__aligned__(__alignof__(long double))));
  /* _Float128 is defined as a basic type, so max_align_t must be
     sufficiently aligned for it.  This code must work in C++, so we
     use __float128 here; that is only available on some
     architectures, but only on i386 is extra alignment needed for
     __float128.  */
#ifdef __i386__
  __float128 __max_align_f128 __attribute__((__aligned__(__alignof(__float128))));
#endif
} max_align_t;

Maybe we can at least put long long and long double:

// Unused members added to make PyModuleConst_Def large enough
// to get a stable ABI and support future additions.
long long m_long_long;
long double m_long_double;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the only issue I found here :)
The stable ABI doesn't have a good story around evolving structs, and I think we should design a general mechanism for that rather than try to future-proof individual structs.
To move this PR forward, could we exclude PyModule_AddConstants & co. from the limited API for the time being?

} value;
} PyModuleConst_Def;

PyAPI_FUNC(int) PyModule_AddConstants(PyObject *, PyModuleConst_Def *);

#define PyModuleConst_None(name) \
{(name), _PyModuleConst_none_type, {.m_long=0}}
#define PyModuleConst_Long(name, value) \
{(name), _PyModuleConst_long_type, {.m_long=(value)}}
#define PyModuleConst_ULong(name, value) \
{(name), _PyModuleConst_ulong_type, {.m_ulong=(value)}}
#define PyModuleConst_Bool(name, value) \
{(name), _PyModuleConst_bool_type, {.m_long=(value)}}
#define PyModuleConst_Double(name, value) \
{(name), _PyModuleConst_double_type, {.m_double=(value)}}
#define PyModuleConst_String(name, value) \
{(name), _PyModuleConst_string_type, {.m_str=(value)}}
#define PyModuleConst_Call(name, value) \
{(name), _PyModuleConst_call_type, {.m_call=(value)}}

#define PyModuleConst_LongMacro(m) PyModuleConst_Long(#m, m)
#define PyModuleConst_StringMacro(m) PyModuleConst_String(#m, m)

#endif /* New in 3.10 */

typedef struct PyModuleDef{
PyModuleDef_Base m_base;
const char* m_name;
Expand Down
14 changes: 14 additions & 0 deletions Lib/test/test_capi.py
Original file line number Diff line number Diff line change
Expand Up @@ -971,5 +971,19 @@ def test_state_access(self):
increment_count(1, 2, 3)


class Test_PyModuleConst_Def(unittest.TestCase):
def test_constants(self):
self.assertIs(_testcapi.const_none, None)
self.assertEqual(_testcapi.const_int, 42)
self.assertEqual(_testcapi.const_uint, _testcapi.ULONG_MAX)
self.assertIs(_testcapi.const_true, True)
self.assertIs(_testcapi.const_false, False)
self.assertEqual(_testcapi.const_almost_tau, 6.2831)
self.assertEqual(_testcapi.const_str, "Hello")
self.assertEqual(_testcapi.const_call, b"23")
self.assertEqual(_testcapi.CONST_INT, 7)
self.assertEqual(_testcapi.CONST_STRING, "world")


if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Add new functions :c:func:`PyModule_AddConstants`,
:c:func:`PyModule_AddNewTypeFromSpec`, :c:func:`PyModule_AddNewException` to
simplify the declaration of attribute in modules.
37 changes: 10 additions & 27 deletions Modules/_hashopenssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -2025,14 +2025,9 @@ hashlib_init_evptype(PyObject *module)
{
_hashlibstate *state = get_hashlib_state(module);

state->EVPtype = (PyTypeObject *)PyType_FromSpec(&EVPtype_spec);
if (state->EVPtype == NULL) {
return -1;
}
if (PyModule_AddType(module, state->EVPtype) < 0) {
return -1;
}
return 0;
state->EVPtype = PyModule_AddNewTypeFromSpec(
module, &EVPtype_spec, NULL);
return state->EVPtype == NULL ? -1 : 0;
}

static int
Expand All @@ -2044,33 +2039,21 @@ hashlib_init_evpxoftype(PyObject *module)
if (state->EVPtype == NULL) {
return -1;
}

state->EVPXOFtype = (PyTypeObject *)PyType_FromSpecWithBases(
&EVPXOFtype_spec, (PyObject *)state->EVPtype
);
if (state->EVPXOFtype == NULL) {
return -1;
}
if (PyModule_AddType(module, state->EVPXOFtype) < 0) {
return -1;
}
state->EVPXOFtype = PyModule_AddNewTypeFromSpec(
module, &EVPXOFtype_spec, (PyObject *)state->EVPtype);
return state->EVPXOFtype == NULL ? -1 : 0;
#endif
return 0;
}

static int
hashlib_init_hmactype(PyObject *module)
{
_hashlibstate *state = get_hashlib_state(module);
_hashlibstate *state = get_hashlib_state(module);

state->HMACtype = (PyTypeObject *)PyType_FromSpec(&HMACtype_spec);
if (state->HMACtype == NULL) {
return -1;
}
if (PyModule_AddType(module, state->HMACtype) < 0) {
return -1;
}
return 0;
state->HMACtype = PyModule_AddNewTypeFromSpec(
module, &HMACtype_spec, NULL);
return state->HMACtype == NULL ? -1 : 0;
}

static int
Expand Down
17 changes: 4 additions & 13 deletions Modules/_ssl.c
Loading