Stream: general

Topic: Is there any better way to write this code?


gfreezy (Mar 07 2019 at 16:24, on Zulip):
fn decode_message<T: Read>(
    tree: &mut MessageTree,
    transaction: Option<&mut InnerTransaction>,
    buf: &mut T,
) -> Fallible<()> {

    match transaction {
        None => {
            let mut chs = [0];
            loop {
                let size = buf.read(&mut chs)?;
                if size == 0 {
                    break;
                }
                let ch = chs[0];

                match dbg!(ch) {
                    b't' => decode_transaction(tree, None, buf)?,
                    b'T' => return Ok(()),
                    b'E' => decode_event(tree, None, buf)?,
                    b'M' => decode_metric(tree, None, buf)?,
                    b'H' => decode_heartbeat(tree, None, buf)?,
                    b'L' => decode_trace(tree, None, buf)?,
                    _ => unimplemented!("unsupported type"),
                }
            }
        },
        Some(transaction) => {
            let mut chs = [0];
            loop {
                let size = buf.read(&mut chs)?;
                if size == 0 {
                    break;
                }
                let ch = chs[0];

                match dbg!(ch) {
                    b't' => decode_transaction(tree, Some(transaction), buf)?,
                    b'T' => return Ok(()),
                    b'E' => decode_event(tree, Some(transaction), buf)?,
                    b'M' => decode_metric(tree, Some(transaction), buf)?,
                    b'H' => decode_heartbeat(tree, Some(transaction), buf)?,
                    b'L' => decode_trace(tree, Some(transaction), buf)?,
                    _ => unimplemented!("unsupported type"),
                }
            }
        }
    }

    Ok(())
}
oli (Mar 07 2019 at 16:36, on Zulip):

You could replace the entire match transaction with

let mut chs = [0];
            loop {
                let size = buf.read(&mut chs)?;
                if size == 0 {
                    break;
                }
                let ch = chs[0];

                match dbg!(ch) {
                    b't' => decode_transaction(tree, transaction, buf)?,
                    b'T' => return Ok(()),
                    b'E' => decode_event(tree, transaction, buf)?,
                    b'M' => decode_metric(tree, transaction, buf)?,
                    b'H' => decode_heartbeat(tree, transaction, buf)?,
                    b'L' => decode_trace(tree, transaction, buf)?,
                    _ => unimplemented!("unsupported type"),
                }
            }

and just pass the Option<&mut...> directly into your decode methods

pnkfelix (Mar 07 2019 at 23:33, on Zulip):

can they? its in a loop, that will move it (and thus it won't be available in the next loop iteration), right?

pnkfelix (Mar 07 2019 at 23:34, on Zulip):

i.e. I think one thing that may be happening is that their original code is reborrowing the &mut underneath the option in the Some case, no?

pnkfelix (Mar 07 2019 at 23:35, on Zulip):

(let me double-check this claim)

pnkfelix (Mar 07 2019 at 23:43, on Zulip):

yes, here is a demonstration of what I am saying (play):

fn foo_1(arg: Option<&mut Oops>) {
    match arg {
        Some(arg) => { loop { handle_arg(Some(arg)) } }
        None => { loop { handle_arg(None) } }
    }
}

fn foo_2(arg: Option<&mut Oops>) {
    loop { handle_arg(arg) }
}
pnkfelix (Mar 07 2019 at 23:43, on Zulip):

in the above, foo_1 compiles successfully while foo_2 has an error, "use of moved value arg"

pnkfelix (Mar 07 2019 at 23:47, on Zulip):

This one works, but yuck:

fn foo_3(mut arg: Option<&mut Oops>) {
    loop { handle_arg(arg.as_mut().map(|x|&mut **x)) }
}
pnkfelix (Mar 07 2019 at 23:50, on Zulip):

/me wonders if this pattern occurs enough to motivate a fn reborrow(&mut self) method on Option<&mut T> and the like

gfreezy (Mar 08 2019 at 02:41, on Zulip):

This works if i changed Option<&mut Oops> to &mut Option<Oops>

fn handle_arg(arg: &mut Option<Oops>) {

}

fn foo_1(arg: &mut Option<Oops>) {
    loop {
        handle_arg(arg)
    }
}
pnkfelix (Mar 08 2019 at 09:40, on Zulip):

@gfreezy yep, that's definitely nice, if it can be used. (note though that there are situations where you would't be able to do that transformation; e.g. if the &mut Oops were coming from somewhere else and you couldn't change the spot where the original Oops lives to hold an Option<Oops>)

Last update: Nov 21 2019 at 23:50UTC