implement PyTypeMethods

[davidhewitt]

Split from #3606

This PR implements the PyTypeMethods trait and the bound variants of the constructors for PyType.

The most interesting thing in this PR is the bound variant of PyType::from_type_ptr, which I decided to introduce as PyType::from_type_ptr_borrowed, because that pointer doesn't carry ownership.

There's possibly arguments that one (or both) of the PyType constructors should not have bound variants introduced and we should rework the APIs, e.g.:

• Deprecate PyType::new in favour of T::type_object (or Python::get_type)
• Find a way to make PyType::from_type_ptr_borrowed less like std::slice::from_raw_parts, e.g. should it force a reference count increase?

[davidhewitt]

Let's make a decision about #3703 (comment) before reviewing this; if we agree to slow things down I'll remove the deprecations from here.

[davidhewitt]

Okay, rebased. To make the deprecation in the macro output conditional I added a gil-refs feature to pyo3-macros. I think that's fine as we'll eventually want that feature to enable / disable creating a GILPool in #[pyfunction] calls.

Also, I'm really not feeling keen on from_type_ptr_borrowed so I might ponder better replacements for that. Suggestions welcome.

[adamreichold]

Okay, rebased. To make the deprecation in the macro output conditional I added a gil-refs feature to pyo3-macros. I think that's fine as we'll eventually want that feature to enable / disable creating a GILPool in #[pyfunction] calls.

IIRC, all GILPool usages are already moved into the impl_ module. Nevertheless, I don't see a reason to avoid forwarding that feature into our macro helper crate.

[codspeed-hq]

CodSpeed Performance Report

Merging #3705 will degrade performances by 19.27%

_{Comparing davidhewitt:type2 (410133d) with main (0dd568d)}

Summary

⚡ 2 improvements
❌ 3 regressions
✅ 74 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	main	davidhewitt:type2	Change
⚡	f64_from_pyobject	488.9 ns	405.6 ns	+20.55%
⚡	list_via_downcast	185 ns	157.2 ns	+17.67%
❌	not_a_list_via_downcast	216.7 ns	244.4 ns	-11.36%
❌	sequence_from_list	300 ns	355.6 ns	-15.63%
❌	sequence_from_tuple	232.8 ns	288.3 ns	-19.27%

[davidhewitt]

Ok, this is finally reviewable! 🎉

[LilyFirefly]

I would like to review this, but it would be helpful if the conflicts were fixed first.

[davidhewitt]

Ahh aha as soon as I'd got it ready the new changes to PyErr conflicted! I'll aim to update tonight 👍

[davidhewitt]

Fixed!

[Icxolu]

I added one remark, otherwise this looks good to me 🚀

[Icxolu]

This does now take ownership, or did I misunderstand what you meant here?

[davidhewitt]

Uff, it's assuming that the reference is a borrowed pointer and creates a new reference. I agree my wording here was not the best.

Maybe it's better if we go a little bit inconsistent here and introduce this method with the new clearer name from_borrowed_type_ptr (forget the bound bit)?

That'd then match the from_borrowed methods which I think I'm about to introduce in a new PR...

[davidhewitt]

I pushed that as a new commit.

[Icxolu]

This reads clearer to me 👍

[davidhewitt]

🎉 thanks @adamreichold @Icxolu @LilyFoote - this was a thorny one and it feels like quite the milestone to finally get this one rounded out!

[LilyFirefly]

Suggested change

	pub unsafe fn from_borrowed_type_ptr<'py>(
	py: Python<'py>,
	p: *mut ffi::PyTypeObject,
	) -> Bound<'py, PyType> {
	pub unsafe fn from_borrowed_type_ptr(
	py: Python<'_>,
	p: *mut ffi::PyTypeObject,
	) -> Bound<'_, PyType> {