Loops, moves, and variables

Hello all,

I'm working on the CodeCrafter's Redis project using Rust. I've started on the RESP parser.

I thought it would be interesting to build a parser that receives incrementally the bytes. The question is not so much about the algorithm per se, but using Rust idiomatically.

Here is the full parser: Redis Serialization Protocol · GitHub

I've created a type fragment

#[derive(Debug)]
pub enum Fragment {
    Value(RedisValue),
    Array(RedisValue, usize, usize),
}

That basically is an indicator of "Is this Aggregated type complete". RedisValue is just a representing the possible RESP values.

I created this method parse_next that receives bytes and tries to parse a full RedisValue from the bytes that is stored in the result attribute of the Parser struct

When ever I parse a RedisValue I check that if we have a "Fragment" of an Array in the stack (now that I'm writing this what a bad name...) if we do, that RedisValue goes into the Array. If you go an array in an array you have to check that if you completed the array on top of the stack and assign it to the value right before the top... I did this with a loop:

                let mut value_ = &mut value;
                let mut i = 0;
                loop {
                    match self.stack.pop() {
                        Some(Fragment::Value(value)) => {
                            let _ = std::mem::replace(value_, value);
                        }

                        Some(Fragment::Array(RedisValue::Array(mut values), size, current)) => {
                            let tmp = RedisValue::Null;
                            values.push(std::mem::replace(value_, tmp));
                            let current = current + 1;
                            if size <= current {
                                let _ = std::mem::replace(value_, RedisValue::Array(values));
                            } else if size > current {
                                self.stack.push(Fragment::Array(
                                    RedisValue::Array(values),
                                    size,
                                    current,
                                ));
                                break;
                            }
                        }
                        None => {
                            let tmp = RedisValue::Null;
                            self.result = Some(std::mem::replace(value_, tmp));
                            break;
                        }
                        Some(Fragment::Array(_, _, _)) => unreachable!(),
                    }
                }

Question:

  1. Because I move everything. As you can see there are no references I need to have this variable outside the loop and use replace. Is this a thing? What would be a better way to do this? Have box?
  2. I understand that matching Enums should be cheap, so recreating this values shouldn't be too much of an issue, it feels functional programming... Is this ok?

if you don't need the old value, just use assignment, no need to use replace():

-  let _ = std::mem::replace(value_, value);
+  *value_ = value;

what's more, I don't see the point to create the binding value_, why not just use the value variable? e.g.:

- let mut value_ = &mut value;
  ...
  match self.stack.pop() {
-   Some(Fragment::Value(value)) -> {
-     let _ = std::mem::replace(value_, value);
+   Some(Fragment::value(new_value)) -> {
+     value = new_value;
  ...

yes, destructuring and reconstructuring the enum basically moves the value of the variant, which should be cheap, unless your data type is very huge in size.

1 Like

Ah ha! I think I get it now! Use de dereference operator to assign! Silly me.

Thanks you @nerditation

I think the main problem here is the vectors.push I'm doing:

                        Some(Fragment::Array(RedisValue::Array(mut values), size, current)) => {
                            let tmp = RedisValue::Null;
                            values.push(std::mem::replace(value_, tmp));
                            let current = current + 1;
                            if size <= current {
                                let _ = std::mem::replace(value_, RedisValue::Array(values));
                            } else if size > current {
                                self.stack.push(Fragment::Array(
                                    RedisValue::Array(values),
                                    size,
                                    current,
                                ));
                                break;
                            }

That push there is a move, and so I have to replace it with something, right?

This seems to work, but looks "ugly":

            Ok(Fragment::Value(mut value)) => {
                loop {
                    match self.stack.pop() {
                        Some(Fragment::Value(new_value)) => {
                            value = new_value;
                        }

                        Some(Fragment::Array(RedisValue::Array(mut values), size, current)) => {
                            values.push(std::mem::replace(&mut value, RedisValue::Null));
                            let current = current + 1;
                            if size <= current {
                                value = RedisValue::Array(values);
                            } else if size > current {
                                self.stack.push(Fragment::Array(
                                    RedisValue::Array(values),
                                    size,
                                    current,
                                ));
                                break;
                            }
                        }
                        None => {
                            self.result = Some(value);
                            break;
                        }
                        Some(Fragment::Array(_, _, _)) => unreachable!(),
                    }
                }

update!

this compiles

            Ok(Fragment::Value(mut value)) => {
                loop {
                    match self.stack.pop() {
                        Some(Fragment::Value(new_value)) => {
                            value = new_value;
                        }

                        Some(Fragment::Array(RedisValue::Array(mut values), size, current)) => {
                            let current = current + 1;
                            if size <= current {
                                values.push(value);
                                value = RedisValue::Array(values);
                            } else if size > current {
                                values.push(value);
                                self.stack.push(Fragment::Array(
                                    RedisValue::Array(values),
                                    size,
                                    current,
                                ));
                                break;
                            }
                        }
                        None => {
                            self.result = Some(value);
                            break;
                        }
                        Some(Fragment::Array(_, _, _)) => unreachable!(),
                    }
                }

Also in a match statement Ok(Fragment::Value(mut value)) => { Is declaring a variable, let mut variable: TypeOfWhaterisInsideValue?

yes, you found the solution.

rust allows you move the old value out of a variable temporarily, then assign a new value to the variable, as long as the "moved-out-of" variable (with uninitialized value) is not used in between.

yes, a match statement creates a variable binding. actually, match is more fundamental than let.

in rust, a variable binding is created using an "identifier pattern" (or a pattern that has an identifier pattern as subpattern) while pattern matching. note, "pattern matching" is not just the match keyword, the let keyword is also pattern matching.

the most common case of simple a let statement can only use an irrefutable pattern, while if-let and let-else can use refutable patterns.

1 Like

Thank you so much for your time!