Hide Ref implementation and Box it's iterator

Hello,
I have the following trait:

pub trait Item {
   pub fn labels(&self) -> &Vec<String>;
}

But since some Item implementors need interior mutability I have come to change the trait definition:

pub trait Item {
     pub fn labels(&self) -> Ref<Vec<String>>;
}

Also, I would like to be able to use my implementor such as:

fn shop() -> Box<Iterator<Item = String>> {
    let i1 = ItemImplementor::new();
    Box::new(i1.labels().into_iterator())
}

The two issue I'm facing are:

  1. I have had to change Item API because of an implementation needed a RefCell which leaks it's Ref to the API. Is it possible to avoid that?
  2. At some point I want to boxed iterator over a Ref but it never works.

I tried to create a wrapper struct:

pub struct VecRefWrapper<'a, T: 'a> {
    pub r: Ref<'a, Vec<T>>
}

impl<'a, 'b: 'a, T: 'a> IntoIterator for &'b VecRefWrapper<'a, T> {
    type IntoIter = Iter<'a, T>;
    type Item = &'a T;

    fn into_iter(self) -> Iter<'a, T> {
        self.r.iter()
    }
}

And change my Item implementation like that:

pub trait Item {
   pub fn labels(&self) -> VecRefWrapper<String>;
}

But then I can't do point #2:

fn shop<'a>() -> Box<dyn Iterator<Item = &'a String> + 'a> {
    let i1 = ItemImplementor::new();
    Box::new(i1.labels().into_iter())
}

shop();

Which gives the following error:

cannot return value referencing temporary value
returns a value referencing data owned by the current function

use `.collect()` to allocate the iterator

Note that I would like to avoid .collect() as much as possible. But, if there is no other way...
Any idea on how to solve those two issues?

In general, Rust won't let you abstract away memory management. Not caring about how things are allocated and freed is a privilege of garbage-collected languages. Rust tracks memory management and memory safety via types, so all this will keep "leaking" via types.

The problem you have is that labels() borrows from i1. If something is temporarily borrowed from somewhere else, that's an unbreakable relationship that can't be hidden. This can't be "undone" with into_iterator() (into_* would be correct way to pass ownership of owned items, but it doesn't help for borrows).

Ref is valid for only as long as i1 is in scope. The scope of that variable is the inside of the shop function, so Rust correctly stops you from returning an iterator that is no longer allowed to access the data it iterates.

For other reasons, Rust doesn't allow self-referential structs, so you can't return i1 and its Ref at the same time either.

Can you return ItemImplementor? Or Arc<ItemImplementor> if you need only one instance of it? That would allow the caller to keep it in scope for as long as they need labels() to iterate. In these cases if something doesn't live long enough in your function, make the caller create/hold it.

3 Likes

Contrary to the previous comment, there is nothing fundamental here that would prevent this from working, as i1 just needs to move into the dynamic object. There would be a way with a wrapper struct

That said, I'm not sure it's feasible to help with the information you've given, or rather, there's some choices here which need elaboration, otherwise it may get too complex when it doesn't need to be.

Can you elaborate on the use case where you needed interior mutability? It's not too frequent that something like that is needed and can't be worked around here. Also, &Vec<String> in general can be replaced by &[String] which will be better for performance and readability. The cargo clippy command can show some opportunities to make these sorts of improvements for your project.

1 Like

Thanks for the answers.
To elaborate more on my usecase, I have got a Shop struct which is my "source of truth".
The Shop contains a cache where items are loaded and stored in an HashMap. Then, the Item struct is contains two field, one to access the Shop (and so, the cache) and one for the id of the Item. It looks like this:

struct CachedItem {
    labels: Vec<String>,
     // Other fields ommited for clarity
}

struct Cache {
    items: HashMap<String, CachedItem>
}

impl Cache {
     pub fn get_or_load_item(&mut self, id: &String) -> CachedItem {
          todo!();
     }
}

