17

I have a Fibonacci struct that can be used as an iterator for anything that implements One, Zero, Add and Clone. This works great for all integer types.

I want to use this struct for BigInteger types which are implemented with a Vec and are expensive to call clone() on. I would like to use Add on two references to T which then returns a new T (no cloning then).

For the life of me I can't make one that compiles though...

Working:

extern crate num;

use std::ops::Add;
use std::mem;
use num::traits::{One, Zero};

pub struct Fibonacci<T> {
    curr: T,
    next: T,
}

pub fn new<T: One + Zero>() -> Fibonacci<T> {
    Fibonacci {
        curr: T::zero(),
        next: T::one(),
    }
}

impl<'a, T: Clone + Add<T, Output = T>> Iterator for Fibonacci<T> {
    type Item = T;

    fn next(&mut self) -> Option<T> {
        mem::swap(&mut self.next, &mut self.curr);
        self.next = self.next.clone() + self.curr.clone();
        Some(self.curr.clone())
    }
}

#[test]
fn test_fibonacci() {
    let first_12 = new::<i64>().take(12).collect::<Vec<_>>();
    assert_eq!(vec![1, 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144], first_12);
}

Desired:

extern crate num;

use std::ops::Add;
use std::mem;
use num::traits::{One, Zero};

pub struct Fibonacci<T> {
    curr: T,
    next: T,
}

pub fn new<T: One + Zero>() -> Fibonacci<T> {
    Fibonacci {
        curr: T::zero(),
        next: T::one(),
    }
}

impl<'a, T: Clone + 'a> Iterator for Fibonacci<T>
where
    &'a T: Add<&'a T, Output = T>,
{
    type Item = T;

    fn next(&mut self) -> Option<T> {
        mem::swap(&mut self.next, &mut self.curr);
        self.next = &self.next + &self.curr;
        Some(self.curr.clone())
    }
}

#[test]
fn test_fibonacci() {
    let first_12 = new::<i64>().take(12).collect::<Vec<_>>();
    assert_eq!(vec![1, 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144], first_12);
}

This gives the error 

error[E0495]: cannot infer an appropriate lifetime for borrow expression due to conflicting requirements
  --> src/main.rs:27:21
   |
27 |         self.next = &self.next + &self.curr;
   |                     ^^^^^^^^^^
   |
note: first, the lifetime cannot outlive the anonymous lifetime #1 defined on the method body at 25:5...
  --> src/main.rs:25:5
   |
25 | /     fn next(&mut self) -> Option<T> {
26 | |         mem::swap(&mut self.next, &mut self.curr);
27 | |         self.next = &self.next + &self.curr;
28 | |         Some(self.curr.clone())
29 | |     }
   | |_____^
note: ...so that reference does not outlive borrowed content
  --> src/main.rs:27:21
   |
27 |         self.next = &self.next + &self.curr;
   |                     ^^^^^^^^^^
note: but, the lifetime must be valid for the lifetime 'a as defined on the impl at 19:1...
  --> src/main.rs:19:1
   |
19 | / impl<'a, T: Clone + 'a> Iterator for Fibonacci<T>
20 | | where
21 | |     &'a T: Add<&'a T, Output = T>,
22 | | {
...  |
29 | |     }
30 | | }
   | |_^
note: ...so that types are compatible (expected std::ops::Add, found std::ops::Add<&'a T>)
  --> src/main.rs:27:32
   |
27 |         self.next = &self.next + &self.curr;
   |                                ^

error[E0495]: cannot infer an appropriate lifetime for borrow expression due to conflicting requirements
  --> src/main.rs:27:34
   |
27 |         self.next = &self.next + &self.curr;
   |                                  ^^^^^^^^^^
   |
note: first, the lifetime cannot outlive the anonymous lifetime #1 defined on the method body at 25:5...
  --> src/main.rs:25:5
   |
25 | /     fn next(&mut self) -> Option<T> {
26 | |         mem::swap(&mut self.next, &mut self.curr);
27 | |         self.next = &self.next + &self.curr;
28 | |         Some(self.curr.clone())
29 | |     }
   | |_____^
note: ...so that reference does not outlive borrowed content
  --> src/main.rs:27:34
   |
27 |         self.next = &self.next + &self.curr;
   |                                  ^^^^^^^^^^
note: but, the lifetime must be valid for the lifetime 'a as defined on the impl at 19:1...
  --> src/main.rs:19:1
   |
19 | / impl<'a, T: Clone + 'a> Iterator for Fibonacci<T>
20 | | where
21 | |     &'a T: Add<&'a T, Output = T>,
22 | | {
...  |
29 | |     }
30 | | }
   | |_^
note: ...so that reference does not outlive borrowed content
  --> src/main.rs:27:34
   |
