How to get value from within enum in a nice way

Hi!

I'm toying around with a stack for a virtual machine. The stack can hold a bunch of values, when you pop from the stack you know what you want to pop. If the value on top of the stack is not the value you expect we panic.

My current implementation looks like this:

#[derive(Debug)]
pub enum Value {
    Boolean(bool),
    Byte(u8),
    Short(i16),
    Int(i32),
    Long(i64),
    Char(char),
    Float(f32),
    Double(f64),
    Reference(u32),
    ReturnAddress(u32),
}

pub struct VirtualMachine {
    stack: Vec<Value>,
}

impl VirtualMachine {
    
    pub fn pop_int(&mut self) -> i32 {
        let value = self.stack.pop().expect("Tried to pop from empty stack.");
        match value {
            Value::Int(i) => i,
            _ => panic!("Tried to pop {:?} as an Int.", value),
        }
    }
    
    pub fn pop_reference(&mut self) -> u32 {
        let value = self.stack.pop().expect("Tried to pop from empty stack.");
        match value {
            Value::Reference(r) => r,
            _ => panic!("Tried to pop {:?} as a Reference.", value),
        }
    }
    
    pub fn pop_return_address(&mut self) -> u32 {
        let value = self.stack.pop().expect("Tried to pop from empty stack.");
        match value {
            Value::ReturnAddress(r) => r,
            _ => panic!("Tried to pop {:?} as a ReturnAddress.", value),
        }
    }
    
    // pub fn pop_byte, pop_long, pop_double, etc...
}

The code is very repetitive, with the getters for each type. Is there a better way of doing this without the repetition?

1 Like

What about using if let?

impl VirtualMachine {
  pub fn pop_int(&mut self) -> i32 {
    if let Some(Value::Int(i)) = self.stack.pop() {
      i
    } else {
      panic!()
    }
  }
}

I'd recommend against blowing up if your VM tries to pop the wrong thing though. Instead, return an Option and let the caller decide whether to crash or gracefully handle the error.

You can also write a macro_rules macro to generate these pop_XXX() method if you want to avoid a couple lines of duplication.

You could write a macro:

macro_rules! as_variant {
    ($value:expr, $variant:path) => {
        match $value {
            $variant(x) => Some(x),
            _ => None,
        }
    };
}

pub fn pop_int(&mut self) -> i32 {
    let value = self.stack.pop().expect("Tried to pop from empty stack.");
    as_variant(value, Value::Int).expect("Tried to pop an Int.")
}

Though I'd make a set of functions on Value like

pub fn as_int(self) -> Option<i32> {
    as_variant!(self, Value::Int)
}

// ... repeat for the rest

and then have your pop_x functions return an Option:

pub fn pop_int(&mut self) -> Option<i32> {
    self.stack.pop()?.as_int()
}

Though if Value is intended to be user-facing you could just have a fn pop(&mut self) -> Option<Value> method and let the user do the as_int.

1 Like

Yeah, the if let is probably better now when I only do one match.

There is no reason to not panic in this case because it is a fatal error. This is just a toy so I mess it up pretty often and want to know what the errors are. I assume the real implementation don't check type just assumes the stack contains the expected type.

I've been playing around with macros. I came up with this:

macro_rules! pop_type(
    ($stack:expr, $expected_type:tt) => {
        match $stack.pop_operand() {
            $expected_type(i) => i,
            value => panic!("Tried to pop {:?} as a {}", value, stringify!($expected_type)),
        }
    }
);

So I can pop:

let reference = pop_type!(vm, Reference);

Which is kind of nice syntax and has no duplication. However I don't like that the "public" api to this module will rely on a macro for popping.

Something like this would be very cool:

let reference: u32 = vm.pop().expect(Reference);

But I don't think it is possible to use enums that way. Maybe it's possible with a separate set of enums, one that holds the type and one that holds the values but that may be even more messy than my current solution.

1 Like

That's very interesting! That's pretty much the syntax I would want. I'll check that out!

Sure, just introduce your own trait which knows how to extract itself from a Value.

fn main() {
    let mut vm = VirtualMachine::default();
    vm.stack.push(Value::Boolean(true));

    let got: bool = vm.pop();
    println!("Got {}", got);
}

#[derive(Debug, Default, PartialEq)]
pub struct VirtualMachine {
    stack: Vec<Value>,
}

