ACP: Add `{Ref,RefMut}::try_map` method

[GrigorenkoPV]

Proposal: Add {Ref,RefMut}::try_map method

Problem statement

The filter_map method introduced in 1.63 extended the existing map method by allowing the conversion to be fallible. However, in case of a failure, the conversion doesn't have a straight-forward way to return any value.

Motivating examples or use cases

1. Consider this piece of code from the Rust compiler:

RefMut::filter_map(
    self.result.borrow_mut(),
    |r| r.get_or_insert_with(|| f().map(Steal::new)).as_mut().ok(),
)
.map_err(|r| *r.as_ref().unwrap().as_ref().map(|_| ()).unwrap_err())
.map(QueryResult)

Using try_map it can be rewritten as follows:

RefMut::try_map(
    self.result.borrow_mut(),
    |r| r.get_or_insert_with(|| f().map(Steal::new)).as_mut()
)
.map_err(|(_, &mut e)| e)
.map(QueryResult)

This removes the need to use 2 unwraps.

2. The use-case where I personally encountered the need for such API:

use anyhow::{anyhow, Error};
use std::collections::{hash_map::Entry as HashMapEntry, HashMap};

pub fn start_new_segment(
    segments: RefMut<'_, HashMap<Key, Vec<Value>>>,
    key: Key,
) -> Result<RefMut<'_, Vec<Value>>, Error> {
    RefMut::try_map(segments, |map| match map.entry(key) {
        HashMapEntry::Occupied(e) => Err(anyhow!("{} already exits: {:?}", e.key(), e.get())),
        HashMapEntry::Vacant(e) => Ok(e.insert(vec![])),
    })
    .map_err(|(_, e)| e)
}

Solution sketch

https://github.com/rust-lang/rust/pull/118087/files

Design considerations

1. Do we need to return the original RefMut in case of failure:
   • Pros:
     • More general, potentially covers more use-cases.
     • Consistent with filter_map.
   • Cons:
     • You have to .map_err(|(_, e)| e) to use ?
     • Need to return 2 values in Result::Err.
2. What do we put in Resut:Err in return value?
   1. A struct (pros: field order doesn't matter, cons: Occam's razor)
   2. A tuple (pros: more ergonomic, cons: have to decided on the order of values)
3. Can we generalize this using Try / Residual?

   I have considered generalizing this to use the Try & Residual just like #79711 does for array::try_map, but it does not seem to be possible: we want to essentially .map_err(|e| (orig, e)) but this does not seem to be supported with Try. (Plus I am not even sure if it is possible to express the fact that &U in Try::Output would have to have the same lifetime as the &T input of F.) [src]

Alternatives

External implementation using Option and unwrap

pub fn try_map<'a, T, U, E>(
    r: RefMut<'a, T>,
    f: impl FnOnce(&mut T) -> Result<&mut U, E>,
) -> Result<RefMut<'a, U>, (RefMut<'a, T>, E)>
where
    T: ?Sized + 'a,
    U: ?Sized + 'a,
{
    let mut error = None;
    RefMut::filter_map(r, |t| match f(t) {
        Ok(u) => Some(u),
        Err(e) => {
            error = Some(e);
            None
        }
    })
    .map_err(|r| (r, error.unwrap()))
}

External implementation using unsafe

pub fn try_map<'a, T, U, E>(
    mut r: RefMut<'a, T>,
    f: impl FnOnce(&mut T) -> Result<&mut U, E>,
) -> Result<RefMut<'a, U>, (RefMut<'a, T>, E)>
where
    T: ?Sized + 'a,
    U: ?Sized + 'a,
{
    let t: *mut T = r.deref_mut();
    let u: *mut U = match f(unsafe { &mut *t }) {
        Ok(u) => u,
        Err(e) => return Err((r, e)),
    };
    Ok(RefMut::<'a, T>::map(r, |_| unsafe { &mut *u }))
}

(Might be incorrect)

Links and related work

https://github.com/rust-lang/rust/pull/118087

[Slixe]

I also have a need of this feature which would greatly help with easier error handling when dealing with Ref/RefMut.

[GrigorenkoPV]

I also have a need of this feature which would greatly help with easier error handling when dealing with Ref/RefMut.

Well, this will probably take ages before it's accepted (and even more before it's stabilized), just as everything that has to do with libs-api, but meanwhile you can use one of the workaround from the OP.