Open adamreichold opened 4 months ago
👍 I guess we probably want a derive macro for this trait too.
I started to look into this and there are hurdles, for starters we really want the blanket impl
impl<T> PyClone for T
where
T: Clone,
{
fn clone_ref(&self, _py: Python<'_>) -> Self {
self.clone()
}
}
so that we can fall back to normal Clone
in getters for types like i32
.
But this conflicts with necessary impls like
impl<T> PyClone for Option<T> where T: PyClone;
impl<T> PyClone for Vec<T> where T: PyClone;
and this also does not really end because set for which we would want to provide PyClone
is open-ended.
(It also conflicts with the PyClone
impl for Py
if the py-clone
feature is enabled, but we could suppress that.)
So I don't think it is possible to make this as seamless as Clone
especially considering types outside of our purview.
The only reasonable idea I currently have is to again make use of use being in macro context when we call clone
in impl_py_getter_def
, meaning we use autoref specialization to prefer calling clone_ref
if it exists and fallback to clone
otherwise. Meaning we also do not provide a blanket impl of PyClone
at all and implement it for our types and some basic ones like Option
and Vec
.
What do people think? Hopefully anybody finds a better approach. I will really feel bad about adding yet another case of autoref specialization to our proc macros...
Uff, this kind of complexity sounds all too familiar to me.
The only reasonable idea I currently have is to again make use of use being in macro context when we call
clone
inimpl_py_getter_def
, meaning we use autoref specialization to prefer callingclone_ref
if it exists and fallback toclone
otherwise. Meaning we also do not provide a blanket impl ofPyClone
at all and implement it for our types and some basic ones likeOption
andVec
.What do people think? Hopefully anybody finds a better approach. I will really feel bad about adding yet another case of autoref specialization to our proc macros...
I did indeed look into the specialization approach in #4254, and I used ToPyObject
for cases like Vec<Py<T>>
instead of the current IntoPy + Clone
.
I at least didn't use autoref specialization and instead used type-level const logic to specialize, which might be an easier pill to swallow? 😂
Due to issues making delay reference count increments sound, they were removed #4095 and replaced by an
Clone
implementation forPy
which will panic without the GIL being held, gated by thepy-clone
feature.This has several usability downsides, e.g.
#[pyo3(get)]
stops working withPy<T>
fields which however is a very important use case when sharing data with Python code. Similarly, thePyBacked*
types cannot be unconditionallyClone
themselves.Both problems (and likely others) should be resolvable if we add a trait, e.g.
CloneRef
orPyClone
with signatureand implement it unconditionally for
Py<T>
and add a blanket impl based onClone
. The proc macro machinery behind#[pyo3(get)]
could then go via this trait instead of the plainClone
.