impl VirtualMachine {
    pub fn pop<T: FromValue>(&mut self) -> T {
        let value = self.stack.pop().expect("Empty stack");
        T::from_value(value)
    }
}

pub trait FromValue: Sized {
    fn from_value(value: Value) -> Self;
}

impl FromValue for bool {
    fn from_value(value: Value) -> bool {
        match value {
            Value::Boolean(b) => b,
            other => panic!("Expected a boolean but found {:?}", other),
        }
    }
}

(playground)

Note that you don't actually reduce the amount of code here. You still need to write the FromValue impls... Although there's no reason why they can't be generated via a macro :stuck_out_tongue_winking_eye:

1 Like

I've toyed with that idea a bit the the TryInto and Into traits. The problem is that there is at least two types that are the same type. For example the Reference(u32) and ReturnAddress(u32). So I don't think it's possible for this to fail if the top of the stack is a Reference but we expect a ReturnAddress?

Another low hanging fruit idea is to put a little more work into each match, like:

pub fn pop_int(&mut self) -> i32 {
    match self.stack.pop() {
        Some(Value::Int(i)) => i,
        Some(value) => panic!("Tried to pop {:?} as an Int.", value),
        None => panic!("Tried to pop from empty stack."),
    }
}

Though I'm not sure it buys much, it might look a little cleaner at least.

I think I will go with something like this:


macro_rules! into_raw(
    ($value:expr, $expected_type:tt) => {
        match $value {
            $expected_type(i) => i,
            value => panic!("Tried to use a {:?} as a {}", value, stringify!($expected_type)),
        }
    }
);

impl Value {
    pub fn into_raw_boolean(self) -> bool {
        into_raw!(self, Boolean)
    }
    
    // ...
}

pub struct VirtualMachine {
    stack: Vec<Value>,
}

impl VirtualMachine {
    pub fn pop(&mut self) -> Value {
        self.stack.pop().expect("Tried to pop from empty stack")
    }
}

I liked the idea of putting the responsibility of the conversion to the Value rather than the vm. I think the api is pretty clean as vm.pop().into_raw_reference(). Maybe vm.pop().expect_reference() is a better name for it since it may panic.

1 Like

A big repetetive impl block sounds like a potential use case for a proc macro. If you haven't made one before, generating getters for each enum variant would make for a good learning project I think.
I recently made my first (toy) proc macro that does something similar and had a good time implementing it: https://github.com/Heliozoa/enum-fields/blob/master/src/lib.rs

2 Likes

Thanks! Will look into that for sure, never heard of before

If you don’t mind lots of heap allocations, you can do this with Any and newtypes instead of an enum. You might even be able to find a crate that can avoid the heap allocation for small dynamically-sized types.

use std::any::Any;

pub trait StackValue: 'static + std::fmt::Debug { }

macro_rules! def_stack_values {
    ($($name:ident($repr:ty),)*) => {$(
        #[derive(Debug)] pub struct $name(pub $repr);
        impl StackValue for $name {}
    )*}
}

def_stack_values!{
    Boolean(bool),
    Byte(u8),
    Short(i16),
    Int(i32),
    Long(i64),
    Char(char),
    Float(f32),
    Double(f64),
    Reference(u32),
    ReturnAddress(u32),
}

#[derive(Default)]
pub struct Stack(Vec<Box<dyn Any>>);

impl Stack {
    pub fn push(&mut self, x: impl StackValue) {
        self.0.push(Box::new(x));
    }

    // Should probably return Result instead of panicing
    pub fn pop<T:StackValue>(&mut self)->T {
        let item = self.0.pop().unwrap();
        let downcast:Box<T> = item.downcast().unwrap();
        *downcast
    }
}

fn main() {
    let mut stack:Stack = std::default::Default::default();
    stack.push(Boolean(true));
    stack.push(Float(42.3));

    let Float(f) = stack.pop();
    let Boolean(b) = stack.pop();

    println!("{:?}", (f,b));
}

(Playground)

I can understand why one would want to avoid macros. Whenever I look at someone elses documentation and then use the src button and it jumps to some hard-to-understand macro block instead of a method, it's a huge turn-off for me.

Macros give me the same vibes as regular expressions: At first, you're intimidated by them, then you learn how to write them and think they're great. Finally, you write some monster-code once and nobody (including yourself) will be able to decipher it without lengthy documentation with good examples, at which point it would've been possible to write more verbose code, that is self-explanatory, instead.

Long story short, don't overuse them and you'll be fine.

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.