archlinux / alpm.rs

Rust bindings for libalpm
GNU General Public License v3.0
112 stars 21 forks source link

AlpmList and Lifetimes #12

Closed fosskers closed 3 years ago

fosskers commented 3 years ago

Hey there, a quick question about lifetimes. I had thought that given Db<'a>, anything that comes out of it would also be 'a. Usually that's true, say with Db::pkg. When we call Package::depends(), we get an AlpmList<'a, Dep<'a>>. Okay, still looks good. The issue is when we try to do something with this list. The following doesn't compile:

let v: Vec<_> = p.depends().iter().map(|d| d.name()).collect();

We are told:

error[E0515]: cannot return value referencing function parameter `d`
  --> aura-core/src/deps.rs:71:60
   |
71 |                 let v: Vec<_> = p.depends().iter().map(|d| d.name()).collect();
   |                                                            -^^^^^^^
   |                                                            |
   |                                                            returns a value referencing data owned by the current function
   |                                                            `d` is borrowed here

I was surprised that it thinks the .map owns d, considering iter() is usually the borrowing variant (vs into_iter()). As a consequence I'm having trouble using this AlpmList specifically (other uses of it have been fine).

Is the depends() call freshly allocating, hence iter() owns the contents of the list?

fosskers commented 3 years ago

Notes while reading source code:

pub fn iter(&'b self) -> Iter<'a, 'b, T>
impl<'a, 'b, T> Iterator for Iter<'a, 'b, T>
where
    T: IntoAlpmListItem<'a, 'b>,
{
    type Item = T::Borrow;
    fn next(&mut self) -> Option<Self::Item> {
        let data = self.next_data();

        match data {
            Some(data) => unsafe { Some(T::as_alpm_list_item(self.list.handle, data)) },
            None => None,
        }
    }

    fn size_hint(&self) -> (usize, Option<usize>) {
        let size = unsafe { alpm_list_count(self.list.list) };
        (size, Some(size))
    }
}
unsafe impl<'a, 'b> IntoAlpmListItem<'a, 'b> for Dep<'a> {
    type Borrow = Self;
    unsafe fn into_alpm_list_item(_handle: &'a Alpm, ptr: *mut c_void) -> Self {
        Dep::from_ptr(ptr as *mut alpm_depend_t)
    }

    unsafe fn as_alpm_list_item(_handle: &'a Alpm, ptr: *mut c_void) -> Self::Borrow {
        Dep::from_ptr(ptr as *mut alpm_depend_t)
    }
}
pub struct Dep<'a> {
    pub(crate) inner: *mut alpm_depend_t,
    pub(crate) phantom: PhantomData<&'a ()>,
}

impl<'a> Dep<'a> {
    pub(crate) unsafe fn from_ptr(ptr: *mut alpm_depend_t) -> Dep<'a> {
        Dep {
            inner: ptr,
            phantom: PhantomData,
        }
    }
}

Okay, so a new Dep wrapper is indeed getting allocated. Although I'm still not 100% confident about what that implies about the lifetimes. If we claimed it was 'a, the same 'a as the Db, would Rust let it float around in memory even outside the .map()?

Morganamilo commented 3 years ago

Dep is to Depend as &str is to String. So there's so real allocation going on if you meant it in that way.

Yes it's safe for dep to have the lifetime of dep to be 'a because the depend lives in the Db. So you can drop the list/package/db and it's still valid.

However .name on dep borrows self to that's what you're running into. I'll see if that can be extended when I'm home.

Morganamilo commented 3 years ago

Also please don't show me the code, it's awful and burns. But it was the only way to make functions accept iterators and alpmlists while being zero cost.

fosskers commented 3 years ago

it's awful and burns

I wasn't judging :laughing:

I'll see if that can be extended when I'm home.

Thank you!

Morganamilo commented 3 years ago

Should be fixed now.

fosskers commented 3 years ago

Like a charm :+1: