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

Design type creation API #4

Open
encukou opened this issue Aug 22, 2022 · 10 comments
Open

Design type creation API #4

encukou opened this issue Aug 22, 2022 · 10 comments

Comments

@encukou
Copy link

encukou commented Aug 22, 2022

Type creation is a pain point in the current Limited API. (The problems are systemic -- module spec has similar issues -- but they're most visible there & a solution should be applicable elsewhere.)
The initial description -- PyType_Spec now and “layout and inheritance description” in #2 (comment) -- can't easily be extended. The current extension mechanism, slots, is not type-safe. Adding methods and properties to a live class (my interpretation of Mark's “current thinking” in the linked issue) means modifying a possibly-immutable class, a no-no as it could invalidate useful invariants.

We could have an opaque “configuration” object that's allocated and configured using functions, with a “finalize” function that turns it into a class. Not sure about performance and other implications there.

@markshannon
Copy link
Owner

Adding members to a class is fine as long as it is mutable, and making a class immutable is also fine.
Therefore the process of making an immutable class would to be create the (mutable) class, add members to it, then make it immutable.

Are there any cases where that wouldn't work?

@markshannon
Copy link
Owner

Here is a sketch.

There are four parts of a class that must be defined when the class is created.

  • The metaclass
  • The layout of objects of that class.
  • The MRO of the class (Python may allow this to change, but we should play it safe for extension classes and not allow the MRO to change).
  • Its name

Everything else can be added after class creation, including making it immutable.

int PyApi_Class_Create(
    PyStrRef name, PyRef* mro, uintptr_t mro_length,
    PyRef metaclass, uintptr_t opaque_section_size, 
    UtfString *slot_names, uintptr_t slot_count,
    PyClassRef *result);
int PyApi_Class_SetAttr(PyClassRef cls, PyStrRef name, PyRef value);
void PyApi_Class_MakeImmutable(PyClassRef cls);

Alternatively, and probably better, is to replace opaque_section_size with sort sort of layout description, from which we can generate traversal and deallocation functions.

int PyApi_Class_Create(
    PyStrRef name, PyRef* mro, uintptr_t mro_length,
    PyRef metaclass, PyApi_LayoutDescription *layout,
    PyClassRef *result);

Once created, the class can be filled out with:

int PyApi_Class_SetAttr(PyClassRef cls, PyStrRef name, PyRef value);
void PyApi_Class_MakeImmutable(PyClassRef cls);

plus some helper functions to add binary, indexing and call operators.

@encukou thoughts?

@encukou
Copy link
Author

encukou commented Sep 26, 2022

I think the MRO should always be computed from the bases, so this should take bases rather than MRO.
Not sure what semantics you're proposing for the layout description, or for slots (it sounds like a third different thing to be named slots?).
PyApi_Class_MakeImmutable can fail (e.g. if one of the bases is mutable), so it shouldn't be void.
I'll trust you that we're not losing opportunities for optimization by setting the immutable flag after the class is created.
If the “opaque section” is separate for each class in the MRO, there needs to be API to get it for a specific class. (You might be interested in a proposal for incremental improvement in this space, PEP 697, which I'm currently rewriting here.)

@markshannon
Copy link
Owner

I think the MRO should always be computed from the bases, so this should take bases rather than MRO.

How the MRO is computed from the bases is up to the metaclass, so I think it better to specify the MRO.

>>> class Meta(type):
...    def mro(cls): return cls, A, object
... 
>>> class D(metaclass=Meta) : pass
... 
>>> D.__mro__
(<class '__main__.D'>, <class '__main__.A'>, <class 'object'>)

By slots, I mean __slots__ not tp_XXX slots.
The slots are visible to the VM, so we want to treat them differently to the opaque section.

I'll trust you that we're not losing opportunities for optimization by setting the immutable flag after the class is created.

If you care about optimization, then keeping the API simple and consistent is the best thing to do.

I think we should only allow one opaque section in the MRO. Allowing multiple opaque sections slows things down due to the indirection, and makes writing C extensions more error prone, IMO.

@encukou
Copy link
Author

encukou commented Sep 26, 2022

How the MRO is computed from the bases is up to the metaclass, so I think it better to specify the MRO.

How would __bases__ be derived from the MRO?

__slots__ is as a way to define the layout from Python, so it's pretty limited – for example it doesn't support read-only members.
I guess __slots__ and tp_members could be combined using a general enough layout description?

For multiple opaque sections, you could ask authors of binding generators for C++ (or another language with inheritance) for their use cases.

@markshannon
Copy link
Owner

markshannon commented Sep 27, 2022

How would __bases__ be derived from the MRO?

There is no unique list of bases for any MRO, but you can compute a minimal one. Why do you need the bases anyway, apart from having a result for cls.__bases__?

@encukou
Copy link
Author

encukou commented Oct 3, 2022

Sorry, I don't follow your reasoning here. Why should the user pre-compute the MRO? PyApi_Class_Create has access to the metaclass so it can call mro() itself. Taking MRO as input seems like encouraging users to override mro().

@markshannon
Copy link
Owner

The MRO is determined by the metaclass. If the API accepts bases, then we need to call the metaclass to determine the MRO, which complicates things as the VM needs to call metaclass.mro() and managed the lifetime of the MRO.

int PyApi_Class_CreateFromBases(
    PyStrRef name, PyRef* bases, uintptr_t bases_length,
    PyRef metaclass, PyApi_LayoutDescription *layout,
    PyClassRef *result)
{
    PyRef mro;
    int err = PyApi_CallMethod_s(metaclass, "str", bases, bases_length, &mro);
    /* Error handling here */
   return PyApi_Class_Create(name, mro, metaclass, layout, result);
}

It works, but it is the MRO that defines the class, so we might as well expose the API that takes an MRO.

@encukou
Copy link
Author

encukou commented Oct 3, 2022

That PyApi_CallMethod_s call doesn't make much sense to me. I think I'll need to wait until the proposal is more fleshed out.

@encukou
Copy link
Author

encukou commented Oct 6, 2022

I think we should only allow one opaque section in the MRO. Allowing multiple opaque sections slows things down due to the indirection, and makes writing C extensions more error prone, IMO.

It sounds like that would

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

No branches or pull requests

2 participants