Skip to content
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

Limited API fixes based on compile tests #6136

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

da-woods
Copy link
Contributor

@da-woods da-woods commented Apr 7, 2024

Fixes:

  • Fused function binding
  • C array conversion
  • Dataclass fallback code
  • Getting std::string from unicode (on some versions - on others it's just a runtime exception).

Fixes:
* Fused function binding
* C array conversion
* Dataclass fallback code
* Getting char* from unicode (on some versions - on others it's
  just a runtime exception).
@da-woods
Copy link
Contributor Author

da-woods commented Apr 7, 2024

I haven't enabled any specific tests on this because it was tested alongside other pending limited API changes, so I don't know how much this fixes on its own.

@@ -241,6 +241,31 @@ static CYTHON_INLINE const char* __Pyx_PyObject_AsString(PyObject* o) {
#if __PYX_DEFAULT_STRING_ENCODING_IS_ASCII || __PYX_DEFAULT_STRING_ENCODING_IS_DEFAULT
static CYTHON_INLINE const char* __Pyx_PyUnicode_AsStringAndSize(PyObject* o, Py_ssize_t *length) {
if (unlikely(__Pyx_PyUnicode_READY(o) == -1)) return NULL;
#if CYTHON_COMPILING_IN_LIMITED_API && __PYX_LIMITED_VERSION_HEX < 0x030A0000
PyErr_SetString(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can tell, __Pyx_PyUnicode_AsStringAndSize is only ever used in __Pyx_PyObject_AsStringAndSize so it's always used on something that might be a unicode object but isn't known to be. Therefore a runtime error seems appropriate. If it was definitely used on a unicode object I'd probably prefer a compile-time error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant