Lifetime for Iterator item

Hello!

I've got problem related to return a raw pointer and Iterator trait. Look at the code, please.

As I get I should point how many time the value that is returned lives, right? And actually it should live shorter than 'a but I point type Item = &'a mut u32 because I don't understand how to introduce new lifetime parameter. As far as I get a lifetime parameter is bound to some property and I don't see where to bound new one. There is should be a lifetime 'b that is covered by 'a. So this lifetime will be actual for type Item in the Iterator implementation. Does it make sense? If yes, how to introduce it? If not, how can I get this code compiled?

In your case there is no guarantee that self.wrapped_core.get_val() won't return the same reference over and over again, and that would violate aliasing rules.

If you can ensure it returns Some once, and then None forever, then it would be valid, but you'd have to use unsafe to cast the lifetime to convince the borrow checker that you know it's safe.


Keep in mind that the Iterator interface guarantees that this usage is safe:

let iter = collection.iter();
let a = iter.next().unwrap();
let b = iter.next().unwrap();
drop(iter);
use_both(a, b);

So the iterator can't return anything that lives in the iterator (is not borrowed), because it's valid to destroy the iterator and its contents before using elements it returned.

Mutable iterator also can't reuse a value (or return same thing twice), because it's valid to use all of returned values simultaneously, and mutable values in Rust guarantee exclusive access.

For example .collect() relies on all of these properties.

7 Likes

In your case there is no guarantee that self.wrapped_core.get_val() won’t return the same reference over and over again

I realize it but the code is simplified version of actual code to get a potential helper focused on the issue.

If you can ensure it returns Some once, and then None forever, then it would be valid, but you’d have to use unsafe to cast the lifetime to convince the borrow checker that you know it’s safe.

Does it mean I should make casting from a raw pointer to a reference within next() method? I would prefer to use a method that does it because I would like to keep unsafe code only in Core structure. To keep raw pointer related functionality in one place of code. If I do some unsafe code outside I spread it.

I have refactored a little bit your code to make it sound (tl,dr: most functions are now tagged unsafe fn):

// whenever you have structs with lifetimes,
// having this lint is highly recommended
#![deny(elided_lifetimes_in_paths)]

// abstraction boundary to enforce safety guarantees
pub use self::lib::*;
mod lib {
    pub
    struct Core (*mut u32);
    
    impl Core {
        pub
        fn new (ptr: *mut u32) -> Self
        {
            Self (ptr)
        }
    }
    
    impl Core {
        // nothing guarantees that the pointer is valid etc. => unsafe fn
        pub unsafe
        fn iter_mut<'core> (
            self: &'core mut Self,
        ) -> impl Iterator<Item = &'core mut u32>
        {
            IterMut::new(self)
        }
    }
    
    pub
    struct IterMut<'core> {
        wrapped_core: &'core mut Core,
    }
    
    impl<'core> IterMut<'core> {
        // since the iterator yields unsafe &mut references (aliasing etc.)
        // usage of the iterator is `unsafe`; but since `Iterator::next()`
        // cannot be tagged `unsafe`, the tag moves up to all construction
        // sites / functions
        unsafe // this could be pub, but it is not needed
        fn new (
            c: &'core mut Core,
        ) -> Self
        {
            Self { wrapped_core: c }
        }
        
    }
    
    impl<'core> Iterator for IterMut<'core> {
        type Item = &'core mut u32;
    
        // since Self cannot be constructed without `unsafe`,
        // this does not need `unsafe`
        fn next (
            self: &'_ mut Self,
        ) -> Option<&'core mut u32>
        {
            unsafe {
                self.wrapped_core.0.as_mut::<'core>()
            }
        }
    }
}

fn main ()
{
    let mut x = 123;
    dbg!(x);
    unsafe {
        // safety: &mut x is a valid pointer within this scope,
        // and .next() is only called once.
        let mut dummy_core = Core::new(&mut x);
        let mut dummy_wrapper = dummy_core.iter_mut();
        let val = dummy_wrapper.next();
        if let Some(x) = val {
            *x = 33;
        };
    }
    dbg!(x);
}

Now, to get rid of some unsafe-ty, (although the unsafe fn must remain because we are dereferencing raw (potentially invalid) pointers) we can:

  • yield shared &'core u32 references;

  • Store the &'core mut u32 in an Option, and .take() it when yielding, which ensures it being yielded only once. At which point we are just reimplementing Option::into_iter() logic, so we might as well just use it:

    pub
    struct Core (*mut u32);
    
    impl Core {
        pub
        fn new (ptr: *mut u32) -> Self
        {
            Self (ptr)
        }
    }
    
    impl Core {
        // nothing guarantees that the pointer is valid etc. => unsafe fn
        pub unsafe
        fn iter_mut<'core> (
            self: &'core mut Self,
        ) -> impl Iterator<Item = &'core mut u32>
        {
            self.0.as_mut::<'core>().into_iter()
        }
    }
    
1 Like

It appears I don't need implement iterator. The elements it returns can live shorter than core. Even more they can live shorter than iterator.

@Yandros thank you for your efforts but I try to implement safe interface. I need to think about design.

You know it's great that the language insist to think about design in such fashion. If there is no marker about how many time returned data should live I could make a big mistake.

Should I mark the thread as solved?

That's up to you. If you are satisfied with the answers so far, and don't want other people to add to the existing comments, just edit the thread title by prepending the prefix "[Solved] ".

In that case the StreamingIterator interface may be appropriate.

then I am confused by the raw pointer: is it owned (you know it cannot possibly be freed elsewhere)? If so, then use Box<_>. Is it borrowed? If so, use a reference. I don't think you can offer a non-unsafe API by wrapping a raw pointer. At the very least the construction of Core would need to be unsafe.


If you tell us what you are actually trying to build, we will be able to give you more accurate advice :slight_smile:

Actual structure a bit more complex. There is a wrapper around raw pointer make some checks, the core is consists of some entities with sharing politics of the pointers etc. No worries. It's a simple test project. Nothing super important. Thank you so much. Great community!

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.