add Bound::as_any and Bound::into_any (and same for Py)

[davidhewitt]

Split from #3606

This PR adds new methods Bound::as_any() and Bound::into_any() which can be used to cast Bound<T> into Bound<PyAny>.

I found these useful when testing out the new Bound API downstream in pydantic-core.

[codspeed-hq]

CodSpeed Performance Report

Merging #3785 will degrade performances by 11.44%

_{Comparing davidhewitt:bound-as-any (49a57df) with main (516c085)}

Summary

⚡ 2 improvements
❌ 1 regressions
✅ 76 untouched benchmarks

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

Benchmarks breakdown

	Benchmark	main	davidhewitt:bound-as-any	Change
⚡	list_via_downcast	185 ns	157.2 ns	+17.67%
❌	not_a_list_via_downcast	215 ns	242.8 ns	-11.44%
⚡	sequence_from_tuple	425 ns	369.4 ns	+15.04%

[Icxolu]

I also think these are useful additions. I think I looked for these a few time while porting tests and examples to the new API.

I not really a fan of mem::transmute, because it can easily turn into a footgun during refactoring, but I'm not sure if the alternatives here are really that much better.

[davidhewitt]

Thanks, that's good to have support that these proposals make sense 👍

I think the main thing to consider before merge would be if there's better names than as_any and into_any; personally I think that they're short and reasonably obvious what they do.

I think I can avoid the transmute for the &self case, so let's fix that, at least...

[davidhewitt]

I managed to find alternatives to the transmute for both implementations, so that's an improvement!

[Icxolu]

I managed to find alternatives to the transmute for both implementations, so that's an improvement!

Cool, I like these implementations a lot more. They might be a little longer, but don't feel so "unsafe".

As for the naming, I like them. I think they convey the intent well, and read nicely when written in code.

[adamreichold]

If we are going out of our way to avoid transmute, I think we should audit at least the other conversions in this module to make their implementation styles consistent.

[davidhewitt]

That's a very good suggestion. I've pushed two more commits, which carry out the following:

• Add Py::as_any and Py::into_any, to match the first commit.
• Use all of these new safe methods to remove a lot of the unsafe { } parts of the existing code. Where it seemed obvious I also added #[inline] hints, as most of these conversions can be inlined away to nothing.

No more transmute in the whole module, and a lot more consistency 😄

[davidhewitt]

Removed inline here because it's only used in this module, I figure it'll be inlined anyway.

[adamreichold]

While it is available for inlining (just because it is in the same crate), #[inline] will still increase the likelyhood that it actually is inlined, i.e. it will bias the heuristics towards using that option, i.e. I would not remove it since this should be compiled away completed only after inlining which should happen immediately in the first pass of LLVM over the resulting MIR.

[davidhewitt]

(The only code I didn't touch is changed already in #3776.)

[adamreichold]

This looks a bit weird. Maybe an intermediate binding or some parentheses could improve it?

[davidhewitt]

For now I'll go for

        let non_null = (ManuallyDrop::new(self).1).0;
        unsafe { Py::from_non_null(non_null) }

[adamreichold]

This is a nice idiom of using ManuallyDrop for conciseness instead of safety, compared to using forget!

[davidhewitt]

Yes, it only works when pulling a Copy field out of the ManuallyDrop, but fortunately that's what we're doing here! 😂

[davidhewitt]

Thanks again both! 🚀