Skip to content

ACP: Add {Ref,RefMut}::try_map method  #341

Open
@GrigorenkoPV

Description

@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.

  1. 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

rust-lang/rust#118087

Metadata

Metadata

Assignees

No one assigned

    Labels

    T-libs-apiapi-change-proposalA proposal to add or alter unstable APIs in the standard libraries

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions