CODEWITHSUNDEEP
rust
Shaun the Sheep
Shaun the Sheep

Reputation: 22742

Can match on Result here be replaced with map_err and "?"

I have some code which looks like this (greatly simplified version). A function takes two function arguments of type LoadClient and CheckApproval and returns either an error or a string.

pub struct Client {
    pub id: String,
}

pub enum MyErr {
    RequiresApproval(Client, String),
    LoadFailed,
}

pub fn authorize<LoadClient, CheckApproval>(load_client: LoadClient, check_approval: CheckApproval) -> Result<String, MyErr> 
where
   LoadClient: FnOnce(String) -> Result<Client, String>,
   CheckApproval: for<'a> FnOnce(&'a Client, &str) -> Result<&'a str, ()>,
{
    let client = load_client("hello".to_string()).map_err(|_| MyErr::LoadFailed)?;
    let permission = "something";

    // This doesn't compile
    // let authorized = check_approval(&client, permission).map_err(|_| MyErr::RequiresApproval(client, permission.to_string()))?;
    // Ok(authorized.to_string())

    // This version does
    match check_approval(&client, permission) {
        Err(_) => Err(MyErr::RequiresApproval(client, permission.to_string())),
        Ok(authorized) => Ok(authorized.to_string()),
    }
}

I'd like to used ? with the check_approval call (as the commented out code shows) for simpler code and to avoid the extra nesting - the Ok branch in the final match is actually a much longer block.

Unfortunately that doesn't compile:

error[E0505]: cannot move out of `client` because it is borrowed
  --> src/lib.rs:19:66
   |
19 |     let authorized = check_approval(&client, permission).map_err(|_| MyErr::RequiresApproval(client, permission.to_string()))?;
   |                                     -------              ------- ^^^                         ------ move occurs due to use in closure
   |                                     |                    |       |
   |                                     |                    |       move out of `client` occurs here
   |                                     |                    borrow later used by call
   |                                     borrow of `client` occurs here

These seem similar (to my untrained eye). Hasn't the borrowed reference to client been returned by the time map_err is called?

My main question: Is there a way to get round this and write the code without using match?

rust playground link.

Upvotes: 5

Views: 12651

Answers (2)

Richard Neumann
Richard Neumann

Reputation: 3361

You can convert the match statement to a functional expression in one go and thus also avoid the borrowing issue:

pub fn authorize<LoadClient, CheckApproval>(
    load_client: LoadClient,
    check_approval: CheckApproval,
) -> Result<String, MyErr>
where
    LoadClient: FnOnce(String) -> Result<Client, String>,
    CheckApproval: for<'a> FnOnce(&'a Client, &str) -> Result<&'a str, ()>,
{
    let client = load_client("hello".to_string()).map_err(|_| MyErr::LoadFailed)?;
    let permission = "something";

    check_approval(&client, permission)
        .map(|authorized| authorized.to_string())
        .map_err(|_| MyErr::RequiresApproval(client, permission.to_string()))
}

This way you do not need the ? operator or a temporary variable.

Upvotes: 0

rodrigo
rodrigo

Reputation: 98368

While your two versions of the code are semantically equivalent, they are actually quite different for the compiler.

The failing one calls Result::map_err() with a closure that captures the value of client. That is, client is moved into the closure, but it is borrowed when calling check_approval(). And here lies the error, a borrowed value cannot be moved.

You may think that this borrow should finish when the function returns, but that is not the case because of its return type Result<&'a str, ()>, being 'a precisely the lifetime of that borrow. The borrow of client is extended for as long as this 'a exists. And that is why your second version works: when you match your Result, the 'a does not extend to the Err(()) branch, only to the Ok(&'a str) so the Err(()) is able to move client freely.

Is there a way to get round this and write the code without using match?

Well, you are calling authorized.to_string() in the returned &'a str and converting it to an owned String. So, if you can change your CheckApproval constraint to:

CheckApproval: FnOnce(&Client, &str) -> Result<String, ()>,

the problem just goes away.

If you cannot change that, another option is to do the to_string() before moving the client into the closure, finishing the borrow before it can do harm:

let authorized = check_approval(&client, permission)
    .map(|a| a.to_string())
    .map_err(|_| MyErr::RequiresApproval(client, permission.to_string()))?;
Ok(authorized)

Upvotes: 4

Related Questions