struct InnerShop {
     cache: RefCell<Cache>,
     // Other fields ommited for clarity
}

struct Shop(Rc<InnerShop>);

impl Shop {
    pub fn cache_mut(&mut self) -> &mut Cache {
         // Error here, I must use Ref::map and expose the Ref 
         // or RefMut guard everywhere...
         self.0.cache
    }
}

trait Item {
    fn labels(&self) -> Vec<String>;
}

struct ShopItem {
     id: String,
     shop: Shop,
}

impl ShopItem {
    pub fn get_or_load_item_in_cache(&mut self) -> &mut CachedItem {
        self.shop.cache_mut().get_or_load_item(&self.id)
    }
}

impl Item for ShopItem {
    fn labels() -> &[String] {
        let cached_item = self.get_or_load_item_in_cache();
        cached_item.labels()
    }

    // Other functions ommited for clarity
}

Then, I would like to be able to potentially put in a boxed iterator the result of the labels() function. For processing items in a lazy manner, since there may be alot!.

Ah okay, that clarifies it. Yes you've hit a pretty sore point.

With RefCell the only way you can do something like this is by locking the entire structure immutably. The boxed iterator will own this Ref<'_, ...> and also borrow it for the iteration. Unfortunately that would turn rather hairy as it creates what's known as a self-referential struct. I tried a couple approaches here and one way or another you're going to run into this. I would recommend creating a struct for your Iterator and using the ouroboros crate to help create a self-reference. Note that you can avoid the Ref around the HashMap by creating another layer of indirection (making the elements Rc) or using something like the not-very-mature cachemap crate

#[self_referencing]
struct Labels<'a> {
    map_borrow: Ref<'a, CachedItem>,
    #[borrows(map_borrow)]
    labels: std::slice::Iter<'this, &'a String>,
}

This is a starting point which I think will be helpful, but don't be afraid to ask, like I said above it's one of the messier cases in Rust, but you'll be able to handle it this way.

By the way, you should avoid having &String, as this has two indirections, it's a reference to a pointer to a heap-allocated string buffer. Instead prefer &str, this is a reference to the data of the string itself. In this case, you can remove the layer of indirections by mapping with the function |string| &*string which removes the indirection to turn it into a &str via the Deref trait

1 Like

Thanks a lot for the points you have raised, I'm going to investigate them. One question trough, using this:

#[self_referencing]
struct Labels<'a> {
    map_borrow: Ref<'a, CachedItem>,
    #[borrows(map_borrow)]
    labels: std::slice::Iter<'this, &'a String>,
}

Would it be possible to chain and box the Labels struct? Such as:

fn test(items: HashMap<String, CachedItem>) -> Box<dyn Iterator<Item = Labels>> {
    // Here the i.labels() function would return the struct that you defined.
    // Let's suppose that it implement the iterator trait, would this works?
    let iter = items.map(|i| i.labels());
    Box::new(iter)
}

Yes @Loustak , it should be possible to Box that.

There's one detail to dynamic implementations that is sometimes hidden when it's not necessary to specify but may be where you have questions. In general when you see dyn Trait, this type can also have a lifetime parameter, so it is often either dyn Trait + 'static, meaning something which implements the trait and borrows nothing, or dyn Trait + '_, i.e. an implicit, inferred lifetime such as when you have a function like fn doit(&self) -> ...

Apologies on one detail, I believe I mixed up where the RefCell was. It wraps the hash map, not the CachedItem.

One situation would be something like this (where cache_mut returns RefMut and we fix the field to RefMut<'a, Cache>)

fn test<'a>(&mut self, id: &'a str) -> Box<dyn Iterator<Item=Labels> + '_>> {
   let map_borrow: RefMut<'_, Cache> = self.shop.cache_mut();
   let labels = LabelBuilder {
        map_borrow,
        labels_builder: |cache: &mut RefMut<'_, Cache>| {
           cache.get_or_load_item(id).labels()
        }
    }.build();
    Box::new(labels);
}

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.