-
Notifications
You must be signed in to change notification settings - Fork 24
Description
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
- 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.
- 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
- Do we need to return the original
RefMutin 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.
- You have to
- Pros:
- What do we put in
Resut:Errin return value?- A struct (pros: field order doesn't matter, cons: Occam's razor)
- A tuple (pros: more ergonomic, cons: have to decided on the order of values)
- Can we generalize this using
Try/Residual?I have considered generalizing this to use the
Try&Residualjust like #79711 does forarray::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 withTry. (Plus I am not even sure if it is possible to express the fact that&UinTry::Outputwould have to have the same lifetime as the&Tinput ofF.)
[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)