Add MemCx<'mcx> for List<'mcx, T>
#1342
Conversation
pgrx/src/pg_sys.rs
Outdated
| mem::current_context(|cx| { | ||
| crate::list::List::<*mut core::ffi::c_void>::downcast_ptr_in_memcx(range_table, cx) | ||
| .expect("rt_fetch used on non-ptr List") | ||
| .get((index - 1) as _) | ||
| .expect("rt_fetch used out-of-bounds") | ||
| .cast() | ||
| }) |
There was a problem hiding this comment.
This diff suggests to me my approach is kinda incorrect, since I don't really "use" the context.
There was a problem hiding this comment.
Yeah, this is probably going to be the most annoying part here. Maybe this would be better captured as another parameter to List, so non-allocating but owning uses don't have to worry about this.
pgrx/src/mem.rs
Outdated
| /// Acquire the current context and operate inside it. | ||
| pub fn current_context<'curr, F, T>(f: F) -> T | ||
| where | ||
| F: for<'clos> FnOnce(&'clos MemCx<'curr>) -> T, | ||
| { | ||
| let memcx = unsafe { MemCx::from_ptr(pg_sys::CurrentMemoryContext) }; | ||
| let ret = { f(&memcx) }; | ||
| ret | ||
| } |
There was a problem hiding this comment.
Definitely not sure about soundness, but something similar to this is probably required, for ergonomics reasons.
| iter: RawCellIter<T>, | ||
| } | ||
|
|
||
| /// A list being drained. | ||
| #[derive(Debug)] | ||
| pub struct Drain<'a, T> { | ||
| pub struct Drain<'a, 'cx, T> { |
There was a problem hiding this comment.
Really need to move Drain itself "up".
Also: two lifetimes! spooky.
|
Just a note to say I've skimmed over this. I don't have any feedback yet other than it's exciting to see some code demonstrating the ideas. |
pgrx/src/pg_sys.rs
Outdated
| mem::current_context(|cx| { | ||
| crate::list::List::<*mut core::ffi::c_void>::downcast_ptr_in_memcx(range_table, cx) | ||
| .expect("rt_fetch used on non-ptr List") | ||
| .get((index - 1) as _) | ||
| .expect("rt_fetch used out-of-bounds") | ||
| .cast() | ||
| }) |
There was a problem hiding this comment.
Yeah, this is probably going to be the most annoying part here. Maybe this would be better captured as another parameter to List, so non-allocating but owning uses don't have to worry about this.
workingjubilee
force-pushed
the
experimental-memcx-list
branch
from
8c42257
to
d1ee368
Compare
workingjubilee
force-pushed
the
experimental-memcx-list
branch
from
d1ee368
to
dd422ae
Compare
|
Rebased and added some (~300 words) of documentation on memory contexts. I think I will land this now, so that it's easier to build off of and mutate as-we-go. |
workingjubilee
changed the title
MemCx<'mcx> with List<'mcx, T>
workingjubilee
changed the title
MemCx<'mcx> with List<'mcx, T>MemCx<'mcx> for List<'mcx, T>
List requires reallocation in order to push to it with safe Rust, but it is defined as allocated via palloc, which has problems with lifetimes. In order to head off these problems, and begin work on related ones, introduce a type describing a MemoryContext with a lifetime, including scoped (therefore lifetime-bound) access to the current context.