Jacob Birkett
Jacob Birkett

Reputation: 2125

Assert that an `FnOnce` will only ever be used once in a loop

I am interfacing with a CLI utility and find myself writing a lot of duplicate code, for matching strings with regex and error handling. I wanted to reduce the mess by creating a reusable function that takes the compiled regex, the text to match against, and a closure that will be used to return a Result::Err in the event that any of the matching fails.

I am having trouble with the borrow checker, as per-usual. I want to re-use the same closure for all calls to this "helper" function, but cargo check tells me that it can't move the FnOnce because it doesn't implement Copy. The closure is FnOnce and not Fn because it takes ownership of command (std::process::Command) in order to pass it and all of its fields outside of the scope of the function in the event of a failure.

I, as a human, can see that the closure will only ever be called once, but rustc cannot.

The first block of code is showing how I am calling Clazz::captures, and the second is the static method containing the definition of the "helper".

        let account = {
            let email: String;
            let active: bool;
            let expires: NaiveDate;

            let fn_bad_output = || CliError::BadOutput(command);

            email = Self::captures(&RE_EMAIL, &stdout, fn_bad_output)?[0].to_owned();
            active = Self::captures(&RE_ACTIVE, &stdout, fn_bad_output)?[0] == "Active";
            expires = {
                let captures = Self::captures(&RE_EMAIL, &stdout, fn_bad_output)?;
                let date = format!("{}-{:02}-{}", captures[1], captures[2], captures[3]);

                NaiveDate::parse_from_str(&date, "%b-%d-%Y")?
            };

            Account {
                email,
                active,
                expires,
            }
        };
    fn captures<'a, F, E>(re: &Regex, text: &'a str, err: F) -> Result<Vec<&'a str>, E>
    where
        F: FnOnce() -> E,
        E: std::error::Error,
    {
        match re.captures(text) {
            Some(captures) => {
                let mut result = Vec::<&str>::new();

                for capture in captures.iter() {
                    let capture = capture.ok_or_else(err)?;
                    result.push(capture.as_str())
                }

                Ok(result)
            }
            None => Err(err()),
        }
    }

This is what cargo check tells me, including suggestions to restrict the generic bounds which I obviously cannot do.

error[E0382]: use of moved value: `fn_bad_output`
   --> src/cli.rs:109:58
    |
108 |             email = Self::captures(&RE_EMAIL, &stdout, fn_bad_output)?[0].to_owned();
    |                                                        ------------- value moved here
109 |             active = Self::captures(&RE_ACTIVE, &stdout, fn_bad_output)?[0] == "Active";
    |                                                          ^^^^^^^^^^^^^ value used here after move
    |
note: closure cannot be moved more than once as it is not `Copy` due to moving the variable `command` out of its environment
   --> src/cli.rs:106:56
    |
106 |             let fn_bad_output = || CliError::BadOutput(command);
    |                                                        ^^^^^^^

error[E0382]: use of moved value: `fn_bad_output`
   --> src/cli.rs:111:67
    |
109 |             active = Self::captures(&RE_ACTIVE, &stdout, fn_bad_output)?[0] == "Active";
    |                                                          ------------- value moved here
110 |             expires = {
111 |                 let captures = Self::captures(&RE_EMAIL, &stdout, fn_bad_output)?;
    |                                                                   ^^^^^^^^^^^^^ value used here after move
    |
note: closure cannot be moved more than once as it is not `Copy` due to moving the variable `command` out of its environment
   --> src/cli.rs:106:56
    |
106 |             let fn_bad_output = || CliError::BadOutput(command);
    |                                                        ^^^^^^^

error[E0382]: use of moved value: `err`
   --> src/cli.rs:441:54
    |
431 |     fn captures<'a, F, E>(re: &Regex, text: &'a str, err: F) -> Result<Vec<&'a str>, E>
    |                                                      --- move occurs because `err` has type `F`, which does not implement the `Copy` trait
...
441 |                     let capture = capture.ok_or_else(err)?;
    |                                                      ^^^ value moved here, in previous iteration of loop
    |
help: consider further restricting this bound
    |
433 |         F: FnOnce() -> E + Copy,
    |                          ++++++

I have included real source code from my project, if this does not satisfy MCVE I will replace it with a dummy, however I am terrible at coming up with realistic toy examples.

Upvotes: 0

Views: 425

Answers (1)

Vitaly Bogdanov
Vitaly Bogdanov

Reputation: 98

As @user4815162342 said in comments as BadOutput keeps command the Copy trait is not implemented for it. Therefore the fn_bad_output value is moved into the first call of the captures and cannot be moved into the second one.

The simplified version of your code which still reproduces the error (link to playground). I replaced all irrelevant types by () and command by String for simplicity.

struct BadOutput {
    command: String,
}

fn captures<F, E>(ok: bool, err: F) -> Result<(), E>
where
    F: FnOnce() -> E,
{
    if ok {
        Ok(())
    } else {
        Err(err())
    }
}

fn parse(command: String) -> Result<(), BadOutput> {
    let account = {
        let fn_bad_output = || BadOutput{ command };
        let _email = captures(true, fn_bad_output)?;
        let _active = captures(false, fn_bad_output)?;
        Ok(())
    };
    account
}

fn main() {
    parse("test".into());
}

If I understand your question correctly you are trying to simplify the error handling by implementing it in a single line of code (fn_bad_output implementation). To do this it is not necessary to pass fn_bad_output into captures. You can capture the error state of parse instead and convert it into a BadOutput kind of error. This requires one more function to wrap parse call and implement error handing in it but it makes overall code simpler.

The code below demonstrates this idea (link to the playground):

struct BadOutput {
    command: String,
}

fn captures(ok: bool) -> Result<(), ()>
{
    if ok {
        Ok(())
    } else {
        Err(())
    }
}

fn parse_bad_output(command: String)  -> Result<(), BadOutput> {
    parse().or_else(|_| Err(BadOutput{ command }))
}

fn parse() -> Result<(), ()> {
    let account = {
        let _email = captures(true)?;
        let _active = captures(false)?;
        Ok(())
    };
    account
}

fn main() {
    parse_bad_output("test".into());
}

You may have to return the information about the error into parse_bad_output. Then you need to replace second () in the return type of the captures and parse (Result<(), ()>) by the specific type and process returned value inside parse_bad_output.

Upvotes: 1

Related Questions