fix case of gil-refs feature breaking create_exception! macro
#4589
Conversation
| impl<$($generics,)*> ::std::convert::AsRef<$crate::PyAny> for $name { | ||
| #[inline] | ||
| fn as_ref(&self) -> &$crate::PyAny { | ||
| &self.0 | ||
| } | ||
| } | ||
|
|
||
| impl<$($generics,)*> ::std::ops::Deref for $name { | ||
| type Target = $crate::PyAny; | ||
|
|
||
| #[inline] | ||
| fn deref(&self) -> &$crate::PyAny { | ||
| &self.0 | ||
| } | ||
| } |
There was a problem hiding this comment.
Are these any useful without gil-refs?
There was a problem hiding this comment.
I don't think so, in general I think it's helpful to not have the structure of the type objects be constrained by AsRef / Deref.
There was a problem hiding this comment.
Oh I see what you mean, I've duplicated too much here. Will fix later!
There was a problem hiding this comment.
Actually we still have AsRef / Deref here in this macro on main, maybe we could remove later (i.e. in 0.23 or later) but I think we have to leave these here in this PR for the patch fix. AsPyPointer (just below) should be removed though, I pushed that.
There was a problem hiding this comment.
Sure, removing them from main makes sense. I don't think these can ever be used now, because there is no way to construct a literal PyAny (or similar)
* fix case of gil-refs feature breaking `create_exception!` macro * remove `AsPyPointer` on non-gil-refs
Note: this is a patch fix for 0.22.4, the PR is targeting that branch, not
main.Fixes #4562
The bug is that the macros to set up all the traits for native types expand to use
#[cfg(feature = "gil-refs")]inside the expanded code, which doesn't work properly in downstream crates.The solution is to split the macro definitions so that they have gil-refs and non-gil-refs versions.
I verified this works manually; I couldn't think of a good testing strategy for the bug in question besides making yet another test crate. For a patch fix it didn't seem worth it.