Iterating over a chain of values

[ethanabrooks]

I have the following struct:

pub struct ScoreMap {
    pub their_tests: f32,
    pub line_coverage: f32,
    pub branch_coverage: f32,
    pub our_tests: BTreeMap<String, f32>,
}

And the following method:

impl ScoreMap {
    pub fn values(&self) -> Vec<&f32> {
        let mut values: Vec<&f32> = self.our_tests.values().into_iter().collect();
        values.push(&self.line_coverage);
        values.push(&self.branch_coverage);
        values.push(&self.their_tests);
        values
    }

Is there a way to rewrite this so that the values method returns an iterator over values instead of collecting all the values into a vector and returning that?

[kornel]

First of all, it's pointless to use &f32 type. The reference to the value is larger than the value itself, and additionally adds complexity of memory management.

fn values(&self) -> impl Iterator<Item=f32>
   self.our_tests.values().iter().copied()
     .chain([e,t,c].iter().copied())
}

[skysch]

You could also use iter::once to create single element iterators to chain together.

[ethanabrooks]

Hey thanks for the suggestion. When I try to implement however, I get:

error[E0599]: no method named `iter` found for struct `std::collections::btree_map::Values<'_, std::string::String, f32>` in the current scope
  --> src/score_map.rs:17:33
   |
17 |         self.our_tests.values().iter().copied().chain(
   |                                 ^^^^ method not found in `std::collections::btree_map::Values<'_, std::string::String, f32>`

I tried removing .iter, since I believe BTreeMap::values() returns a Value<K, V> which implements Iterator:

pub fn values(&self) -> impl Iterator<Item = f32> {
    self.our_tests.values().copied().chain(
        [self.line_coverage, self.branch_coverage, self.their_tests]
            .iter()
            .copied(),
    )
}

Now I get

error: cannot infer an appropriate lifetime
  --> src/score_map.rs:17:24
   |
16 |     pub fn values(&self) -> impl Iterator<Item = f32> {
   |                             ------------------------- this return type evaluates to the `'static` lifetime...
17 |         self.our_tests.values().copied().chain(
   |         -------------- ^^^^^^
   |         |
   |         ...but this borrow...
   |
note: ...can't outlive the anonymous lifetime #1 defined on the method body at 16:5
  --> src/score_map.rs:16:5
   |
16 | /     pub fn values(&self) -> impl Iterator<Item = f32> {
17 | |         self.our_tests.values().copied().chain(
18 | |             [self.line_coverage, self.branch_coverage, self.their_tests]
19 | |                 .iter()
20 | |                 .copied(),
21 | |         )
22 | |     }
   | |_____^
help: you can add a bound to the return type to make it last less than `'static` and match the anonymous lifetime #1 defined on the method body at 16:5
   |
16 |     pub fn values(&self) -> impl Iterator<Item = f32> + '_ {
   |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

In an effort to implement the suggestion of the compiler I changed the implementation to

pub fn values<'a>(&'a self) -> impl Iterator<Item = f32> + 'a {
    self.our_tests.values().copied().chain(
        [self.line_coverage, self.branch_coverage, self.their_tests]
            .iter()
            .copied(),
    )
}

I thought this would mean that the returned Iterator would have lifetime 'a, same as self. However, I get:

error[E0515]: cannot return value referencing temporary value
  --> src/score_map.rs:17:9
   |
17 | /         self.our_tests.values().copied().chain(
18 | |             [self.line_coverage, self.branch_coverage, self.their_tests]
   | |             ------------------------------------------------------------ temporary value created here
19 | |                 .iter()
20 | |                 .copied(),
21 | |         )
   | |_________^ returns a value referencing data owned by the current function

I think I am out of my depth at this point.

[ethanabrooks]

Would you be willing to provide code for your suggestion? I'm a little new to rust. Thanks.

[mbrubeck]

Here's a version that combines kornel's and skysch's suggestions and fixes the lifetime problems:

    fn values<'a>(&'a self) -> impl Iterator<Item=f32> + 'a {
        use std::iter::once;
        self.our_tests.values().copied()
            .chain(once(self.line_coverage))
            .chain(once(self.branch_coverage))
            .chain(once(self.their_tests))
    }

