add PyClassGuard(Mut)::map
#5311
Conversation
CodSpeed Performance ReportMerging #5311 will not alter performanceComparing Summary
|
|
Awesome, I've wanted this for a long time! Will probably review on Friday 👍 |
There was a problem hiding this comment.
Very nice, this looks good to me, thanks!
I was hoping that this would be a way forward to be able to write #[getter] functions which return borrowed data to a #[pyclass] struct Inner from a #[pyclass] struct Outer, in such a way that we could safely build a "shallow copy" inner which just points at the data stored in Outer.
But looking at test_pyclassguard_map_unrelated, I'm now not so sure. It feels to me that this might allow users to incorrectly tie arbitrary data (such as a reference to some #[pyclass] struct C) which has the same lifetime but no actual relation to Outer.
| /// # Ok::<_, PyErr>(()) | ||
| /// # }).unwrap(); | ||
| /// ``` | ||
| pub fn map<F, U: ?Sized>(self, f: F) -> PyClassGuardMap<'a, U, true> |
There was a problem hiding this comment.
For what it's worth, I wondered if this should be &mut self here, but I think you would run into pain with the fact that would prevent writing a function which takes a PyClassGuardMut and returns a PyClassGuardMap, for example.
This also matches how other MutexGuard types have implemented map, presumably for similar reasons or something else I've not yet thought of.
There was a problem hiding this comment.
Yeah, I think it makes sense to be consistent with the rest of the ecosystem here. Also I imagine that something like
obj.borrow().map(|v| &v.field) might be a common use case, which does only work if we consume the guard and store it inline.
| // It is possible to return something not borrowing from the guard, but that shouldn't | ||
| // matter. `RefCell` has the same behaviour |
This adds
PyClassGuard(Mut)::mapto map the borrow to some component of the pyclass. This borrows some ideas from #4203.Given that these are still unreleased types, I skipped the changelog.