-
-
Notifications
You must be signed in to change notification settings - Fork 34.4k
gh-148464: Add missing __ctype_le/be__ attributes for complex types in the ctype module
#148485
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| Add missing ``__ctype_le/be__`` attributes for | ||
| :class:`~ctypes.c_float_complex` and :class:`~ctypes.c_double_complex`. Patch | ||
| by Sergey B Kirpichev. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -792,6 +792,32 @@ D_get(void *ptr, Py_ssize_t size) | |
| return PyComplex_FromDoubles(x[0], x[1]); | ||
| } | ||
|
|
||
| static PyObject * | ||
| D_set_sw(void *ptr, PyObject *value, Py_ssize_t size) | ||
| { | ||
| assert(NUM_BITS(size) || (size == 2*sizeof(double))); | ||
| Py_complex c = PyComplex_AsCComplex(value); | ||
|
|
||
| if (c.real == -1 && PyErr_Occurred()) { | ||
| return NULL; | ||
| } | ||
| if (PyFloat_Pack8(c.real, ptr, PY_BIG_ENDIAN) | ||
| || PyFloat_Pack8(c.imag, ptr + sizeof(double), PY_BIG_ENDIAN)) | ||
| { | ||
| return NULL; | ||
| } | ||
| _RET(value); | ||
| } | ||
|
|
||
| static PyObject * | ||
| D_get_sw(void *ptr, Py_ssize_t size) | ||
| { | ||
| assert(NUM_BITS(size) || (size == 2*sizeof(double))); | ||
| return PyComplex_FromDoubles(PyFloat_Unpack8(ptr, PY_BIG_ENDIAN), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add error handling for the unpacks here and I'd be happier if
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why undocumented? It's documented and widely used across the CPython codebase, e.g. in same file. |
||
| PyFloat_Unpack8(ptr + sizeof(double), | ||
| PY_BIG_ENDIAN)); | ||
| } | ||
|
|
||
| /* F: float complex */ | ||
| static PyObject * | ||
| F_set(void *ptr, PyObject *value, Py_ssize_t size) | ||
|
|
@@ -817,6 +843,32 @@ F_get(void *ptr, Py_ssize_t size) | |
| return PyComplex_FromDoubles(x[0], x[1]); | ||
| } | ||
|
|
||
| static PyObject * | ||
| F_set_sw(void *ptr, PyObject *value, Py_ssize_t size) | ||
| { | ||
| assert(NUM_BITS(size) || (size == 2*sizeof(float))); | ||
| Py_complex c = PyComplex_AsCComplex(value); | ||
|
|
||
| if (c.real == -1 && PyErr_Occurred()) { | ||
| return NULL; | ||
| } | ||
| if (PyFloat_Pack4(c.real, ptr, PY_BIG_ENDIAN) | ||
| || PyFloat_Pack4(c.imag, ptr + sizeof(float), PY_BIG_ENDIAN)) | ||
| { | ||
| return NULL; | ||
| } | ||
| _RET(value); | ||
| } | ||
|
|
||
| static PyObject * | ||
| F_get_sw(void *ptr, Py_ssize_t size) | ||
| { | ||
| assert(NUM_BITS(size) || (size == 2*sizeof(float))); | ||
| return PyComplex_FromDoubles(PyFloat_Unpack4(ptr, PY_BIG_ENDIAN), | ||
| PyFloat_Unpack4(ptr + sizeof(float), | ||
| PY_BIG_ENDIAN)); | ||
| } | ||
|
|
||
| /* G: long double complex */ | ||
| static PyObject * | ||
| G_set(void *ptr, PyObject *value, Py_ssize_t size) | ||
|
|
@@ -1602,7 +1654,9 @@ for base_code, base_c_type in [ | |
| #if defined(_Py_FFI_SUPPORT_C_COMPLEX) | ||
| if (Py_FFI_COMPLEX_AVAILABLE) { | ||
| TABLE_ENTRY(D, &ffi_type_complex_double); | ||
| TABLE_ENTRY_SW(D, &ffi_type_complex_double); | ||
| TABLE_ENTRY(F, &ffi_type_complex_float); | ||
| TABLE_ENTRY_SW(F, &ffi_type_complex_float); | ||
| TABLE_ENTRY(G, &ffi_type_complex_longdouble); | ||
| } | ||
| #endif | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to https://www.chiark.greenend.org.uk/doc/libffi-dev/html/Structures.html the
elementsshould be a NULL terminated list of pointers. With this allocation we are missing theNULL. According to Claude this is no problem for the current implementation, but it is a breach of contract and a latent bug.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for primitive types (section above), not for structures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you put the duplicated code in a helper function, like
fill_stginfo_from_pffi_type?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you elaborate more? I also think this could be a real problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is indeed a breakage of libffi's invariant. I opened #148573 for this.