Complete abi3 support

[alex]

abi3, also known as pylimited abi, is a feature of built python projects where they can be loaded from multiple versions of python3. pyo3 has some limited support for this, but it's commented out in Cargo.toml because it's currently incomplete and does not compile.

I'm filing this as a tracking bug so folks interested in abi3 support have a place to follow along. I intend to attempt to contribute to resolving this issue.

I imagine this is a three part process:

1. switching things from internals -> abi3 APIs when there's a 1:1 equivalent
2. disabling pyo3 APIs that map to things that don't exist in abi3 mode
3. figuring out how to re-enable the things from (2)

[kngwyu]

You need #[pyclass] with abi3?
I think most of the code works with PY_LIMITED_API or can be modified to do so, but not sure about #[pyclass].

[davidhewitt]

I think it would be desirable if #[pyclass] can work with the limited API, but we may need to talk a lot to upstream cpython to achieve it.

[alex]

Why do you imagine it'd require upstream support? I imagine most #[pyclass] usage can be achieved with PyType_FromSpec?

[davidhewitt]

Quite possibly - might need a lot of refactoring!

[alex]

I started investigating #[pyclass] support, since it's the biggest piece here. Before I get too deep into implementing, I wanted to lay out my thoughts and make sure this all sounded ok.

First, I'll start by describing the current approach:

• Currently pyo3-dervice-backend emits code that creates a register, which contains a struct for each of the tables of function pointers that a PyTypeObject* contains.
• pyo3 then takes this table, create a PyTypeObject* and fills in all the fields by hand basically, and then calls PyType_Ready.

My proposal for how to make this abi3 compatible:

• pyo3-dervice-backend is changed to generate a list of PEP384 slots
• pyo3 takes those and calls PyType_FromSpec

The one challenge is that some of these APIs are technically public (though they often have #[doc(hidden)]) because they're called from macros that expand in users's crates. Is it ok to change these APIs - perhaps with a major version bump?

One of my goals here is that there's only one version of this code, and not 2 different versions.

[alex]

#1132 has a PoC of what the first steps of this could look like

[davidhewitt]

I'm (perhaps optimistically) marking this for the 0.13 release. Let's see how we get on!

[alex]

I've also done a POC of what the setuptools-rust side of this can look like: PyO3/setuptools-rust#82

[alex]

Status check, looking at the abi3 branch with #1162 incorporated:

Here's the remaining compilation errors with cargo check --no-default-features --features macros. In total there are 15 compilation errors, but I think it's fewer root causes:

• FreeFunc - needs to be #[cfg()]'d out
• A family of errors around tp_new/tp_alloc/tp_free. I haven't looked at these yet
• Need to find a replacement for ffi::PyRun_StringFlags
• Need to find a replacement for ffi::PyUnicode_AsUTF8AndSize - this one is a bit messy. The only equivilant, AFAICT, is to encode the object to a PyBytesObject and get the pointer out of this. Since ffi::PyUnicode_AsUTF8AndSize is used in to_str(&self) -> PyResult<&str> this is a mess, since creating a temporary PyBytesObject would have a different lifetime. The best I can come up with is changing to_str() to return something that impl Deref<Target=&str>. Thoughts?
• tp_dict - setting up class attributes on fresh classes. Can this just be setattr()?
• tp_name - For PyType::name. I can't find an equivilant API, so I think I'll just use getattr("__name__")
• No iterator for sets with limited API - switch to just using a generic iterator

[alex]

Down to 3 underlying issues! PyRun_StringFlags is the next one I plan to look at it. I could use feedback on the other two though.

[alex]

Ok, the last two issues I could use some feedback on. I'd like to to do the PyUnicode_AsUTF8AndSize one next. I don't see how to fix it without changing the API of to_str() (which probably also impacts extract()). Does anyone have thoughts?

[kngwyu]

I'm sorry but I cannot investigate that until this weekend.

[alex]

No problem. I'll keep poking myself.

[kngwyu]

I looked over all APIs and am a bit surprised to see there's no UTF-8 methods with LIMITED_API...
@alex
What alternative do you have in your mind?

[alex]

The only alternative I see is to encode it to a bytes object and then get the slice out of that. But then you can't return a `&[u8]` because you need to keep the bytes object alive somewhere.

…

On Sat, Sep 12, 2020 at 2:47 AM Yuji Kanagawa ***@***.***> wrote: I looked over all APIs and am a bit surprised to see there's no UTF-8 methods with LIMITED_API... @alex <https://github.com/alex> What alternative do you have in your mind? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1125 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAGBF4EJIM3WYF52VJDWDSFMKQ7ANCNFSM4QPXW2MQ> .

-- All that is necessary for evil to succeed is for good people to do nothing.

[alex]

I'm now looking at the other issue around alloc/dealloc. I found that if I delete these lines:

pyo3/src/pyclass.rs

Lines 48 to 52 in 448f0bb

	if Self::is_exact_instance(obj) && ffi::PyObject_CallFinalizerFromDealloc(obj.as_ptr()) < 0
	{
	// tp_finalize resurrected.
	return;
	}

no test fails. Does deleting this lines entirely seem right? Is it just a case that isn't tested?

[alex]

Ok, I poked further and I think indeed this is effectively dead code. is_exact_instance makes sure the ob_type in obj is exactly the same as the one for the class that is Self. Classes that we create never have tp_finalize filled in! And all PyObject_CallFinalizerFromDealloc does is call tp_finalize, which is always null, and therefore as far as I can tell it's a big no-op. I'll send a PR :-)

[programmerjake]

friendly reminder to put a comment in the code so we don't forget to handle this if PyO3 starts to use tp_finalize some time in the future

[alex]

I've filed a bug with CPython about making PyUnicode_AsUTF8AndSize a limited API: https://bugs.python.org/issue41784

[alex]

With #1187 and #1183, we'll now be able to cargo check --no-default-features --features macros!

[alex]

Status update: cargo check --no-default-features --features macros is now error free! I have a PR up (#1188) to get it warning clean.

Then the next step will be to get all the tests passing. After that, we add it to CI and merge I hope:-)

[alex]

#1189 now has all the tests passing, except the doctests.

[alex]

I think this is complete now, with the merge of #1152!