27 |         self.next = &self.next + &self.curr;
   |                                  ^^^^^^^^^^
Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
dten
  • 2,364
  • 21
  • 28
  • You may be interested in [std::ops::AddAssign](https://doc.rust-lang.org/std/ops/trait.AddAssign.html) whose RFC is in the Final Comment period: it lets you overload the "+=" operator. This would allow avoiding `.clone()` calls for the addition, at least. – Matthieu M. Jan 06 '16 at 10:50
  • That would be one fewer clone :) I can't get rid of both though. 12+ weeks until that though I guess.. – dten Jan 06 '16 at 11:57
  • Two fewer clones actually: `self.next = self.next.clone() + self.curr.clone();` would be replaced by `self.next += &self.curr;`. – Matthieu M. Jan 06 '16 at 13:10
  • 1
    Strange, I thought the "cannot infer lifetime … due to conflicting requirements" _listed_ those conflicting requirements. – Jan Hudec Jan 06 '16 at 13:23

1 Answers1

22

How to write a trait bound for adding two references of a generic type?

Let's start with a simplified example:

fn add_things<T>(a: &T, b: &T) {
    a + b;
}

This has the error

error[E0369]: binary operation `+` cannot be applied to type `&T`
 --> src/lib.rs:2:5
  |
2 |     a + b;
  |     ^^^^^
  |
  = note: an implementation of `std::ops::Add` might be missing for `&T`

As the compiler hints, we need to guarantee that Add is implemented for &T. We can express that directly by adding an explicit lifetime to our types and also using that in our trait bounds:

use std::ops::Add;

fn add_things<'a, T>(a: &'a T, b: &'a T)
where
    &'a T: Add,
{
    a + b;
}

Next, let's try a slightly different approach — instead of being handed a reference, we will create one inside the function:

fn add_things<T>(a: T, b: T) {
    let a_ref = &a;
    let b_ref = &b;

    a_ref + b_ref;
}

We get the same error:

error[E0369]: binary operation `+` cannot be applied to type `&T`
 --> src/lib.rs:5:5
  |
5 |     a_ref + b_ref;
  |     ^^^^^^^^^^^^^
  |
  = note: an implementation of `std::ops::Add` might be missing for `&T`

However, trying to add the same fix as before doesn't work. It's also a bit awkward because the lifetime isn't associated with any of the arguments passed in:

use std::ops::Add;

fn add_things<'a, T: 'a>(a: T, b: T)
where
    &'a T: Add,
{
    let a_ref = &a;
    let b_ref = &b;

    a_ref + b_ref;
}

error[E0597]: `a` does not live long enough
  --> src/lib.rs:7:17
   |
3  | fn add_things<'a, T: 'a>(a: T, b: T)
   |               -- lifetime `'a` defined here
...
7  |     let a_ref = &a;
   |                 ^^
   |                 |
   |                 borrowed value does not live long enough
   |                 assignment requires that `a` is borrowed for `'a`
...
11 | }
   | - `a` dropped here while still borrowed

Placing the 'a lifetime on the impl means that the caller of the method gets to determine what the lifetime should be. Since the reference is taken inside the method, the caller can never even see what that lifetime would be.

Instead, you want to place a restriction that a reference of an arbitrary lifetime implements a trait. This is called a Higher Ranked Trait Bound (HRTB):

use std::ops::Add;

fn add_things<T>(a: T, b: T)
where
    for<'a> &'a T: Add,
{
    let a_ref = &a;
    let b_ref = &b;

    a_ref + b_ref;
}

Applied back to your original code, you were very close:

impl<T> Iterator for Fibonacci<T>
where
    T: Clone,
    for<'a> &'a T: Add<Output = T>,
{
    type Item = T;

    fn next(&mut self) -> Option<T> {
        mem::swap(&mut self.next, &mut self.curr);
        self.next = &self.next + &self.curr;
        Some(self.curr.clone())
    }
}

See also:

Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
  • Do you know how to eliminate the last clone? Intuitively I would think that the actual number would have to be *outside* the iterator, but I could not make it work. – Matthieu M. Jan 06 '16 at 14:51
  • @MatthieuM. I don't see how it would be possible to eliminate that clone, since the `current` value is saved by the struct but we also want to return it from the iterator. My next thought would be to return a reference to `current`, but that would require putting a lifetime on `self` which is a no-go. Can you expand on your "outside the iterator" idea? – Shepmaster Jan 06 '16 at 14:56
  • My idea was to create a State `struct` to hold the state and then another Iterator `struct` referencing the first (`&mut`) and mutating it as it "advances" so that the Iterator could return references into the State; but I could not manage to align my borrows. – Matthieu M. Jan 06 '16 at 14:59
  • @MatthieuM. interesting. Something like [this](http://is.gd/fRblHv)? It seems to have the same problems as returning a reference into the iterator itself. It seems like these two implementations must be isomorphic, but I can't quite place my finger on why. – Shepmaster Jan 06 '16 at 15:08
  • @MatthieuM. it's got to be down to aliasing. If we returned a reference, that reference would have to die before the subsequent call to `next`, otherwise there'd be aliasing. Especially the bad kind where there's a mutable reference and immutable references. – Shepmaster Jan 06 '16 at 15:16
  • Something like http://is.gd/NmZFNs and yes I suspect the issue is that the returned reference would need to borrow the current iterator to ensure it does not iterate under our feet. – Matthieu M. Jan 06 '16 at 15:18