[ethanabrooks]

This is great and worked beautifully
Would you be willing to explain some of it? Why the onces and the copied?

This would really help me understand the lifetime system better.

[Yandros]

First of all, here is a version close to your attempt that does work:

pub fn values (self: &'_ Self) -> impl Iterator<Item = f32> + '_ {
/* equivalent to your:
pub fn values<'a>(self: &'a Self) -> impl Iterator<Item = f32> + 'a { */
    self.our_tests // &'_ HashMap (borrows from *self)
        .values() // impl Iterator<Item = &'_ f32> + '_ (still borrowing from *self)
        .copied() // impl Iterator<Item = f32> + '_ (ditto, but yields f32 instead of &f32)
        .chain(
            vec![self.line_coverage, self.branch_coverage, self.their_tests] // Vec<f32>
                .into_iter() // impl Iterator<Item = f32> + 'static (owned)
        ) // by chaining two iterators of lifetimes `'_` and `'static`,
          // we do manage to get an iterator with lifetime `'_` :)
}

The issue in your code lies in the (second) argument to .chain(); you had:

[self.line_coverage, self.branch_coverage, self.their_tests] // [f32; 3] (a local)
    .iter() // impl Iterator<Item = &'local f32> + 'local  (borrows the local)
    .copied() // impl Iterator<Item = f32> + 'local
) // chaining `'_` with `'local` yields an iterator that cannot outlive either
  // so the resulting iterator cannot outlive the `[f32; 3]` local array
  // so it cannot be used as a return value!

So the issue lies in that you cannot return an iterator which results from an .iter() call on a local value.

The solution is "easy":

use .into_iter() instead of .iter() to return an iterator that takes ownership of the local...

...and is thus part of the returned value (instead of dropped at the end of the function's body).

So you may try:

[self.line_coverage, self.branch_coverage, self.their_tests] // [f32; 3] (a local)
    .into_iter() // ???? (would be + 'static thinks it takes ownership of the local)
    .copied() // impl Iterator<Item = f32> + 'static
) // chaining `'_` with `'static` yields an iterator that cannot outlive either
  // so the resulting iterator is `'_`: fine

but ... sadly you cannot do that with Rust arrays. It is one of the main limitations of the language, and one that is actually likely to be finally fixed, once const generics become stable in rust.

Until then, you can use:

• either a vec![ ... ] which can be .into_iter()ed, but at the cost of a heap allocation (now the three f32 values have to be heap-allocated) instead of the faster stack-allocation of an inline-array;

• or a .chain() of once() / single-value iterators (in other words: [T; N] cannot be .into_iter()ed, but T, when wrapped in a once(), can). So you end up with N calls to chain(once(i_th_value)). This is what @mbrubeck has done.

• a hack when the values are Copy:

  macro_rules! array_into_iter {($array) => ({
      let array = $array;
      (0 .. array.len()).map(move |idx| array[idx])
  })}
  
  // use it as:
  array_into_iter!(
      [self.line_coverage, self.branch_coverage, self.their_tests]
  ) // impl Iterator<Item = f32> + 'static (owns the array!)
  

[ethanabrooks]

This is very clarifying. There is one thing which I do not understand however: if we return a value with a 'local lifetime, doesn't it get moved out of the function? So in the example

[self.line_coverage, self.branch_coverage, self.their_tests] // [f32; 3] (a local)
    .iter() // impl Iterator<Item = &'local f32> + 'local  (borrows the local)
    .copied() // impl Iterator<Item = f32> + 'local
)

wouldn't returning this move it out of the function and so the fact that its lifetime is 'local should not matter, right?

[mbrubeck]

Only the iterator is returned from the function; the array that it borrows from is not. After the function returns, the iterator would hold a dangling reference to that array.

[ethanabrooks]

I see. So the key is into_iter()? Am I correct that this produces a copy of the elements in the iterator and allows them to be moved out of the function? In contrast iter() just creates references to the elements?

[mbrubeck]

Yes, Vec::into_iter returns an iterator that owns its values instead of borrowing them. There is no such iterator in the standard library for arrays, unfortunately.