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 way of assigning to immutable types and/or making a class immutable #121654

Closed
da-woods opened this issue Jul 12, 2024 · 10 comments
Closed
Labels
topic-C-API type-feature A feature request or enhancement

Comments

@da-woods
Copy link
Contributor

da-woods commented Jul 12, 2024

Feature or enhancement

Proposal:

This is one of the uglier bits of Cython's limited API implementation.

When we create our extension type ("cdef classes") we want them to be presented to users as immutable (mainly because that's what they are in non-limited-API modes and we want it to remain the same).

However, we do want to assign things to the type dict just after type creation (i.e. before it's presented to the user). That's for 2 things:

cdef class Example:
    class_attribute = 1 # needs assigning
    def f(self):  # also needs assigning
        pass

We make f our own function type (to get better introspection that the default Python C function), so need to be able to assign those functions into the class namespace (i.e. we don't want to just use PyMethodDef).

What doesn't work:

  • PyObject_SetAttr - fails because the type is immutable
  • getting __dict__ - returns a read-only proxy
  • PyType_GetDict - not in the Limited API.

My current workaround is to use PyObject_GenericSetAttr. This is supposed to be used as a good initial value of tp_setattro for mutable types. However, it doesn't check the mutability of the type, and does succeed in modifying immutable types. However, it feels like I'm misusing non-documented unintended behaviour in something.

Solutions I think could work:

  • expose PyType_GetDict,
  • some official way to set an attribute of an immutable type (that could just be deciding my GenericGetAttr way is actually fine),
  • some way to change a mutable type to immutable (so we create it as mutable, make the additions we need, then change it to immutable).
  • something else I haven't thought of?

@markshannon @encukou @vstinner tagging you as people who might have an interest in this. Not urgent though - we have a functioning workaround.

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Linked PRs

@da-woods da-woods added the type-feature A feature request or enhancement label Jul 12, 2024
@encukou
Copy link
Member

encukou commented Jul 18, 2024

Hmm. Would it work to have a slot like tp_getset, tp_members or tp_methods, but with a name and value, used to populate for class attributes?
IOW, can you create the values you're assigning before you have the class object?

API for changing a mutable type to immutable sounds doable. It might even be a better way to create types: create the object, then set slots/attributes (using setattr, or an “apply slots” function to reduce API calls), then “freeze” it if it's to be immutable. See markshannon/New-C-API-for-Python#4 (comment) for some previous discussion.

@da-woods
Copy link
Contributor Author

IOW, can you create the values you're assigning before you have the class object?

I think that would work. It's not how we currently do it, but I don't think anything we do relies on the class existing first.

Would it work to have a slot like tp_getset, tp_members or tp_methods, but with a name and value, used to populate for class attributes?

I'm not completely sure - we currently use statically allocated specs, and tables of slots for these (which wouldn't work, for this case). However I don't think it's a strict requirement.

@vstinner
Copy link
Member

some way to change a mutable type to immutable (so we create it as mutable, make the additions we need, then change it to immutable).

I would prefer this approach, rather than leaving "holes" where an "immutable" type can be modified on purpose. Can you propose an API for that?

@encukou
Copy link
Member

encukou commented Jul 29, 2024

Looks like the API could be simple, but I haven't started trying to implement it:

  • int PyType_Freeze(PyTypeObject* type): Turn a mutable class into an immutable one.

To make it more usable, we should add:

  • int PyType_ApplySlots(PyTypeObject* type, PyType_Slot *slots, Py_ssize_t nslots): apply a PyType_Slot[]: array to a mutable class. (nslots=-1 for a zero-terminated array.) Can only handle some slot types, for example it can't set tp_base or (at least in the initial implementation) tp_members.

and perhaps some parts of “new type creation” and GUPOL.

@da-woods
Copy link
Contributor Author

int PyType_Freeze(PyTypeObject* type): Turn a mutable class into an immutable one.

That was going to be my suggestion. I think this makes more sense than a general mechanism for changing type flags.

PyType_ApplySlots

I don't think Cython would need this - everything we want to do before immortalization can be done with PyObject_SetAttr. That doesn't mean it isn't useful - just that I don't have a use-case :)

@vstinner
Copy link
Member

I wrote PR gh-122457 to implement PyType_Freeze().

@da-woods
Copy link
Contributor Author

da-woods commented Aug 4, 2024

I've moved away from the PyObject_GenericSetAttr-hack because it turns out that someone's already broken it in the main branch of Python. I've found a different (slightly less hacky) way to get access to the type's __dict__ and modify it.

I still think PyType_Freeze is the nicer long-term solution though, so that remains useful to have.

But if you're ever considering modifying PyObject_GenericSetAttr and are wondering if you should do it because it might break Cython then don't worry - someone else got there first.

@wjakob
Copy link
Contributor

wjakob commented Sep 3, 2024

@da-woods — I'm curious about the motivation of making Cython's classes immutable by default. Is this to benefit from performance improvements related to the specializing interpreter? Or simply to avoid bugs due to accidental modification of a type.

@da-woods
Copy link
Contributor Author

da-woods commented Sep 3, 2024

Or simply to avoid bugs due to accidental modification of a type.

Mostly this.

Also because the existing non-Limited API version uses static types which are automatically immutable and we find it simpler if the two modes of compilation have the same behaviour.

vstinner added a commit to vstinner/cpython that referenced this issue Sep 30, 2024
vstinner added a commit to vstinner/cpython that referenced this issue Sep 30, 2024
vstinner added a commit that referenced this issue Oct 25, 2024
@vstinner
Copy link
Member

PyType_Freeze() is now implemented by change db96327.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-C-API type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

5 participants