5
\$\begingroup\$

I'd like to improve how ownership is handled within a simple interpreter written in Rust. The interpreter takes simple calculator like expressions and returns either a string of the AST or a number of the evaluation:

// Simple expression for working on numbers
enum Exp {
    Add { e1 : Box <Exp>, e2 : Box <Exp> },
    Mul { e1 : Box <Exp>, e2 : Box <Exp> },
    Int { i : i32}
}

// Evaluate an expression to a number
fn exp_to_num(e : Exp) -> i32 {
    match e {
        Exp::Add {e1,e2} => return exp_to_num(*e1) + exp_to_num(*e2),
        Exp::Mul {e1,e2} => return exp_to_num(*e1) * exp_to_num(*e2),
        Exp::Int {i} => return i
    }
}

// Evaluate an expression to a string
fn exp_to_str(e : Exp) -> String {
    match e {
        Exp::Add {e1,e2} =>
            return format!("({}) + ({})",exp_to_str(*e1),exp_to_str(*e2)),
        Exp::Mul {e1,e2} =>
            return format!("({}) * ({})",exp_to_str(*e1),exp_to_str(*e2)),
        Exp::Int {i} =>
            return format!("{}",i)
    }
}

fn main() {

    // Create an expression
    let e1 = Exp::Add {
        e1 : Box::new(Exp::Mul {
            e1 : Box::new(Exp::Int { i : 2 }),
            e2 : Box::new(Exp::Int { i : 3 })}),
        e2 : Box::new(Exp::Int {i : 4})};

    // Evaluate the expression
    println!("e1 : {}", exp_to_num(e1));
    //println!("e1 : {}", exp_to_str(e1));
}

The last command can not be uncommented since it appears as though exp_to_num takes ownership of e1. I'd like to fix this.

In C++, I'd just make the arguments const &. In Rust, my best attempt at this is the following code:

// Simple expression for working on numbers
enum Exp {
    Add { e1 : Box <Exp>, e2 : Box <Exp> },
    Mul { e1 : Box <Exp>, e2 : Box <Exp> },
    Int { i : i32}
}

// Evaluate an expression to a number
fn exp_to_num(e : & Exp) -> i32 {
    match *e {
        Exp::Add {ref e1,ref e2} => return exp_to_num(&*e1) + exp_to_num(&*e2),
        Exp::Mul {ref e1,ref e2} => return exp_to_num(&*e1) * exp_to_num(&*e2),
        Exp::Int {i} => return i
    }
}

// Evaluate an expression to a string
fn exp_to_str(e : & Exp) -> String {
    match *e {
        Exp::Add {ref e1,ref e2} =>
            return format!("({}) + ({})",exp_to_str(&*e1),exp_to_str(&*e2)),
        Exp::Mul {ref e1,ref e2} =>
            return format!("({}) * ({})",exp_to_str(&*e1),exp_to_str(&*e2)),
        Exp::Int {i} =>
            return format!("{}",i)
    }
}

fn main() {

    // Create an expression
    let e1 = Exp::Add {
        e1 : Box::new(Exp::Mul {
            e1 : Box::new(Exp::Int { i : 2 }),
            e2 : Box::new(Exp::Int { i : 3 })}),
        e2 : Box::new(Exp::Int {i : 4})};

    // Evaluate the expression
    println!("e1 : {}", exp_to_num(&e1));
    println!("e1 : {}", exp_to_str(&e1));
}

Though this works, it feels verbose. Specifically, it feels verbose to match references, Exp::Add {ref e1,ref e2}, and verbose to find a reference to an unboxed expression, exp_to_num(&*e1). Can passing a constant reference be made more concise and compact?

Alternatively, we could just clone the memory, which gives

// Simple expression for working on numbers
#[derive(Clone)]
enum Exp {
    Add { e1 : Box <Exp>, e2 : Box <Exp> },
    Mul { e1 : Box <Exp>, e2 : Box <Exp> },
    Int { i : i32}
}

// Evaluate an expression to a number
fn exp_to_num(e : Exp) -> i32 {
    match e {
        Exp::Add {e1,e2} => return exp_to_num(*e1) + exp_to_num(*e2),
        Exp::Mul {e1,e2} => return exp_to_num(*e1) * exp_to_num(*e2),
        Exp::Int {i} => return i
    }
}

// Evaluate an expression to a string
fn exp_to_str(e : Exp) -> String {
    match e {
        Exp::Add {e1,e2} =>
            return format!("({}) + ({})",exp_to_str(*e1),exp_to_str(*e2)),
        Exp::Mul {e1,e2} =>
            return format!("({}) * ({})",exp_to_str(*e1),exp_to_str(*e2)),
        Exp::Int {i} =>
            return format!("{}",i)
    }
}

fn main() {

    // Create an expression
    let e1 = Exp::Add {
        e1 : Box::new(Exp::Mul {
            e1 : Box::new(Exp::Int { i : 2 }),
            e2 : Box::new(Exp::Int { i : 3 })}),
        e2 : Box::new(Exp::Int {i : 4})};

    // Evaluate the expression
    println!("e1 : {}", exp_to_num(e1.clone()));
    println!("e1 : {}", exp_to_str(e1.clone()));
}

This also feels somewhat verbose because we have to call clone rather than just allowing an implicit copy. Is a clone required due to Box? Can this be made more concise?

\$\endgroup\$
0

1 Answer 1

6
\$\begingroup\$
  1. Learn to love rustfmt. It will correct your code to the idiomatic Rust style.

  2. Learn to love clippy. It will provide more lints to change your code to be more idiomatic. For example, it identifies many unneeded return statements in your code.

  3. When you accept a reference, you do not need to dereference and reference it. You can just use the variable directly.

  4. Recent Rust versions removed the need to use ref in patterns when it's "obviously" the right thing. It also removed the need to match on a dereferenced variable (match *foo).

  5. Instead of calling format!("{}", foo), just call foo.to_string().

  6. Instead of having functions called foo_bar taking a Foo as the first argument, make them methods. Methods can automatically take references to their self parameter.

  7. Naming the subexpressions and integer value doesn't really bring much value. I'd switch to a tuple struct.

  8. Bringing the variants of the enum into scope in a method (use Foo::*) can reduce duplication.

  9. Instead of creating a custom to_str method, implement Display.

  10. Use /// for documentation comments.

use std::fmt;

/// Simple expression for working on numbers
enum Exp {
    Add(Box<Exp>, Box<Exp>),
    Mul(Box<Exp>, Box<Exp>),
    Int(i32),
}

impl Exp {
    /// Evaluate an expression to a number
    fn to_num(&self) -> i32 {
        use Exp::*;
        match self {
            Add(e1, e2) => e1.to_num() + e2.to_num(),
            Mul(e1, e2) => e1.to_num() * e2.to_num(),
            Int(i) => *i,
        }
    }
}

impl fmt::Display for Exp {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        use Exp::*;
        match self {
            Add(e1, e2) => write!(f, "({}) + ({})", e1, e2),
            Mul(e1, e2) => write!(f, "({}) * ({})", e1, e2),
            Int(i) => write!(f, "{}", i),
        }
    }
}

fn main() {
    // Create an expression
    let e1 = Exp::Add(
        Box::new(Exp::Mul(Box::new(Exp::Int(2)), Box::new(Exp::Int(3)))),
        Box::new(Exp::Int(4)),
    );

    // Evaluate the expression
    println!("e1 : {}", e1.to_num());
    println!("e1 : {}", e1);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.