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

pypy: disable PyFunction #2542

Merged
merged 1 commit into from
Aug 13, 2022
Merged

Conversation

davidhewitt
Copy link
Member

Apparently this is an incomplete definition so it's not safe to use, see https://foss.heptapod.net/pypy/pypy/-/issues/3776

@davidhewitt davidhewitt force-pushed the pypy-pyfunction-type branch 3 times, most recently from 8d3d2fe to d7f1a0d Compare August 11, 2022 21:01
@davidhewitt davidhewitt enabled auto-merge August 11, 2022 21:02
Copy link
Member

@mejrs mejrs left a comment

Choose a reason for hiding this comment

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

This needs a rebase on the clippy fixes, I think.

@davidhewitt davidhewitt force-pushed the pypy-pyfunction-type branch 3 times, most recently from d867711 to 79942f8 Compare August 13, 2022 16:23
@davidhewitt davidhewitt force-pushed the pypy-pyfunction-type branch from 79942f8 to c58ff77 Compare August 13, 2022 16:51
@davidhewitt davidhewitt merged commit b389a6b into PyO3:main Aug 13, 2022
@davidhewitt davidhewitt deleted the pypy-pyfunction-type branch August 13, 2022 17:18
@mattip
Copy link
Contributor

mattip commented Oct 9, 2022

Is there a downside to making this an opaque type? Will this limit what can be done with PyO3 and PyPy?

@davidhewitt
Copy link
Member Author

Disabling this doesn't have a huge impact, it just means that users cannot check for this type using PyO3's safe APIs. They can still use the raw ffi call to do so.

The problem is that PyO3's type modelling assumes that a type_object() -> &PyType method can be implemented which hands out a valid PyType object to users. And because PyFunction::type_object() was handing out a &PyType with uninitialized fields, users hit crashes.

@mattip
Copy link
Contributor

mattip commented Oct 13, 2022

PyPy tries to implement the c-api emulation on a "need to have" basis. Do you know what project was hitting crashes, so I can interact with them and make the changes necessary to make the code work?

@davidhewitt
Copy link
Member Author

Sure thing - it was this code in pydantic-core.

@mattip
Copy link
Contributor

mattip commented Oct 13, 2022

right, that is the PyObject_IsInstance(<something>, &PyFunction_Type) problem. Is that the only waythis type object is used?

@davidhewitt
Copy link
Member Author

I think if PyO3 exposed the PyFunction type object as a high-level API then I wouldn't place a limit on how it might be used. Methods on the PyType object include getting the __qualname__ attribute or using it in PyObject_IsSubclass, however users could also just use it anywhere a PyAny is expected so unless this object is fully initialised I wouldn't be confident that users wouldn't produce crashes.

@davidhewitt davidhewitt mentioned this pull request Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants