Repository: MystenLabs/sui
File: external-crates/move/crates/move-vm-runtime/src/execution/values/values_impl.rs (Rust)

Code before:
            | Value::U64(_)
            | Value::U128(_)
            | Value::U256(_)
            | Value::Bool(_)
            | Value::Address(_)
            | Value::Vec(_)
            | Value::Struct(_)
            | Value::Variant(_)
            | Value::Reference(_) => {
                return Err(partial_vm_error!(
                    INTERNAL_TYPE_ERROR,
                    "expected vector<u8>, found {:?}",
                    self
                ));
            }
        };
        #[allow(clippy::unreachable)]
        // We just checked the type, so this match should always succeed. If it fails, who knows:
        // we should just panic full stop.
        Ok(std::cell::Ref::map(borrowed, |value| {
            match value {
                Value::PrimVec(PrimVec::VecU8(vec)) => vec,
                // We already checked, but provide a safe fallback
                _ => unreachable!("INTERNAL PANIC: Value changed between matching and borrowing."),
            }
        }))
    }

    pub fn pop(&self, type_param: &Type) -> PartialVMResult<Value> {
        let value = &mut *self.0.try_borrow_mut()?;
        check_elem_layout(type_param, value)?;

        macro_rules! pop_vec_item {
            ($items:expr, $value:ident, $rhs:expr) => {
                match $items.pop() {
                    Some($value) => Ok($rhs),
                    None => {
                        Err(partial_vm_error!(VECTOR_OPERATION_ERROR)
                            .with_sub_status(POP_EMPTY_VEC))
                    }
                }
            };
        }

        use PrimVec as PV;
        use VectorMatch as V;

        match value.vector_mut_ref()?.0 {
            V::PrimVec(PV::VecU8(xs)) => pop_vec_item!(xs, x, Value::U8(x)),
            V::PrimVec(PV::VecU16(xs)) => pop_vec_item!(xs, x, Value::U16(x)),
            V::PrimVec(PV::VecU32(xs)) => pop_vec_item!(xs, x, Value::U32(x)),

Review comment (refactor):
Would it be better to move this into the `PrimVec::VecU8` match arm (if that is possible?). 

Seems a bit more direct/less error prone if a new value were to be introduced in the future.

Code after:
                        Value::PrimVec(PrimVec::VecU8(vec)) => vec,
                        // We already checked, but provide a safe fallback
                        _ => unreachable!(
                            "INTERNAL PANIC: Value changed between matching and borrowing."
                        ),
                    }
                }))
            }
            Value::PrimVec(_)
            | Value::Invalid
            | Value::U8(_)
            | Value::U16(_)
            | Value::U32(_)
            | Value::U64(_)
            | Value::U128(_)
            | Value::U256(_)
            | Value::Bool(_)
            | Value::Address(_)
            | Value::Vec(_)
            | Value::Struct(_)
            | Value::Variant(_)
            | Value::Reference(_) => Err(partial_vm_error!(
                INTERNAL_TYPE_ERROR,
                "expected vector<u8>, found {:?}",
                self
            )),
        }
    }

    pub fn pop(&self, type_param: &Type) -> PartialVMResult<Value> {
        let value = &mut *self.0.try_borrow_mut()?;
        check_elem_layout(type_param, value)?;

        macro_rules! pop_vec_item {
            ($items:expr, $value:ident, $rhs:expr) => {
                match $items.pop() {
                    Some($value) => Ok($rhs),
                    None => {
                        Err(partial_vm_error!(VECTOR_OPERATION_ERROR)
                            .with_sub_status(POP_EMPTY_VEC))
                    }
                }
            };
        }

        use PrimVec as PV;
        use VectorMatch as V;

        match value.vector_mut_ref()?.0 {
            V::PrimVec(PV::VecU8(xs)) => pop_vec_item!(xs, x, Value::U8(x)),
            V::PrimVec(PV::VecU16(xs)) => pop_vec_item!(xs, x, Value::U16(x)),