Temporary value dropped while borrowed while pushing elements into a Vec

Viewed 300

I'm trying to solve the RPN calculator exercise at exercism but stumbled upon this temporary value dropped while borrowed error that I can't seem to work out.

Here's my code:

#[derive(Debug)]
pub enum CalculatorInput {
    Add,
    Subtract,
    Multiply,
    Divide,
    Value(i32),
}

pub fn evaluate(inputs: &[CalculatorInput]) -> Option<i32> {
    let mut stack = Vec::new();

    for input in inputs {
        match input {
            CalculatorInput::Value(value) => {
                stack.push(value);
            },
            operator => {
                if stack.len() < 2 {
                    return None;
                }
                let second = stack.pop().unwrap();
                let first = stack.pop().unwrap();
                let result = match operator {
                  CalculatorInput::Add => first + second,
                  CalculatorInput::Subtract => first - second,
                  CalculatorInput::Multiply => first * second,
                  CalculatorInput::Divide => first / second,
                  CalculatorInput::Value(_) => return None,
                };
                stack.push(&result.clone());
            }
        }
    }
    if stack.len() != 1 {
        None
    } else {
        Some(*stack.pop().unwrap())
    }
}

And the error I get:

error[E0716]: temporary value dropped while borrowed
  --> src/lib.rs:32:29
   |
32 |                 stack.push(&result.clone());
   |                             ^^^^^^^^^^^^^^ - temporary value is freed at the end of this statement
   |                             |
   |                             creates a temporary which is freed while still in use
...
36 |     if stack.len() != 1 {
   |        ----- borrow later used here
   |
   = note: consider using a `let` binding to create a longer lived value

If I understand correctly, the variable result is no loger live outside of the for loop (outside of the operator match branch indeed), that's why I cloned it, but it still gives me the same error.

How can I make a copy of the result which is owned by the stack Vec (if that's what I should do)?


Just for reference, and in case anybody fins this useful, this is the final solution taking into account all the help received:

use crate::CalculatorInput::{Add,Subtract,Multiply,Divide,Value};

#[derive(Debug)]
pub enum CalculatorInput {
    Add,
    Subtract,
    Multiply,
    Divide,
    Value(i32),
}

pub fn evaluate(inputs: &[CalculatorInput]) -> Option<i32> {
    let mut stack: Vec<i32> = Vec::new();

    for input in inputs {
        match input {
            Value(value) => {
                stack.push(*value);
            },
            operator => {
                if stack.len() < 2 {
                    return None;
                }
                let second: i32 = stack.pop().unwrap();
                let first: i32 = stack.pop().unwrap();

                let result: i32 = match operator {
                  Add => first + second,
                  Subtract => first - second,
                  Multiply => first * second,
                  Divide => first / second,
                  Value(_) => return None,
                };
                stack.push(result);
            }
        }
    }
    if stack.len() != 1 {
        None
    } else {
        stack.pop()
    }
}

No need to clone, because i32 implements the Copy trait.

The problem was that my vec was receiving an &i32 instead of i32, and thus rust infered it to be a Vec<&i32>.

2 Answers

The error is because Rust did not infer the type you expected.

In your code, the type of value is inferred to be &i32 because input is a reference of a element in inputs, and you push a value later, therefore the type of stack is inferred to be Vec<&i32>.

A best fix is to explicitly specify the type of stack:

let mut stack: Vec<i32> = Vec::new();

And because i32 has implemented Copy trait, you should never need to clone a i32 value, if it is a reference, just dereference it.

Fixed code:

#[derive(Debug)]
pub enum CalculatorInput {
    Add,
    Subtract,
    Multiply,
    Divide,
    Value(i32),
}

pub fn evaluate(inputs: &[CalculatorInput]) -> Option<i32> {
    let mut stack: Vec<i32> = Vec::new();

    for input in inputs {
        match input {
            CalculatorInput::Value(value) => {
                stack.push(*value);
            }
            operator => {
                if stack.len() < 2 {
                    return None;
                }
                let second = stack.pop().unwrap();
                let first = stack.pop().unwrap();
                let result = match operator {
                    CalculatorInput::Add => first + second,
                    CalculatorInput::Subtract => first - second,
                    CalculatorInput::Multiply => first * second,
                    CalculatorInput::Divide => first / second,
                    CalculatorInput::Value(_) => return None,
                };
                stack.push(result);
            }
        }
    }
    if stack.len() != 1 {
        None
    } else {
        Some(stack.pop().unwrap())
    }
}

You have the same behavior with this simple exemple

fn main() {
    let mut stack = Vec::new();
    
    let a = String::from("test");
    stack.push(&a.clone());
    //-------- ^

    println!("{:?}", stack);
}

and the good way is to not borrow when clone.

fn main() {
    let mut stack = Vec::new();
    
    let a = String::from("test");
    stack.push(a.clone());
    //-------- ^
    
    println!("{:?}", stack);
}

The variable should be used like this stack.push(result.clone()); and change code like this

pub fn evaluate(inputs: &[CalculatorInput]) -> Option<i32> {
    let mut stack: Vec<i32> = Vec::new();
    //---------------- ^
    for input in inputs {
        match input {
            CalculatorInput::Value(value) => {
                stack.push(value.clone());
               //----------------- ^
            },
            operator => {
                if stack.len() < 2 {
                    return None;
                }
                let second = stack.pop().unwrap();
                let first = stack.pop().unwrap();
                let result = match operator {
                  CalculatorInput::Add => first + second,
                  CalculatorInput::Subtract => first - second,
                  CalculatorInput::Multiply => first * second,
                  CalculatorInput::Divide => first / second,
                  CalculatorInput::Value(_) => return None,
                };
                
                stack.push(result.clone());
                        //-^
            }
        }
    }
    if stack.len() != 1 {
        None
    } else {
        Some(stack.pop().unwrap())
    //------- ^
    }
}
Related